Re: [BUG] md hang at schedule in md_write_start

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3636 bytes --]

On Mon, 12 Aug 2013 18:33:49 +0200 Jack Wang <jinpu.wang@profitbricks.com>
wrote:

> Hi Neil,
> 
> 
> We've found md hang in our test, it's easy to reproduce with script
> attached.
> 
> We've tried 3.4 stable kernel and latest mainline, it still exists.
> 
> Looks like flush bdi_writeback_workfn race with md_stop, no idea how to
> fix it, could you kindly give us suggestions?
> 
> Best regards,
> Jack

Thanks for the report.  I can see how that deadlock could happen.

Can you please try this patch and confirm that it fixes it.
I'm not really happy with this approach but nothing better occurs to me yet.

NeilBrown

diff --git a/drivers/md/md.c b/drivers/md/md.c
index a57b0fa..c66af69 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5144,7 +5144,7 @@ int md_run(struct mddev *mddev)
 	
 	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
 	
-	if (mddev->flags)
+	if (mddev->flags & MD_UPDATE_SB_FLAGS)
 		md_update_sb(mddev, 0);
 
 	md_new_event(mddev);
@@ -5289,7 +5289,7 @@ static void __md_stop_writes(struct mddev *mddev)
 	md_super_wait(mddev);
 
 	if (mddev->ro == 0 &&
-	    (!mddev->in_sync || mddev->flags)) {
+	    (!mddev->in_sync || (mddev->flags & MD_UPDATE_SB_FLAGS))) {
 		/* mark array as shutdown cleanly */
 		mddev->in_sync = 1;
 		md_update_sb(mddev, 1);
@@ -5337,8 +5337,11 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev)
 		err = -EBUSY;
 		goto out;
 	}
-	if (bdev)
+	if (bdev) {
+		set_bit(MD_FINAL_FLUSH, &mddev->flags);
 		sync_blockdev(bdev);
+		clear_bit(MD_FINAL_FLUSH, &mddev->flags);
+	}
 	if (mddev->pers) {
 		__md_stop_writes(mddev);
 
@@ -5373,13 +5376,16 @@ static int do_md_stop(struct mddev * mddev, int mode,
 		mutex_unlock(&mddev->open_mutex);
 		return -EBUSY;
 	}
-	if (bdev)
+	if (bdev) {
 		/* It is possible IO was issued on some other
 		 * open file which was closed before we took ->open_mutex.
 		 * As that was not the last close __blkdev_put will not
 		 * have called sync_blockdev, so we must.
 		 */
+		set_bit(MD_FINAL_FLUSH, &mddev->flags);
 		sync_blockdev(bdev);
+		clear_bit(MD_FINAL_FLUSH, &mddev->flags);
+	}
 
 	if (mddev->pers) {
 		if (mddev->ro)
@@ -7814,7 +7820,7 @@ void md_check_recovery(struct mddev *mddev)
 				sysfs_notify_dirent_safe(mddev->sysfs_state);
 		}
 
-		if (mddev->flags)
+		if (mddev->flags & MD_UPDATE_SB_FLAGS)
 			md_update_sb(mddev, 0);
 
 		if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) &&
@@ -7904,7 +7910,10 @@ void md_check_recovery(struct mddev *mddev)
 					sysfs_notify_dirent_safe(mddev->sysfs_action);
 		}
 		mddev_unlock(mddev);
-	}
+	} else if (test_bit(MD_FINAL_FLUSH, &mddev->flags) &&
+		   mddev->in_sync == 0 &&
+		   (mddev->flags & MD_UPDATE_SB_FLAGS))
+		md_update_sb(mddev, 0);
 }
 
 void md_reap_sync_thread(struct mddev *mddev)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 77924d3..e1e003a 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -209,7 +209,12 @@ struct mddev {
 #define MD_CHANGE_DEVS	0	/* Some device status has changed */
 #define MD_CHANGE_CLEAN 1	/* transition to or from 'clean' */
 #define MD_CHANGE_PENDING 2	/* switch from 'clean' to 'active' in progress */
+#define MD_UPDATE_SB_FLAGS (1 | 2 | 4)	/* If these are set, md_update_sb needed */
 #define MD_ARRAY_FIRST_USE 3    /* First use of array, needs initialization */
+#define MD_FINAL_FLUSH	4	/* md_check_recovery is permitted to call
+				 * md_update_sb() to switch to 'active'
+				 * without taking reconfig_mutex
+				 */
 
 	int				suspended;
 	atomic_t			active_io;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]