md: fix race when unfreezing sync_action

A recent change removed the need for locking around writing
to "sync_action" (and various other places), but introduced a
subtle race.
When e.g. setting 'reshape' on a 'frozen' array, the 'frozen'
flag is cleared before 'reshape' is set, so the md thread can
get in and start trying recovery - which isn't wanted.

So instead of clearing MD_RECOVERY_FROZEN for any command
except 'frozen', only clear it when each specific command
is parsed.  This allows the handling of 'reshape' to clear
the bit while a lock is held.

Also remove some places where we set MD_RECOVERY_NEEDED,
as it is always set on non-error exit of the function.


Signed-off-by: NeilBrown <neilb@suse.de>
Fixes: 6791875e2e53 ("md: make reconfig_mutex optional for writes to md sysfs files.")

1 files changed, 8 insertions, 6 deletions

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d4f31e195e26..8f10f4ea70ea 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4211,12 +4211,12 @@ action_store(struct mddev *mddev, const char *page, size_t len)
         if (!mddev->pers || !mddev->pers->sync_request)
                 return -EINVAL;
 
-        if (cmd_match(page, "frozen"))
-                set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
-        else
-                clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
 
         if (cmd_match(page, "idle") || cmd_match(page, "frozen")) {
+                if (cmd_match(page, "frozen"))
+                        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);
@@ -4229,16 +4229,17 @@ action_store(struct mddev *mddev, const char *page, size_t len)
                    test_bit(MD_RECOVERY_NEEDED, &mddev->recovery))
                 return -EBUSY;
         else if (cmd_match(page, "resync"))
-                set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
+                clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
         else if (cmd_match(page, "recover")) {
+                clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
                 set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
-                set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
         } else if (cmd_match(page, "reshape")) {
                 int err;
                 if (mddev->pers->start_reshape == NULL)
                         return -EINVAL;
                 err = mddev_lock(mddev);
                 if (!err) {
+                        clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
                         err = mddev->pers->start_reshape(mddev);
                         mddev_unlock(mddev);
                 }
@@ -4250,6 +4251,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
                         set_bit(MD_RECOVERY_CHECK, &mddev->recovery);
                 else if (!cmd_match(page, "repair"))
                         return -EINVAL;
+                clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
                 set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
                 set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
         }