Skip to contentSkip to conversation

raid5:Only move IO_THRESHOLD stripes from delay_list to hold_list in raid5_activate_delayed().

PATCH In review majianpeng 3 participants 4 messages Jul 6, 2012 – Jul 7, 2012 MD RAID

lore
Share & tools
lore

majianpeng

author
#0 lore raw

PATCH 1/1 raid5:Only move IO_THRESHOLD stripes from delay_list to hold_list in raid5_activate_delayed().

To imporve write performance and as posssible as to achieve full-write, it only fetch IO_THRESHOLD stripes from delay_list to hold_list.

Signed-off-by majianpeng <[email protected]>
---
 drivers/md/raid5.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/md/raid5.c +3 -0
index 04348d7..45308c5 100644
@@ -3662,6 +3662,7 @@ finish:
 
 static void raid5_activate_delayed(struct r5conf *conf)
 {
+	int count = 0;
 	if (atomic_read(&conf->preread_active_stripes) < IO_THRESHOLD) {
 		while (!list_empty(&conf->delayed_list)) {
 			struct list_head *l = conf->delayed_list.next;

@@ -3672,6 +3673,8 @@ static void raid5_activate_delayed(struct r5conf *conf)
 			if (!test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
 				atomic_inc(&conf->preread_active_stripes);
 			list_add_tail(&sh->lru, &conf->hold_list);
+			if (count++ >= IO_THRESHOLD)
+				break;
 		}
 	}
 }
Signature · 1 line
1.7.5.4
#1 lore raw

Dear Ma Jian,

thanks for the patch.

Am Freitag, den 06.07.2012, 20:38 +0800 schrieb majianpeng:

Quoted 1 line

To imporve write performance and as posssible as to achieve full-write,

s,imporve,improve,
s,posssible,possible,

The second part of the sentence is not clear to me.

Maybe: … and to achieve full-write if possible.

Quoted 1 line

it only fetch IO_THRESHOLD stripes from delay_list to hold_list.

Can you proof the speed up with some benchmarks? That would be awesome.

Quoted 1 line
Signed-off-by majianpeng <[email protected]>

Last not least, could you write your first and last names separated by a space? (I do not know if I separated them correctly.)

Ma Jian Peng (马建朋) <[email protected]>

    $ git config user.name "Ma Jian Peng (马建朋)"

Maybe even pass --global to it.

Quoted 17 lines
---
 drivers/md/raid5.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/md/raid5.c +1 -0
index 04348d7..45308c5 100644
@@ -3662,6 +3662,7 @@ finish:
 
 static void raid5_activate_delayed(struct r5conf *conf)
 {
+	int count = 0;
 	if (atomic_read(&conf->preread_active_stripes) < IO_THRESHOLD) {
 		while (!list_empty(&conf->delayed_list)) {
 			struct list_head *l = conf->delayed_list.next;

drivers/md/raid5.c static void raid5_activate_delayed(struct r5conf *conf) · line 3680

Quoted 8 lines
drivers/md/raid5.c +2 -0
@@ -3672,6 +3673,8 @@ static void raid5_activate_delayed(struct r5conf *conf)
 			if (!test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
 				atomic_inc(&conf->preread_active_stripes);
 			list_add_tail(&sh->lru, &conf->hold_list);
+			if (count++ >= IO_THRESHOLD)
+				break;
 		}
 	}
 }

Thanks,

Paul

author
#2 lore raw

On 2012-07-06 20:56 Paul Menzel <[email protected]> Wrote:

Quoted 12 lines

Dear Ma Jian,

thanks for the patch.

Am Freitag, den 06.07.2012, 20:38 +0800 schrieb majianpeng:

Quoted 1 line

To imporve write performance and as posssible as to achieve full-write,

s,imporve,improve,
s,posssible,possible,

Quoted 7 lines

The second part of the sentence is not clear to me.

Maybe: … and to achieve full-write if possible.

Quoted 1 line

it only fetch IO_THRESHOLD stripes from delay_list to hold_list.

Can you proof the speed up with some benchmarks? That would be awesome.

Yes, I will do

Quoted 10 lines
Quoted 2 lines
Signed-off-by majianpeng <[email protected]>

Last not least, could you write your first and last names separated by a space? (I do not know if I separated them correctly.)

Ma Jian Peng (马建朋) <[email protected]>

   $ git config user.name "Ma Jian Peng (马建朋)"

Thanks, I will accecpt it.

Quoted 33 lines

Maybe even pass --global to it.

Quoted 17 lines
---
 drivers/md/raid5.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/md/raid5.c +1 -0
index 04348d7..45308c5 100644
@@ -3662,6 +3662,7 @@ finish:
 
 static void raid5_activate_delayed(struct r5conf *conf)
 {
+	int count = 0;
 	if (atomic_read(&conf->preread_active_stripes) < IO_THRESHOLD) {
 		while (!list_empty(&conf->delayed_list)) {
 			struct list_head *l = conf->delayed_list.next;

drivers/md/raid5.c static void raid5_activate_delayed(struct r5conf *conf) · line 3680

Quoted 8 lines
drivers/md/raid5.c +2 -0
@@ -3672,6 +3673,8 @@ static void raid5_activate_delayed(struct r5conf *conf)
 			if (!test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
 				atomic_inc(&conf->preread_active_stripes);
 			list_add_tail(&sh->lru, &conf->hold_list);
+			if (count++ >= IO_THRESHOLD)
+				break;
 		}
 	}
 }

Thanks,

Paul

#3 lore raw
In reply to majianpeng
Quoted 4 lines · majianpeng

To imporve write performance and as posssible as to achieve full-write, it only fetch IO_THRESHOLD stripes from delay_list to hold_list.

Signed-off-by majianpeng <[email protected]>

Yes, I do think we need something like this to further limit the maximum in-flight pre-read, especially for the async / multithread xor case where we process stripes faster. However, this is the job of get_priority_stripe(), not raid5_activate_delayed(), to rate limit pre-reads versus full stripes. One pre-read stripe at a time seems to over bias full stripe writes. I wonder if you see the same performance effect by echo 0 > /sys/block/<mddev>/md/preread_bypass_threshold?

--
Dan