Re: [PATCH v4 1/3] md: suspend array before raid10 reshape via sync_action
"yu kuai" <[email protected]>
| Newsgroups | gmane.linux.raid,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi, 在 2026/6/3 11:59, Chen Cheng 写道: > From: Chen Cheng <[email protected]> > > The sync_action=reshape path currently enters mddev_start_reshape() with > reconfig_mutex held but without suspending the array first. For raid10, > that means raid10_start_reshape() has to drop reconfig_mutex and reacquire > the array through mddev_suspend_and_lock_nointr() before it can safely > switch geometry-dependent state. > > Use mddev_suspend_and_lock() for ACTION_RESHAPE in action_store(), so > the sysfs reshape path reaches mddev_start_reshape() with the array > already suspended and locked. > > Other sync_action operations keep using mddev_lock() unchanged. > > Signed-off-by: Chen Cheng <[email protected]> > --- > drivers/md/md.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 096bb64e87bd..5bc937e149ac 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -5256,30 +5256,39 @@ static int mddev_start_reshape(struct mddev *mddev) > > static ssize_t > action_store(struct mddev *mddev, const char *page, size_t len) > { > int ret; > + bool suspended = false; > enum sync_action action; > > if (!mddev->pers || !mddev->pers->sync_request) > return -EINVAL; > > + action = md_sync_action_by_name(page); > retry: > if (work_busy(&mddev->sync_work)) > flush_work(&mddev->sync_work); > > - ret = mddev_lock(mddev); > + if (action == ACTION_RESHAPE) { > + ret = mddev_suspend_and_lock(mddev); suspend during retry is too heavy. Please move suspend above retry. > + suspended = true; > + } else { > + ret = mddev_lock(mddev); > + suspended = false; > + } > if (ret) > return ret; > > if (work_busy(&mddev->sync_work)) { > - mddev_unlock(mddev); > + if (suspended) > + mddev_unlock_and_resume(mddev); > + else > + mddev_unlock(mddev); > goto retry; > } > > - action = md_sync_action_by_name(page); > - > /* TODO: mdadm rely on "idle" to start sync_thread. */ > if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { > switch (action) { > case ACTION_FROZEN: > md_frozen_sync_thread(mddev); > @@ -5344,11 +5353,14 @@ action_store(struct mddev *mddev, const char *page, size_t len) > md_wakeup_thread(mddev->thread); > sysfs_notify_dirent_safe(mddev->sysfs_action); > ret = len; > > out: > - mddev_unlock(mddev); > + if (suspended) > + mddev_unlock_and_resume(mddev); > + else > + mddev_unlock(mddev); > return ret; > } > > static struct md_sysfs_entry md_scan_mode = > __ATTR_PREALLOC(sync_action, S_IRUGO|S_IWUSR, action_show, action_store); -- Thanks, Kuai