Re: Re: md/raid5:Fix recover/replace stop if handle stipe failed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I tested and corrected some bug.

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 23ac880..f8480f8 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2471,39 +2471,42 @@ handle_failed_sync(struct r5conf *conf, struct stripe_head *sh,
        int abort = 0;
        int i;
 
-       md_done_sync(conf->mddev, STRIPE_SECTORS, 0);
        clear_bit(STRIPE_SYNCING, &sh->state);
        s->syncing = 0;
        s->replacing = 0;
        /* There is nothing more to do for sync/check/repair.
+        * Don't even need to abort as that is handled elsewhere
+        * if needed, and not always wanted e.g. if there is a known
+        * bad block here.
         * For recover/replace we need to record a bad block on all
         * non-sync devices, or abort the recovery
         */
-       if (!test_bit(MD_RECOVERY_RECOVER, &conf->mddev->recovery))
-               return;
-       /* During recovery devices cannot be removed, so locking and
-        * refcounting of rdevs is not needed
-        */
-       for (i = 0; i < conf->raid_disks; i++) {
-               struct md_rdev *rdev = conf->disks[i].rdev;
-               if (rdev
-                   && !test_bit(Faulty, &rdev->flags)
-                   && !test_bit(In_sync, &rdev->flags)
-                   && !rdev_set_badblocks(rdev, sh->sector,
-                                          STRIPE_SECTORS, 0))
-                       abort = 1;
-               rdev = conf->disks[i].replacement;
-               if (rdev
-                   && !test_bit(Faulty, &rdev->flags)
-                   && !test_bit(In_sync, &rdev->flags)
-                   && !rdev_set_badblocks(rdev, sh->sector,
-                                          STRIPE_SECTORS, 0))
-                       abort = 1;
-       }
-       if (abort) {
-               conf->recovery_disabled = conf->mddev->recovery_disabled;
-               set_bit(MD_RECOVERY_INTR, &conf->mddev->recovery);
-       }
+       if (test_bit(MD_RECOVERY_RECOVER, &conf->mddev->recovery)) {
+               /* During recovery devices cannot be removed, so
+                * locking and refcounting of rdevs is not needed
+                */
+               for (i = 0; i < conf->raid_disks; i++) {
+                       struct md_rdev *rdev = conf->disks[i].rdev;
+                       if (rdev
+                           && !test_bit(Faulty, &rdev->flags)
+                           && !test_bit(In_sync, &rdev->flags)
+                           && !rdev_set_badblocks(rdev, sh->sector,
+                                                  STRIPE_SECTORS, 0))
+                               abort = 1;
+                       rdev = conf->disks[i].replacement;
+                       if (rdev
+                           && !test_bit(Faulty, &rdev->flags)
+                           && !test_bit(In_sync, &rdev->flags)
+                           && !rdev_set_badblocks(rdev, sh->sector,
+                                                  STRIPE_SECTORS, 0))
+                               abort = 1;
+               }
+               if (abort)
+                       conf->recovery_disabled =
+                               conf->mddev->recovery_disabled;
+       } else
+               abort = 1;
+       md_done_sync(conf->mddev, STRIPE_SECTORS, !abort);
 }
 
 static int want_replace(struct stripe_head *sh, int disk_idx)
