Skip to contentSkip to conversation

Patch "md: Close race when setting 'action' to 'idle'." has been added to the 4.0-stable tree

PATCH [email protected] 1 participant 1 message Jun 19, 2015

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

md: Close race when setting 'action' to 'idle'.

to the 4.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
md-close-race-when-setting-action-to-idle.patch
and it can be found in the queue-4.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 8e8e2518fceca407bb8fc2a6710d19d2e217892e Mon Sep 17 00:00:00 2001

From: NeilBrown <[email protected]>
Date: Fri, 12 Jun 2015 19:51:27 +1000
Subject: md: Close race when setting 'action' to 'idle'.

From: NeilBrown <[email protected]>

commit 8e8e2518fceca407bb8fc2a6710d19d2e217892e upstream.

Checking ->sync_thread without holding the mddev_lock() isn't really safe, even after flushing the workqueue which ensures md_start_sync() has been run.

While this code is waiting for the lock, md_check_recovery could reap the thread itself, and then start another thread (e.g. recovery might finish, then reshape starts). When this thread gets the lock md_start_sync() hasn't run so it doesn't get reaped, but MD_RECOVERY_RUNNING gets cleared. This allows two threads to start which leads to confusion.

So don't both if MD_RECOVERY_RUNNING isn't set, but if it is do the flush and the test and the reap all under the mddev_lock to avoid any race with md_check_recovery.

Fixes 6791875e2e53 ("md: make reconfig_mutex optional for writes to md sysfs files.")
---
 drivers/md/md.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/md/md.c +6 -5
@@ -4144,13 +4144,14 @@ action_store(struct mddev *mddev, const
 			set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
 		else
 			clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
-		flush_workqueue(md_misc_wq);
-		if (mddev->sync_thread) {
-			set_bit(MD_RECOVERY_INTR, &mddev->recovery);
-			if (mddev_lock(mddev) == 0) {
+		if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) &&
+		    mddev_lock(mddev) == 0) {
+			flush_workqueue(md_misc_wq);
+			if (mddev->sync_thread) {
+				set_bit(MD_RECOVERY_INTR, &mddev->recovery);
 				md_reap_sync_thread(mddev);
-				mddev_unlock(mddev);
 			}
+			mddev_unlock(mddev);
 		}
 	} else if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
 		   test_bit(MD_RECOVERY_NEEDED, &mddev->recovery))

Patches currently in stable-queue which might be from [email protected] are

queue-4.0/md-close-race-when-setting-action-to-idle.patch queue-4.0/md-don-t-return-0-from-array_state_store.patch queue-4.0/block-fix-ext_dev_lock-lockdep-report.patch queue-4.0/block-discard-bdi_unregister-in-favour-of-bdi_destroy.patch queue-4.0/blk-mq-free-hctx-ctxs-in-queue-s-release-handler.patch --
To unsubscribe from this list: send the line "unsubscribe stable" in