[PATCH v2 2/6] md: remove redundant check of 'mddev->sync_thread'
From: Yu Kuai <hidden>
Date: 2023-11-24 08:05:08
Also in:
lkml
Subsystem:
software raid (multiple disks) support, the rest · Maintainers:
Song Liu, Yu Kuai, Linus Torvalds
From: Yu Kuai <redacted>
The lifetime of sync_thread:
1) Set MD_RECOVERY_NEEDED and wake up daemon thread (by ioctl/sysfs or
other events);
2) Daemon thread woke up, md_check_recovery() found that
MD_RECOVERY_NEEDED is set:
a) try to grab reconfig_mutex;
b) set MD_RECOVERY_RUNNING;
c) clear MD_RECOVERY_NEEDED, and then queue sync_work;
3) md_start_sync() choose sync_action, then register sync_thread;
4) md_do_sync() is done, set MD_RECOVERY_DONE and wake up daemon thread;
5) Daemon thread woke up, md_check_recovery() found that
MD_RECOVERY_DONE is set:
a) try to grab reconfig_mutex;
b) unregister sync_thread;
c) clear MD_RECOVERY_RUNNING and MD_RECOVERY_DONE;
Hence if MD_RECOVERY_RUNNING is not set, mddev->sync_thread must be
NULL, there is no need to judge "mddev->sync_thread == NULL" if
MD_RECOVERY_RUNNING is not set.
Noted that unregister sync_thread(md_reap_sync_thread()) from other
context is wrong and will cause deadlock, for example commit
130443d60b1b ("md: refactor idle/frozen_sync_thread() to fix deadlock").
Signed-off-by: Yu Kuai <redacted>
---
drivers/md/md.c | 22 +++++++---------------
drivers/md/raid5.c | 6 ++----
2 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 1701e2fb219f..1d33a39d4f13 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c@@ -3302,8 +3302,7 @@ static ssize_t new_offset_store(struct md_rdev *rdev, if (kstrtoull(buf, 10, &new_offset) < 0) return -EINVAL; - if (mddev->sync_thread || - test_bit(MD_RECOVERY_RUNNING,&mddev->recovery)) + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) return -EBUSY; if (new_offset == rdev->data_offset) /* reset is always permitted */
@@ -3987,8 +3986,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len) */ rv = -EBUSY; - if (mddev->sync_thread || - test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || mddev->reshape_position != MaxSector || mddev->sysfs_active) goto out_unlock;
@@ -4898,8 +4896,7 @@ static void frozen_sync_thread(struct mddev *mddev) set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); stop_sync_thread(mddev); - wait_event(resync_wait, mddev->sync_thread == NULL && - !test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)); + wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)); mutex_unlock(&mddev->sync_mutex); }
@@ -6388,7 +6385,6 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev) mutex_lock(&mddev->open_mutex); if ((mddev->pers && atomic_read(&mddev->openers) > !!bdev) || - mddev->sync_thread || test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { pr_warn("md: %s still in use.\n",mdname(mddev)); if (did_freeze) {
@@ -6444,15 +6440,13 @@ static int do_md_stop(struct mddev *mddev, int mode, md_wakeup_thread_directly(mddev->sync_thread); mddev_unlock(mddev); - wait_event(resync_wait, (mddev->sync_thread == NULL && - !test_bit(MD_RECOVERY_RUNNING, - &mddev->recovery))); + 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) || mddev->sysfs_active || - mddev->sync_thread || test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { pr_warn("md: %s still in use.\n",mdname(mddev)); mutex_unlock(&mddev->open_mutex);
@@ -7298,8 +7292,7 @@ static int update_size(struct mddev *mddev, sector_t num_sectors) * of each device. If num_sectors is zero, we find the largest size * that fits. */ - if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || - mddev->sync_thread) + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) return -EBUSY; if (!md_is_rdwr(mddev)) return -EROFS;
@@ -7336,8 +7329,7 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks) if (raid_disks <= 0 || (mddev->max_disks && raid_disks >= mddev->max_disks)) return -EINVAL; - if (mddev->sync_thread || - test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || test_bit(MD_RESYNCING_REMOTE, &mddev->recovery) || mddev->reshape_position != MaxSector) return -EBUSY;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 4207e945e8c8..ec6cb8185207 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c@@ -7018,10 +7018,8 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len) pr_debug("md/raid: change stripe_size from %lu to %lu\n", conf->stripe_size, new); - if (mddev->sync_thread || - test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || - mddev->reshape_position != MaxSector || - mddev->sysfs_active) { + if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) || + mddev->reshape_position != MaxSector || mddev->sysfs_active) { err = -EBUSY; goto out_unlock; }
--
2.39.2