Commit f851b60d authored by NeilBrown's avatar NeilBrown

md: Check MD_RECOVERY_RUNNING as well as ->sync_thread.

A recent change to md started the ->sync_thread from a asynchronously
from a work_queue rather than synchronously.  This means that there
can be a small window between the time when MD_RECOVERY_RUNNING is set
and when ->sync_thread is set.

So code that checks ->sync_thread might now conclude that the thread
has not been started and (because a lock is held) will not be started.
That is no longer the case.

Most of those places are best fixed by testing MD_RECOVERY_RUNNING
as well.  To make this completely reliable, we wake_up(&resync_wait)
after clearing that flag as well as after clearing ->sync_thread.

Other places are better served by flushing the relevant workqueue
to ensure that that if the sync thread was starting, it has now
started.  This is particularly best if we are about to stop the
sync thread.

Fixes: ac05f256Signed-off-by: default avatarNeilBrown <neilb@suse.de>
parent 7d7e64f2
...@@ -2695,7 +2695,8 @@ static ssize_t new_offset_store(struct md_rdev *rdev, ...@@ -2695,7 +2695,8 @@ static ssize_t new_offset_store(struct md_rdev *rdev,
if (kstrtoull(buf, 10, &new_offset) < 0) if (kstrtoull(buf, 10, &new_offset) < 0)
return -EINVAL; return -EINVAL;
if (mddev->sync_thread) if (mddev->sync_thread ||
test_bit(MD_RECOVERY_RUNNING,&mddev->recovery))
return -EBUSY; return -EBUSY;
if (new_offset == rdev->data_offset) if (new_offset == rdev->data_offset)
/* reset is always permitted */ /* reset is always permitted */
...@@ -3272,6 +3273,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len) ...@@ -3272,6 +3273,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
*/ */
if (mddev->sync_thread || if (mddev->sync_thread ||
test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
mddev->reshape_position != MaxSector || mddev->reshape_position != MaxSector ||
mddev->sysfs_active) mddev->sysfs_active)
return -EBUSY; return -EBUSY;
...@@ -4026,6 +4028,7 @@ action_store(struct mddev *mddev, const char *page, size_t len) ...@@ -4026,6 +4028,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery); clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
if (cmd_match(page, "idle") || cmd_match(page, "frozen")) { if (cmd_match(page, "idle") || cmd_match(page, "frozen")) {
flush_workqueue(md_misc_wq);
if (mddev->sync_thread) { if (mddev->sync_thread) {
set_bit(MD_RECOVERY_INTR, &mddev->recovery); set_bit(MD_RECOVERY_INTR, &mddev->recovery);
md_reap_sync_thread(mddev); md_reap_sync_thread(mddev);
...@@ -5044,6 +5047,7 @@ static void md_clean(struct mddev *mddev) ...@@ -5044,6 +5047,7 @@ static void md_clean(struct mddev *mddev)
static void __md_stop_writes(struct mddev *mddev) static void __md_stop_writes(struct mddev *mddev)
{ {
set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
flush_workqueue(md_misc_wq);
if (mddev->sync_thread) { if (mddev->sync_thread) {
set_bit(MD_RECOVERY_INTR, &mddev->recovery); set_bit(MD_RECOVERY_INTR, &mddev->recovery);
md_reap_sync_thread(mddev); md_reap_sync_thread(mddev);
...@@ -5104,19 +5108,22 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev) ...@@ -5104,19 +5108,22 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev)
set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
md_wakeup_thread(mddev->thread); md_wakeup_thread(mddev->thread);
} }
if (mddev->sync_thread) { if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
set_bit(MD_RECOVERY_INTR, &mddev->recovery); set_bit(MD_RECOVERY_INTR, &mddev->recovery);
if (mddev->sync_thread)
/* Thread might be blocked waiting for metadata update /* Thread might be blocked waiting for metadata update
* which will now never happen */ * which will now never happen */
wake_up_process(mddev->sync_thread->tsk); wake_up_process(mddev->sync_thread->tsk);
}
mddev_unlock(mddev); mddev_unlock(mddev);
wait_event(resync_wait, mddev->sync_thread == NULL); wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING,
&mddev->recovery));
mddev_lock_nointr(mddev); mddev_lock_nointr(mddev);
mutex_lock(&mddev->open_mutex); mutex_lock(&mddev->open_mutex);
if ((mddev->pers && atomic_read(&mddev->openers) > !!bdev) || if ((mddev->pers && atomic_read(&mddev->openers) > !!bdev) ||
mddev->sync_thread || mddev->sync_thread ||
test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
(bdev && !test_bit(MD_STILL_CLOSED, &mddev->flags))) { (bdev && !test_bit(MD_STILL_CLOSED, &mddev->flags))) {
printk("md: %s still in use.\n",mdname(mddev)); printk("md: %s still in use.\n",mdname(mddev));
if (did_freeze) { if (did_freeze) {
...@@ -5162,20 +5169,24 @@ static int do_md_stop(struct mddev *mddev, int mode, ...@@ -5162,20 +5169,24 @@ static int do_md_stop(struct mddev *mddev, int mode,
set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
md_wakeup_thread(mddev->thread); md_wakeup_thread(mddev->thread);
} }
if (mddev->sync_thread) { if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
set_bit(MD_RECOVERY_INTR, &mddev->recovery); set_bit(MD_RECOVERY_INTR, &mddev->recovery);
if (mddev->sync_thread)
/* Thread might be blocked waiting for metadata update /* Thread might be blocked waiting for metadata update
* which will now never happen */ * which will now never happen */
wake_up_process(mddev->sync_thread->tsk); wake_up_process(mddev->sync_thread->tsk);
}
mddev_unlock(mddev); mddev_unlock(mddev);
wait_event(resync_wait, mddev->sync_thread == NULL); wait_event(resync_wait, (mddev->sync_thread == NULL &&
!test_bit(MD_RECOVERY_RUNNING,
&mddev->recovery)));
mddev_lock_nointr(mddev); mddev_lock_nointr(mddev);
mutex_lock(&mddev->open_mutex); mutex_lock(&mddev->open_mutex);
if ((mddev->pers && atomic_read(&mddev->openers) > !!bdev) || if ((mddev->pers && atomic_read(&mddev->openers) > !!bdev) ||
mddev->sysfs_active || mddev->sysfs_active ||
mddev->sync_thread || mddev->sync_thread ||
test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
(bdev && !test_bit(MD_STILL_CLOSED, &mddev->flags))) { (bdev && !test_bit(MD_STILL_CLOSED, &mddev->flags))) {
printk("md: %s still in use.\n",mdname(mddev)); printk("md: %s still in use.\n",mdname(mddev));
mutex_unlock(&mddev->open_mutex); mutex_unlock(&mddev->open_mutex);
...@@ -5950,7 +5961,8 @@ static int update_size(struct mddev *mddev, sector_t num_sectors) ...@@ -5950,7 +5961,8 @@ static int update_size(struct mddev *mddev, sector_t num_sectors)
* of each device. If num_sectors is zero, we find the largest size * of each device. If num_sectors is zero, we find the largest size
* that fits. * that fits.
*/ */
if (mddev->sync_thread) if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
mddev->sync_thread)
return -EBUSY; return -EBUSY;
if (mddev->ro) if (mddev->ro)
return -EROFS; return -EROFS;
...@@ -5981,7 +5993,9 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks) ...@@ -5981,7 +5993,9 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks)
if (raid_disks <= 0 || if (raid_disks <= 0 ||
(mddev->max_disks && raid_disks >= mddev->max_disks)) (mddev->max_disks && raid_disks >= mddev->max_disks))
return -EINVAL; return -EINVAL;
if (mddev->sync_thread || mddev->reshape_position != MaxSector) if (mddev->sync_thread ||
test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
mddev->reshape_position != MaxSector)
return -EBUSY; return -EBUSY;
rdev_for_each(rdev, mddev) { rdev_for_each(rdev, mddev) {
...@@ -7593,6 +7607,7 @@ static void md_start_sync(struct work_struct *ws) ...@@ -7593,6 +7607,7 @@ static void md_start_sync(struct work_struct *ws)
clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery); clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
clear_bit(MD_RECOVERY_CHECK, &mddev->recovery); clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery); clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
wake_up(&resync_wait);
if (test_and_clear_bit(MD_RECOVERY_RECOVER, if (test_and_clear_bit(MD_RECOVERY_RECOVER,
&mddev->recovery)) &mddev->recovery))
if (mddev->sysfs_action) if (mddev->sysfs_action)
...@@ -7761,6 +7776,7 @@ void md_check_recovery(struct mddev *mddev) ...@@ -7761,6 +7776,7 @@ void md_check_recovery(struct mddev *mddev)
not_running: not_running:
if (!mddev->sync_thread) { if (!mddev->sync_thread) {
clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery); clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
wake_up(&resync_wait);
if (test_and_clear_bit(MD_RECOVERY_RECOVER, if (test_and_clear_bit(MD_RECOVERY_RECOVER,
&mddev->recovery)) &mddev->recovery))
if (mddev->sysfs_action) if (mddev->sysfs_action)
...@@ -7779,7 +7795,6 @@ void md_reap_sync_thread(struct mddev *mddev) ...@@ -7779,7 +7795,6 @@ void md_reap_sync_thread(struct mddev *mddev)
/* resync has finished, collect result */ /* resync has finished, collect result */
md_unregister_thread(&mddev->sync_thread); md_unregister_thread(&mddev->sync_thread);
wake_up(&resync_wait);
if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery) && if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery) &&
!test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)) { !test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)) {
/* success...*/ /* success...*/
...@@ -7807,6 +7822,7 @@ void md_reap_sync_thread(struct mddev *mddev) ...@@ -7807,6 +7822,7 @@ void md_reap_sync_thread(struct mddev *mddev)
clear_bit(MD_RECOVERY_RESHAPE, &mddev->recovery); clear_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery); clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
clear_bit(MD_RECOVERY_CHECK, &mddev->recovery); clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
wake_up(&resync_wait);
/* flag recovery needed just to double check */ /* flag recovery needed just to double check */
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery); set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
sysfs_notify_dirent_safe(mddev->sysfs_action); sysfs_notify_dirent_safe(mddev->sysfs_action);
......
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