Commit 312045ee authored by NeilBrown's avatar NeilBrown

md: remove check for MD_RECOVERY_NEEDED in action_store.

md currently doesn't allow a 'sync_action' such as 'reshape' to be set
while MD_RECOVERY_NEEDED is set.

This s a problem, particularly since commit 738a2738 as that can
cause ->check_shape to call mddev_resume() which sets
MD_RECOVERY_NEEDED.  So by the time we come to start 'reshape' it is
very likely that MD_RECOVERY_NEEDED is still set.

Testing for this flag is not really needed and is in any case very
racy as it can be set at any moment - asynchronously.  Any race
between setting a sync_action and setting MD_RECOVERY_NEEDED must
already be handled properly in some locked code, probably
md_check_recovery(), so remove the test here.

The test on MD_RECOVERY_RUNNING is also racy in the 'reshape' case
so we should test it again after getting mddev_lock().

As this fixes a race and a regression which can cause 'reshape' to
fail, it is suitable for -stable kernels since 4.1
Reported-by: default avatarXiao Ni <xni@redhat.com>
Fixes: 738a2738 ("md/raid5: fix allocation of 'scribble' array.")
Cc: stable@vger.kernel.org (v4.1+)
Signed-off-by: default avatarNeilBrown <neilb@suse.com>
parent 9f7e4327
...@@ -4326,8 +4326,7 @@ action_store(struct mddev *mddev, const char *page, size_t len) ...@@ -4326,8 +4326,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
} }
mddev_unlock(mddev); mddev_unlock(mddev);
} }
} else if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || } else if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
test_bit(MD_RECOVERY_NEEDED, &mddev->recovery))
return -EBUSY; return -EBUSY;
else if (cmd_match(page, "resync")) else if (cmd_match(page, "resync"))
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery); clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
...@@ -4340,8 +4339,12 @@ action_store(struct mddev *mddev, const char *page, size_t len) ...@@ -4340,8 +4339,12 @@ action_store(struct mddev *mddev, const char *page, size_t len)
return -EINVAL; return -EINVAL;
err = mddev_lock(mddev); err = mddev_lock(mddev);
if (!err) { if (!err) {
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
err = -EBUSY;
else {
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery); clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
err = mddev->pers->start_reshape(mddev); err = mddev->pers->start_reshape(mddev);
}
mddev_unlock(mddev); mddev_unlock(mddev);
} }
if (err) if (err)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment