From: Jack Wang <[email protected]> mddev_update_io_opt() runs from end_reshape() in the sync thread, and md_reap_sync_thread() waits for that thread with reconfig_mutex held. Taking q->limits_lock there hangs a finishing reshape whenever the lock's holder waits for I/O that only md_check_recovery() can let complete, and no ordering avoids it: the sync thread is what lets that I/O finish.
Hand the update to a work item, which holds neither reconfig_mutex nor the suspend and so takes q->limits_lock in the order the rest of md uses, before suspending. __md_stop() flushes it, as it suspends the array. Also give the function a queue_limits argument, so a caller that already owns an update has it changed in place; the users of that path follow. Assisted-by: LLM Signed-off-by: Jack Wang <[email protected]> --- drivers/md/md.c | 49 ++++++++++++++++++++++++++++++++++++++------- drivers/md/md.h | 7 ++++++- drivers/md/raid10.c | 2 +- drivers/md/raid5.c | 2 +- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index 3067ea05ba27..87e17ba86d93 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -671,6 +671,7 @@ void mddev_put(struct mddev *mddev) static void md_safemode_timeout(struct timer_list *t); static void md_start_sync(struct work_struct *ws); +static void md_io_opt_work(struct work_struct *ws); static void active_io_release(struct percpu_ref *ref) { @@ -794,6 +795,7 @@ int mddev_init(struct mddev *mddev) mddev->level = LEVEL_NONE; INIT_WORK(&mddev->sync_work, md_start_sync); + INIT_WORK(&mddev->io_opt_work, md_io_opt_work); INIT_WORK(&mddev->del_work, mddev_delayed_delete); return 0; @@ -6330,20 +6332,51 @@ int mddev_stack_rdev_into(struct mddev *mddev, struct md_rdev *rdev, EXPORT_SYMBOL_GPL(mddev_stack_rdev_into); /* update the optimal I/O size after a reshape */ -void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes) +static void md_io_opt_work(struct work_struct *ws) { + struct mddev *mddev = container_of(ws, struct mddev, io_opt_work); + struct request_queue *q = mddev->gendisk->queue; struct queue_limits lim; + /* + * Nothing is held here, so take q->limits_lock in the order the rest + * of md uses: before the suspend, see md_start_sync(). + */ + lim = queue_limits_start_update(q); + if (mddev_suspend(mddev, false) < 0) { + queue_limits_cancel_update(q); + return; + } + lim.io_opt = lim.io_min * READ_ONCE(mddev->io_opt_nr_stripes); + if (queue_limits_commit_update(q, &lim)) + pr_err("%s: could not apply queue limits\n", mdname(mddev)); + mddev_resume(mddev); +} + +void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes, + struct queue_limits *lim) +{ if (mddev_is_dm(mddev)) return; - /* don't bother updating io_opt if we can't suspend the array */ - if (mddev_suspend(mddev, false) < 0) + /* + * With an update owned by the caller just change it in place; it is + * committed, and the array resumed, by whoever started it. Taking + * q->limits_lock here would nest it inside reconfig_mutex and the + * suspend, which deadlocks, see md_start_sync(). + */ + if (lim) { + lim->io_opt = lim->io_min * nr_stripes; return; - lim = queue_limits_start_update(mddev->gendisk->queue); - lim.io_opt = lim.io_min * nr_stripes; - queue_limits_commit_update(mddev->gendisk->queue, &lim); - mddev_resume(mddev); + } + + /* + * Called from the sync thread, which md_reap_sync_thread() waits for + * with reconfig_mutex held, so q->limits_lock cannot be taken here + * either. Hand it to a work item that holds neither. + */ + WRITE_ONCE(mddev->io_opt_nr_stripes, nr_stripes); + queue_work(md_misc_wq, &mddev->io_opt_work); } EXPORT_SYMBOL_GPL(mddev_update_io_opt); @@ -7139,6 +7172,8 @@ static void __md_stop(struct mddev *mddev) { struct md_personality *pers = mddev->pers; + /* the deferred io_opt update suspends the array, so let it finish */ + flush_work(&mddev->io_opt_work); mddev_detach(mddev); md_bitmap_destroy(mddev); spin_lock(&mddev->lock); diff --git a/drivers/md/md.h b/drivers/md/md.h index ebdd57677062..1b2e8720f0d1 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h @@ -554,6 +554,10 @@ struct mddev { /* used for register new sync thread */ struct work_struct sync_work; + /* deferred io_opt update, see mddev_update_io_opt() */ + struct work_struct io_opt_work; + unsigned int io_opt_nr_stripes; + /* "lock" protects: * flush_bio transition from NULL to !NULL * rdev superblocks, events @@ -1056,7 +1060,8 @@ int mddev_stack_rdev_into(struct mddev *mddev, struct md_rdev *rdev, * is added with the array's current limits. */ #define MDDEV_STACK_SKIP ((struct queue_limits *)ERR_PTR(-EAGAIN)) -void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes); +void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes, + struct queue_limits *lim); extern const struct block_device_operations md_fops; diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 222bd7badcff..5580ca77ef1e 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -4932,7 +4932,7 @@ static void end_reshape(struct r10conf *conf) conf->reshape_safe = MaxSector; spin_unlock_irq(&conf->device_lock); - mddev_update_io_opt(conf->mddev, raid10_nr_stripes(conf)); + mddev_update_io_opt(conf->mddev, raid10_nr_stripes(conf), NULL); conf->fullsync = 0; } diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 0ec555ada64a..3faa2a94c03b 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -8800,7 +8800,7 @@ static void end_reshape(struct r5conf *conf) wake_up(&conf->wait_for_reshape); mddev_update_io_opt(conf->mddev, - conf->raid_disks - conf->max_degraded); + conf->raid_disks - conf->max_degraded, NULL); } } -- 2.43.0
