Messages in this thread Patch in this message | | | Subject | Re: [PATCH -next 6/8] md: factor out a helper to stop sync_thread | From | Yu Kuai <> | Date | Fri, 24 Nov 2023 09:59:46 +0800 |
| |
Hi,
在 2023/11/21 21:01, Yu Kuai 写道: >>> >>> It looks like the three roles (md_set_readonly, do_md_stop and >>> stop_sync_thread) need to wait for different events. We can move these >>> codes out this helper function and make this helper function to be >>> more common. >> >> Or get lock again before returning this function and leave the wait here?
How about following patch?
drivers/md/md.c | 89 +++++++++++++++++++++++++++++++++++------------------------------------------------------ 1 file changed, 35 insertions(+), 54 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c index 78f32a2e434c..fe46b67f6e87 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -4868,35 +4868,18 @@ action_show(struct mddev *mddev, char *page) return sprintf(page, "%s\n", action_names[get_sync_action(mddev)]); }
-static int stop_sync_thread(struct mddev *mddev) +static void prepare_to_stop_sync_thread(struct mddev *mddev) + __releases(&mddev->reconfig_mutex) { - int err = mddev_lock(mddev); - - if (err) - return err; - - /* - * Check again in case MD_RECOVERY_RUNNING is cleared before lock is - * held. - */ - if (!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { - mddev_unlock(mddev); - return 0; - } - - if (work_pending(&mddev->sync_work)) - flush_workqueue(md_misc_wq); - set_bit(MD_RECOVERY_INTR, &mddev->recovery); /* - * Thread might be blocked waiting for metadata update which will now - * never happen + * Thread might be blocked waiting for metadata update which + * will now never happen. */ md_wakeup_thread_directly(mddev->sync_thread); - mddev_unlock(mddev); - - return 0; + if (work_pending(&mddev->sync_work)) + flush_work(&mddev->sync_work); }
static int idle_sync_thread(struct mddev *mddev) @@ -4905,13 +4888,17 @@ static int idle_sync_thread(struct mddev *mddev) int err;
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery); - err = stop_sync_thread(mddev); + err = mddev_lock(mddev); if (err) return err;
- err = wait_event_interruptible(resync_wait, + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { + prepare_to_stop_sync_thread(mddev); + err = wait_event_interruptible(resync_wait, sync_seq != atomic_read(&mddev->sync_seq) || !test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)); + } + return err; }
@@ -4920,12 +4907,15 @@ static int frozen_sync_thread(struct mddev *mddev) int err;
set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); - err = stop_sync_thread(mddev); + err = mddev_lock(mddev); if (err) return err;
- err = wait_event_interruptible(resync_wait, + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { + prepare_to_stop_sync_thread(mddev); + err = wait_event_interruptible(resync_wait, !test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)); + }
return err; } @@ -6350,11 +6340,11 @@ static void md_clean(struct mddev *mddev) static void __md_stop_writes(struct mddev *mddev) { set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); - if (work_pending(&mddev->sync_work)) - flush_workqueue(md_misc_wq); - if (mddev->sync_thread) { - set_bit(MD_RECOVERY_INTR, &mddev->recovery); - md_reap_sync_thread(mddev); + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { + prepare_to_stop_sync_thread(mddev); + wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING, + &mddev->recovery)); + mddev_lock_nointr(mddev); }
del_timer_sync(&mddev->safemode_timer); @@ -6447,18 +6437,15 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev) did_freeze = 1; set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); } - if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) - set_bit(MD_RECOVERY_INTR, &mddev->recovery);
- /* - * Thread might be blocked waiting for metadata update which will now - * never happen - */ - md_wakeup_thread_directly(mddev->sync_thread); + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { + prepare_to_stop_sync_thread(mddev); + wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING, + &mddev->recovery)); + } else { + mddev_unlock(mddev); + }
- mddev_unlock(mddev); - wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING, - &mddev->recovery)); wait_event(mddev->sb_wait, !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)); mddev_lock_nointr(mddev); @@ -6509,19 +6496,13 @@ static int do_md_stop(struct mddev *mddev, int mode, did_freeze = 1; set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); } - if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) - set_bit(MD_RECOVERY_INTR, &mddev->recovery); - - /* - * Thread might be blocked waiting for metadata update which will now - * never happen - */ - md_wakeup_thread_directly(mddev->sync_thread);
- mddev_unlock(mddev); - wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING, - &mddev->recovery)); - mddev_lock_nointr(mddev); + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { + prepare_to_stop_sync_thread(mddev); + wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING, + &mddev->recovery)); + mddev_lock_nointr(mddev); + }
mutex_lock(&mddev->open_mutex); if ((mddev->pers && atomic_read(&mddev->openers) > !!bdev) ||
| |