@@ -3203,7 +3206,8 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
                        /* Not in-sync */;
                else if (is_bad) {
                        /* also not in-sync */
-                       if (!test_bit(WriteErrorSeen, &rdev->flags)) {
+                       if (!test_bit(WriteErrorSeen, &rdev->flags) &&
+                           test_bit(R5_UPTODATE, &dev->flags)) {
                                /* treat as in-sync, but with a read error
                                 * which we can now try to correct
                                 */


------------------				 
majianpeng
2012-03-27

-------------------------------------------------------------
发件人:NeilBrown
发送日期:2012-03-27 11:26:50
收件人:majianpeng
抄送:linux-raid
主题:Re: md/raid5:Fix recover/replace stop if handle stipe failed

On Wed, 14 Mar 2012 17:27:44 +0800 "majianpeng" <majianpeng@xxxxxxxxx> wrote:

> I created a raid5 using three disks and disk0 add bad blocks.I set faulty disk2 and remov disk2 and readd disk2.
> It seems to recover well and set disk2 badblocks  as disk0.
> But the md0_resync repeatly stop and start.
> The recovery_start of disk2 all the same .
> 

Thanks for the extra details (and sorry for the delay in replying).

There certainly is something wrong with handling bad blocks during recovery.

I think this patch should fix it.  Are you able to test it and confirm?

Thanks,
NeilBrown


diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 23ac880..2186e0e 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2471,39 +2471,41 @@ handle_failed_sync(struct r5conf *conf, struct stripe_head *sh,
 	int abort = 0;
 	int i;
 
-	md_done_sync(conf->mddev, STRIPE_SECTORS, 0);
 	clear_bit(STRIPE_SYNCING, &sh->state);
 	s->syncing = 0;
 	s->replacing = 0;
 	/* There is nothing more to do for sync/check/repair.
+	 * Don't even need to abort as that is handled elsewhere
+	 * if needed, and not always wanted e.g. if there is a known
+	 * bad block here.
 	 * For recover/replace we need to record a bad block on all
 	 * non-sync devices, or abort the recovery
 	 */
-	if (!test_bit(MD_RECOVERY_RECOVER, &conf->mddev->recovery))
-		return;
-	/* During recovery devices cannot be removed, so locking and
-	 * refcounting of rdevs is not needed
-	 */
-	for (i = 0; i < conf->raid_disks; i++) {
-		struct md_rdev *rdev = conf->disks[i].rdev;
-		if (rdev
-		    && !test_bit(Faulty, &rdev->flags)
-		    && !test_bit(In_sync, &rdev->flags)
-		    && !rdev_set_badblocks(rdev, sh->sector,
-					   STRIPE_SECTORS, 0))
-			abort = 1;
-		rdev = conf->disks[i].replacement;
-		if (rdev
-		    && !test_bit(Faulty, &rdev->flags)
-		    && !test_bit(In_sync, &rdev->flags)
-		    && !rdev_set_badblocks(rdev, sh->sector,
-					   STRIPE_SECTORS, 0))
-			abort = 1;
-	}
-	if (abort) {
-		conf->recovery_disabled = conf->mddev->recovery_disabled;
-		set_bit(MD_RECOVERY_INTR, &conf->mddev->recovery);
+	if (test_bit(MD_RECOVERY_RECOVER, &conf->mddev->recovery)) {
+		/* During recovery devices cannot be removed, so
+		 * locking and refcounting of rdevs is not needed
+		 */
+		for (i = 0; i < conf->raid_disks; i++) {
+			struct md_rdev *rdev = conf->disks[i].rdev;
+			if (rdev
+			    && !test_bit(Faulty, &rdev->flags)
+			    && !test_bit(In_sync, &rdev->flags)
+			    && !rdev_set_badblocks(rdev, sh->sector,
+						   STRIPE_SECTORS, 0))
+				abort = 1;
+			rdev = conf->disks[i].replacement;
+			if (rdev
+			    && !test_bit(Faulty, &rdev->flags)
+			    && !test_bit(In_sync, &rdev->flags)
+			    && !rdev_set_badblocks(rdev, sh->sector,
+						   STRIPE_SECTORS, 0))
+				abort = 1;
+		}
+		if (abort)
+			conf->recovery_disabled =
+				conf->mddev->recovery_disabled;
 	}
+	md_done_sync(conf->mddev, STRIPE_SECTORS, !abort);
 }
 
 static int want_replace(struct stripe_head *sh, int disk_idx)
@@ -3203,7 +3205,8 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
 			/* Not in-sync */;
 		else if (is_bad) {
 			/* also not in-sync */
-			if (!test_bit(WriteErrorSeen, &rdev->flags)) {
+			if (!test_bit(WriteErrorSeen, &rdev->flags) &&
+			    test_bit(R5_UPTODATE, &sh->devs[i].flags)) {
 				/* treat as in-sync, but with a read error
 				 * which we can now try to correct
 				 */
?韬{.n?????%??檩??w?{.n???{炳盯w???塄}?财??j:+v??????2??璀??摺?囤??z夸z罐?+?????w棹f



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux