Re: [PATCH] MD: Allow restarting an interrupted incremental recovery.

[NeilBrown <neilb@xxxxxxx>]

On Mon, 17 Oct 2011 19:22:11 -0400 Andrei Warkentin <andreiw@xxxxxxxxxx>
wrote:

> If an incremental recovery was interrupted, a subsequent
> re-add will result in a full recovery, even though an
> incremental should be possible (seen with raid1).
> 
> Solve this problem by not updating the superblock on the
> recovering device until array is not degraded any longer.
> 
> Cc: Neil Brown <neilb@xxxxxxx>
> Signed-off-by: Andrei Warkentin <andreiw@xxxxxxxxxx>
> ---
>  drivers/md/md.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 5404b22..8ebbae4 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2444,9 +2444,12 @@ repeat:
>  			continue; /* no noise on spare devices */
>  		if (test_bit(Faulty, &rdev->flags))
>  			dprintk("(skipping faulty ");
> +		else if (rdev->saved_raid_disk != -1)
> +			dprintk("(skipping incremental s/r ");
>  
>  		dprintk("%s ", bdevname(rdev->bdev,b));
> -		if (!test_bit(Faulty, &rdev->flags)) {
> +		if (!test_bit(Faulty, &rdev->flags) &&
> +		    rdev->saved_raid_disk == -1) {
>  			md_super_write(mddev,rdev,
>  				       rdev->sb_start, rdev->sb_size,
>  				       rdev->sb_page);
> @@ -7353,15 +7356,16 @@ static void reap_sync_thread(mddev_t *mddev)
>  	if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
>  	    mddev->pers->finish_reshape)
>  		mddev->pers->finish_reshape(mddev);
> -	md_update_sb(mddev, 1);
>  
>  	/* if array is no-longer degraded, then any saved_raid_disk
> -	 * information must be scrapped
> +	 * information must be scrapped, and superblock for
> +	 * incrementally recovered device written out.
>  	 */
>  	if (!mddev->degraded)
>  		list_for_each_entry(rdev, &mddev->disks, same_set)
>  			rdev->saved_raid_disk = -1;
>  
> +	md_update_sb(mddev, 1);
>  	clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
>  	clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
>  	clear_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);


Thanks.  I've applied this and pushed it to my for-next branch.

My current HEAD use pr_debug instead of dprintk so I fixed that.

Also I realised that clearing saved_raid_disk when an array is not degraded
is no longer enough.  We also need to clear it when the device becomes
In_sync.
Consider a 3-drive RAID1 with two drives missing.  You add back one of them
and when it is recovered it needs saved_raid_disk cleared so that the
superblock gets written out.

So below is what I applied.

Thanks,
NeilBrown

commit d70ed2e4fafdbef0800e73942482bb075c21578b
Author: Andrei Warkentin <andreiw@xxxxxxxxxx>
Date:   Tue Oct 18 12:16:48 2011 +1100

    MD: Allow restarting an interrupted incremental recovery.
    
    If an incremental recovery was interrupted, a subsequent
    re-add will result in a full recovery, even though an
    incremental should be possible (seen with raid1).
    
    Solve this problem by not updating the superblock on the
    recovering device until array is not degraded any longer.
    
    Cc: Neil Brown <neilb@xxxxxxx>
    Signed-off-by: Andrei Warkentin <andreiw@xxxxxxxxxx>
    Signed-off-by: NeilBrown <neilb@xxxxxxx>

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0ea3485..e8d198d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2449,7 +2449,8 @@ repeat:
 		if (rdev->sb_loaded != 1)
 			continue; /* no noise on spare devices */
 
-		if (!test_bit(Faulty, &rdev->flags)) {
+		if (!test_bit(Faulty, &rdev->flags) &&
+		    rdev->saved_raid_disk == -1) {
 			md_super_write(mddev,rdev,
 				       rdev->sb_start, rdev->sb_size,
 				       rdev->sb_page);
@@ -2465,9 +2466,12 @@ repeat:
 				rdev->badblocks.size = 0;
 			}
 
-		} else
+		} else if (test_bit(Faulty, &rdev->flags))
 			pr_debug("md: %s (skipping faulty)\n",
 				 bdevname(rdev->bdev, b));
+		else
+			pr_debug("(skipping incremental s/r ");
+
 		if (mddev->level == LEVEL_MULTIPATH)
 			/* only need to write one superblock... */
 			break;
@@ -7366,15 +7370,19 @@ static void reap_sync_thread(struct mddev *mddev)
 	if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
 	    mddev->pers->finish_reshape)
 		mddev->pers->finish_reshape(mddev);
-	md_update_sb(mddev, 1);
 
-	/* if array is no-longer degraded, then any saved_raid_disk
-	 * information must be scrapped
+	/* If array is no-longer degraded, then any saved_raid_disk
+	 * information must be scrapped.  Also if any device is now
+	 * In_sync we must scrape the saved_raid_disk for that device
+	 * do the superblock for an incrementally recovered device
+	 * written out.
 	 */
-	if (!mddev->degraded)
-		list_for_each_entry(rdev, &mddev->disks, same_set)
+	list_for_each_entry(rdev, &mddev->disks, same_set)
+		if (!mddev->degraded ||
+		    test_bit(In_sync, &rdev->flags))
 			rdev->saved_raid_disk = -1;
 
+	md_update_sb(mddev, 1);
 	clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
 	clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
 	clear_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);

Attachment: signature.asc
Description: PGP signature