Re: [PATCH -next 6/8] md: factor out a helper to stop sync_thread
From: Yu Kuai <hidden>
Date: 2023-11-24 01:59:55
Also in:
lkml
Subsystem:
software raid (multiple disks) support, the rest · Maintainers:
Song Liu, Yu Kuai, Linus Torvalds
Hi, 在 2023/11/21 21:01, Yu Kuai 写道:
quoted
quoted
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) ||