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

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

[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 <majianpeng@gmail.com>
---
 drivers/md/raid5.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 04348d7..45308c5 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -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;
 		}
 	}
 }
-- 
1.7.5.4

Re: [PATCH] raid5:Only move IO_THRESHOLD stripes from delay_list to hold_list in raid5_activate_delayed().

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]

Dear Ma Jian,


thanks for the patch.


Am Freitag, den 06.07.2012, 20:38 +0800 schrieb majianpeng:
> 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.

> 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.

> Signed-off-by: majianpeng <majianpeng@gmail.com>

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 (马建朋) <majianpeng@gmail.com>

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

Maybe even pass `--global` to it.

> ---
>  drivers/md/raid5.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 04348d7..45308c5 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -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;
>  		}
>  	}
>  }


Thanks,

Paul

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Re: [PATCH] raid5:Only move IO_THRESHOLD stripes from delay_list to hold_list in raid5_activate_delayed().

[majianpeng]

On 2012-07-06 20:56 Paul Menzel <paulepanter@users.sourceforge.net> Wrote:
>Dear Ma Jian,
>
>
>thanks for the patch.
>
>
>Am Freitag, den 06.07.2012, 20:38 +0800 schrieb majianpeng:
>> 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.
>
>> 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 
>
>> Signed-off-by: majianpeng <majianpeng@gmail.com>
>
>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 (马建朋) <majianpeng@gmail.com>
>
>    $ git config user.name "Ma Jian Peng (马建朋)"
>
Thanks, I will accecpt it.
>Maybe even pass `--global` to it.
>
>> ---
>>  drivers/md/raid5.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>> 
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 04348d7..45308c5 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -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;
>>  		}
>>  	}
>>  }
>
>
>Thanks,
>
>Paul
>

Re: [PATCH] raid5:Only move IO_THRESHOLD stripes from delay_list to hold_list in raid5_activate_delayed().

[Dan Williams]

On Fri, Jul 6, 2012 at 5:38 AM, majianpeng <majianpeng@gmail.com> wrote:
> 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 <majianpeng@gmail.com>

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