md/raid5: Reduce one write-operation when handle badsector
PATCH 1/4 md/raid5: Reduce one write-operation when handle badsector
For badsector,it took two steps:rewrite and reread.So for write-operation,it make no sense to rewrite after write-operation.
--- drivers/md/raid5.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/md/raid5.c +3 -1
@@ -1921,8 +1921,10 @@ 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); +set_bit (R5_ReWrite, &sh->dev[i].flags); + } }rdev_dec_pending (rdev, conf->mddev);
Signature · 1 line
1.7.9.5
Quoted 2 lines · On Thu, 28 Feb 2013 15:51:01 +0800 majianpeng <[email protected]> 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.
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.
NeilBrown
Quoted 22 lines
Signed-off-by Jianpeng Ma <[email protected]>--- drivers/md/raid5.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/md/raid5.c +3
-1@@ -1921,8 +1921,10 @@ 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); +set_bit (R5_ReWrite, &sh->dev[i].flags); + } }rdev_dec_pending (rdev, conf->mddev);
Quoted 7 lines · On Thu, 28 Feb 2013 15:51:01 +0800 majianpeng <[email protected]> wrote:
Quoted 3 lines
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.
Quoted 4 lines
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:
Quoted 15 lines
/* If the failed drives are just a ReadError, then we might need
* to progress the repair/check process
*/
if (s.failed <= conf->max_degraded && !conf->mddev->ro)
for (i = 0; i < s.failed; i++) {
struct r5dev *dev = &sh->dev[s.failed_num[i]];
if (test_bit(R5_ReadError, &dev->flags)
&& !test_bit(R5_LOCKED, &dev->flags)
&& test_bit(R5_UPTODATE, &dev->flags)
) {
if (!test_bit(R5_ReWrite, &dev->flags)) {
set_bit(R5_Wantwrite, &dev->flags);
set_bit(R5_ReWrite, &dev->flags);
set_bit(R5_LOCKED, &dev->flags);
s.locked++;Quoted 8 lines
} else {
/* let's read it back */
set_bit(R5_Wantread, &dev->flags);
set_bit(R5_LOCKED, &dev->flags);
s.locked++;
}
}
} But the flag ReWrite can't set. So it do write-operatoin again.Then do read-operation.
Thanks!
Jianpeng Ma
Quoted 27 lines
NeilBrown
Quoted 22 lines
Signed-off-by Jianpeng Ma <[email protected]>--- drivers/md/raid5.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/md/raid5.c +3
-1@@ -1921,8 +1921,10 @@ 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); +set_bit (R5_ReWrite, &sh->dev[i].flags); + } }rdev_dec_pending (rdev, conf->mddev);
Quoted 8 lines · On Mon, 4 Mar 2013 09:52:57 +0800 majianpeng <[email protected]> wrote:
Quoted 7 lines · On Thu, 28 Feb 2013 15:51:01 +0800 majianpeng <[email protected]> wrote:
Quoted 3 lines
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.
Quoted 8 lines
Quoted 4 lines
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.
Inanalyse_stripe, it check badsector and setR5_ReadError.
write--->MadeGood->clear badblock log. But it can't clearR5_ReadErrorflag.
So it will exec those code in funchandle_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.
drivers/md/raid5.c +8 -1
@@ -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)) +/* 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
Quoted 30 lines · On Mon, 4 Mar 2013 09:52:57 +0800 majianpeng <[email protected]> wrote:
Quoted 9 lines
Quoted 8 lines · On Thu, 28 Feb 2013 15:51:01 +0800 majianpeng <[email protected]> wrote:
Quoted 3 lines
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.
Quoted 8 lines
Quoted 4 lines
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.
Inanalyse_stripe, it check badsector and setR5_ReadError.
write--->MadeGood->clear badblock log. But it can't clearR5_ReadErrorflag.
So it will exec those code in funchandle_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.
drivers/md/raid5.c +0
-0
drivers/md/raid5.c static void raid5_end_write_request(struct bio *bi, int error) · line 1916
Quoted 7 lines
drivers/md/raid5.c +2
-1@@ -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
Quoted 12 lines
drivers/md/raid5.c +6
-0+/* 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