Re: Re: [PATCH 1/4] md/raid5: Reduce one write-operation when handle badsector

[majianpeng <majianpeng@gmail.com>]

>On Mon, 4 Mar 2013 09:52:57 +0800 majianpeng <majianpeng@gmail.com> wrote:
>
>> >On Thu, 28 Feb 2013 15:51:01 +0800 majianpeng <majianpeng@gmail.com> wrote:
>> >
>> >> For badsector,it took two steps:rewrite and reread.So for
>> >> write-operation,it make no sense to rewrite after write-operation.
>> >
>> >Please please please please please try to explain yourself better.
>> >
>> Sorry for that. 
>> >This patch doesn't make any sense.  The write succeeded.  We set the MadeGood
>> >flag so that later code will remove the block from the bad-block list.
>> >There will be no re-read or re-write.
>> >
>> For a striep which contain badsector is doing write-operation.
>> In analyse_stripe, it check badsector and set R5_ReadError.
>>  write--->MadeGood->clear badblock log. But it can't clear R5_ReadError flag.
>> So it will exec those code in func handle_stripe:
>
>After you wrote that, did you read it again to see if it makes any sense?  It
>does but only just.
>
>I'd rather have a good explanation in the code so it is clear what is going
>on.
>
>diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>index 3e0ec16..277d9c2 100644
>--- a/drivers/md/raid5.c
>+++ b/drivers/md/raid5.c
>@@ -1911,8 +1911,15 @@ static void raid5_end_write_request(struct bio *bi, int error)
> 					&rdev->mddev->recovery);
> 		} else if (is_badblock(rdev, sh->sector,
> 				       STRIPE_SECTORS,
>-				       &first_bad, &bad_sectors))
>+				       &first_bad, &bad_sectors)) {
> 			set_bit(R5_MadeGood, &sh->dev[i].flags);
>+			if (test_bit(R5_ReadError, &sh->dev[i].flags))
Why do it test R5_ReadError?
Even if  it didn't set R5_ReadError, it also do in analyse_stripe.

Thanks!
Jianpeng Ma
>+				/* That was a successful write so make
>+				 * sure it looks like we already did
>+				 * a re-write.
>+				 */
>+				set_bit(R5_ReWrite, &sh->dev[i].flags);
>+		}
> 	}
> 	rdev_dec_pending(rdev, conf->mddev);
> 
>Thanks,
>NeilBrown
>