diff options
| author | NeilBrown <neilb@suse.de> | 2015-06-12 19:51:27 +1000 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2015-06-22 17:03:35 -0700 |
| commit | ad48fa9ae00009037975d535245dc8a63e997e4f (patch) | |
| tree | 0c35420f9526af44767ccdef35226819acdbe465 | |
| parent | 5dbedee441b11de887d945c655ace5f6215f5d4c (diff) | |
| download | lwn-ad48fa9ae00009037975d535245dc8a63e997e4f.tar.gz lwn-ad48fa9ae00009037975d535245dc8a63e997e4f.zip | |
md: Close race when setting 'action' to 'idle'.
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.
Signed-off-by: NeilBrown <neilb@suse.de>
Fixes: 6791875e2e53 ("md: make reconfig_mutex optional for writes to md sysfs files.")
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
| -rw-r--r-- | drivers/md/md.c | 11 |
1 files changed, 6 insertions, 5 deletions
diff --git a/drivers/md/md.c b/drivers/md/md.c index 907534b7f40d..5333d6167ca6 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -4144,13 +4144,14 @@ action_store(struct mddev *mddev, const char *page, size_t len) 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)) |