Re: Re: [PATCH 3/4] md: Don't do normal-write on unresync area of replacement-disk.

[majianpeng <majianpeng@gmail.com>]

>On Thu, 28 Feb 2013 15:50:37 +0800 majianpeng <majianpeng@gmail.com> wrote:
>
>> Replacement is a fullsync which don't depent on bitmap.So regardless of
>> the presence and absence of bitmap, it do full resync.
>> If offset of normal io is larger than offset of resync,it will write
>> again when resync arrived this offset.
>
>This might be OK for RAID1 and RAID10 as recover is paused when writes
>happen, but that is not the case for RAID5, so it isn't safe to test against
>curr_resync - it gets updated a bit too later.
>
->curr_resync + STRIPE_SECTORS is the next stripe which willbe replaced.
How about the ->curr_resync+STRIPE_SECTORS?

>Also you messed up the formatting in raid10.c
>
Can you explain in detail?
>I'm not convinced this optimisation is really worth it.
>
Maybe for HDD disk, it only improve speed by reducing some write operation.
But for ssd disk, it can reduce one write.

Thanks!
Jianpeng Ma
>NeilBrown
>
>
>> 
>> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
>> ---
>>  drivers/md/raid1.c  |    4 +++-
>>  drivers/md/raid10.c |    4 +++-
>>  drivers/md/raid5.c  |    3 ++-
>>  3 files changed, 8 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index d5bddfc..142a5fa 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -1173,7 +1173,9 @@ read_again:
>>  				set_bit(R1BIO_Degraded, &r1_bio->state);
>>  			continue;
>>  		}
>> -
>> +		if (test_bit(Replacement, &rdev->flags) &&
>> +			conf->mddev->curr_resync < r1_bio->sector)
>> +			continue;
>>  		atomic_inc(&rdev->nr_pending);
>>  		if (test_bit(WriteErrorSeen, &rdev->flags)) {
>>  			sector_t first_bad;
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 64d4824..bb11cfb 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -1337,7 +1337,9 @@ retry_write:
>>  			     || test_bit(Unmerged, &rdev->flags)))
>>  			rdev = NULL;
>>  		if (rrdev && (test_bit(Faulty, &rrdev->flags)
>> -			      || test_bit(Unmerged, &rrdev->flags)))
>> +			|| test_bit(Unmerged, &rrdev->flags) || 
>> +			(test_bit(Replacement, &rrdev->flags) &&
>> +			conf->mddev->curr_resync < r10_bio->sector)))
>>  			rrdev = NULL;
>>  
>>  		r10_bio->devs[i].bio = NULL;
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index bd49623..e0a2a39 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -602,7 +602,8 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
>>  			rdev = NULL;
>>  		if (rdev)
>>  			atomic_inc(&rdev->nr_pending);
>> -		if (rrdev && test_bit(Faulty, &rrdev->flags))
>> +		if (rrdev && (test_bit(Faulty, &rrdev->flags) ||
>> +			conf->mddev->curr_resync < sh->sector))
>>  			rrdev = NULL;
>>  		if (rrdev)
>>  			atomic_inc(&rrdev->nr_pending);
>
>