* [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O
@ 2026-09-10 8:11 Jack Wang
2026-09-10 8:11 ` [PATCH v2 1/8] md: pass a queue_limits down to ->hot_add_disk() Jack Wang
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
Writing to a queue limits attribute of an md array while a spare is
being re-added deadlocks the array. I reported this earlier here:
https://lore.kernel.org/linux-raid/CAMGffE=heGA3y8FjQ0Sm1jj-kd-=H9Y54WozKASSEZhc9UNKjA@mail.gmail.com/
Four tasks, one array:
udev-worker queue_attr_store() holds q->limits_lock, waits in
blk_mq_freeze_queue() for q_usage_counter to drain
fio holds a q_usage_counter reference, parked in
md_handle_request()'s is_suspended() loop
mdadm suspended the array, waits for reconfig_mutex
md_start_sync holds reconfig_mutex, waits for q->limits_lock
The last leg is mddev_stack_new_rdev() from ->hot_add_disk(). Since
commit c99f66e4084a ("block: fix queue freeze vs limits lock order in
sysfs store methods") the sysfs store holds q->limits_lock across the
freeze, so md must not block on that lock while it is holding back the
I/O the freeze waits for. That is the same hazard mddev_suspend()
already documents for reconfig_mutex.
v1 kept q->limits_lock outermost where it could and used a trylock
everywhere else. Christoph asked for the lock ordering to be fixed
instead, so v2 drops the trylock and the block patch that added it.
The rule v2 applies is that q->limits_lock nests outside reconfig_mutex
and the suspend everywhere, with no exceptions to paper over. The two
callers that cannot own an update do not need one: check_sb_changes()
and md_check_recovery() only ever re-add a device that is already a
member, so its limits are stacked and the update is a refresh, and they
now take the no-stack path unconditionally. mddev_update_io_opt() is
not an add and does need the update, so patch 4 defers it to a work
item that takes the lock with nothing held.
Making the hoists unconditional exposed a second cycle that the trylock
had been hiding, which Nilay hit with lockdep on v1:
q->limits_lock -> reconfig_mutex added by this series
reconfig_mutex -> disk->open_mutex pre-existing, md opens legs
under reconfig_mutex
disk->open_mutex -> q->limits_lock pre-existing, sd_open() ->
sd_revalidate_disk()
Only the middle edge can go, so legs are now opened before any md lock
is taken. Patch 7 does the opens and patch 8 the holder links, which
take disk->open_mutex too and were easy to miss because the release
side only takes blk_holder_mutex. Opening without reconfig_mutex means
the superblock format fields have to be snapshotted and rechecked once
the array is locked.
Patches 1, 3 and 6 are plumbing with no functional change.
Two callers still take q->limits_lock inside reconfig_mutex, both with
the array suspended, and patch 5 says why: ->start_reshape() from
action_store(), which suspends before flushing sync_work, and
raid*_run() -> queue_limits_set() from level_store(), which already
hangs on its own because it freezes the queue while suspended. Both
need more restructuring than belongs here.
The patches are based on v7.3-rc2.
v1: https://lore.kernel.org/linux-raid/20260909063029.GC29874@lst.de/T/#t
Changes since v1:
- Drop the trylock and the block patch adding
queue_limits_start_update_trylock(), per Christoph.
- check_sb_changes() and md_check_recovery() take the no-stack path
unconditionally instead of trying the lock first.
- Defer the io_opt update to a work item rather than skipping it on a
contended pass (patch 4, was "md: don't wait for q->limits_lock in
mddev_update_io_opt()").
- New patches 6-8 to close the reconfig_mutex -> disk->open_mutex
edge that lockdep reported on v1: pass a queue_limits through
->run(), open new legs before locking the array, and link their
holders there too.
- Rebased on v7.3-rc2.
Testing, on v7.3-rc2 with lockdep (PROVE_LOCKING, DEBUG_LOCK_ALLOC):
- the reproducer below, 20 fail/remove/add cycles with fio in flight
and a loop writing queue/max_sectors_kb, where the same test wedges
the array before the series
- array start at raid0, raid1, raid5, raid10 and linear, spare add,
fail and remove, the rdev sysfs stores, ADD_NEW_DISK, array_state
transitions, a level change and a raid5 3 -> 4 reshape to
completion, which is what exercises patch 4's work item: io_opt
goes from 1048576 to 1572864 once end_reshape() runs
- HOT_ADD_DISK of an undersized leg, so bind_rdev_to_array() fails
and patch 8's release path runs
No lockdep reports, and debug_locks stayed 1 throughout. Every patch
builds on its own.
A blktests case covering this deadlock will be sent separately.
The reproducer, for anyone who wants it:
mdadm -C /dev/md111 --force -e 1.2 --assume-clean -l 1 \
--bitmap=internal -n 2 /dev/ram0 /dev/ram1
fio --direct=1 --rw=randrw --ioengine=libaio --iodepth=32 --numjobs=4 \
--time_based=1 --runtime=180 --filename=/dev/md111 --name=repro &
while :; do
echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null
done &
for i in $(seq 20); do
mdadm /dev/md111 --fail /dev/ram0
mdadm /dev/md111 --remove /dev/ram0
mdadm /dev/md111 --add /dev/ram0
mdadm --wait /dev/md111
done
Jack Wang (8):
md: pass a queue_limits down to ->hot_add_disk()
md: don't wait for q->limits_lock in check_sb_changes()
md: pass a queue_limits through the rdev sysfs stores
md: defer the io_opt update out of the sync thread
md: take q->limits_lock before locking and suspending the array
md: pass a queue_limits through ->run()
md: open new legs before locking the array
md: link a new leg's holder before locking the array
drivers/md/dm-raid.c | 4 +-
drivers/md/md-autodetect.c | 38 ++-
drivers/md/md-linear.c | 30 +-
drivers/md/md.c | 661 +++++++++++++++++++++++++++++--------
drivers/md/md.h | 56 +++-
drivers/md/raid0.c | 16 +-
drivers/md/raid1.c | 26 +-
drivers/md/raid10.c | 37 ++-
drivers/md/raid5.c | 54 ++-
9 files changed, 741 insertions(+), 181 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/8] md: pass a queue_limits down to ->hot_add_disk()
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
2026-09-11 10:46 ` Nilay Shroff
2026-09-10 8:11 ` [PATCH v2 2/8] md: don't wait for q->limits_lock in check_sb_changes() Jack Wang
` (6 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
Adding a leg stacks its queue limits, which mddev_stack_new_rdev() does
by taking q->limits_lock itself. Callers holding reconfig_mutex or a
suspended array cannot allow that, and must own the update instead.
Give ->hot_add_disk(), remove_and_add_spares() and
md_choose_sync_action() a struct queue_limits argument with three
states: an update to stack into, NULL to let the personality take the
lock as before, or MDDEV_STACK_SKIP to add the leg without touching the
limits, for callers that can do neither. mddev_stack_rdev_into() stacks
into a caller-owned update without the lock.
Every caller still passes NULL and nothing passes the sentinel yet, so
there is no functional change; the users follow.
Assisted-by: LLM
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/md/dm-raid.c | 2 +-
drivers/md/md-linear.c | 28 ++++++++++++++----
drivers/md/md.c | 66 ++++++++++++++++++++++++++++++++----------
drivers/md/md.h | 11 ++++++-
drivers/md/raid1.c | 10 +++++--
drivers/md/raid10.c | 19 +++++++++---
drivers/md/raid5.c | 5 ++--
7 files changed, 110 insertions(+), 31 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 8f5a5e1342a9..21a1922bee4f 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3923,7 +3923,7 @@ static void attempt_restore_of_faulty_devices(struct raid_set *rs)
clear_bit(Faulty, &r->flags);
clear_bit(WriteErrorSeen, &r->flags);
- if (mddev->pers->hot_add_disk(mddev, r)) {
+ if (mddev->pers->hot_add_disk(mddev, r, NULL)) {
/* Failed to revive this device, try next */
r->raid_disk = r->saved_raid_disk = -1;
r->flags = flags;
diff --git a/drivers/md/md-linear.c b/drivers/md/md-linear.c
index 73b367b61b87..da82c313d459 100644
--- a/drivers/md/md-linear.c
+++ b/drivers/md/md-linear.c
@@ -65,11 +65,16 @@ static sector_t linear_size(struct mddev *mddev, sector_t sectors, int raid_disk
return array_sectors;
}
-static int linear_set_limits(struct mddev *mddev)
+static int linear_set_limits(struct mddev *mddev,
+ struct queue_limits *caller_lim)
{
struct queue_limits lim;
int err;
+ /* the caller can neither stack nor take q->limits_lock */
+ if (caller_lim == MDDEV_STACK_SKIP)
+ return 0;
+
md_init_stacking_limits(&lim);
lim.features |= BLK_FEAT_NOWAIT;
lim.max_hw_sectors = mddev->chunk_sectors;
@@ -82,10 +87,20 @@ static int linear_set_limits(struct mddev *mddev)
if (err)
return err;
+ /*
+ * The caller owns an update and commits it itself; taking
+ * q->limits_lock here would take it a second time.
+ */
+ if (caller_lim) {
+ *caller_lim = lim;
+ return 0;
+ }
+
return queue_limits_set(mddev->gendisk->queue, &lim);
}
-static struct linear_conf *linear_conf(struct mddev *mddev, int raid_disks)
+static struct linear_conf *linear_conf(struct mddev *mddev, int raid_disks,
+ struct queue_limits *lim)
{
struct linear_conf *conf;
struct md_rdev *rdev;
@@ -151,7 +166,7 @@ static struct linear_conf *linear_conf(struct mddev *mddev, int raid_disks)
conf->disks[i].rdev->sectors;
if (!mddev_is_dm(mddev)) {
- ret = linear_set_limits(mddev);
+ ret = linear_set_limits(mddev, lim);
if (ret)
goto out;
}
@@ -171,7 +186,7 @@ static int linear_run(struct mddev *mddev)
if (md_check_no_bitmap(mddev))
return -EINVAL;
- conf = linear_conf(mddev, mddev->raid_disks);
+ conf = linear_conf(mddev, mddev->raid_disks, NULL);
if (IS_ERR(conf))
return PTR_ERR(conf);
@@ -186,7 +201,8 @@ static int linear_run(struct mddev *mddev)
return ret;
}
-static int linear_add(struct mddev *mddev, struct md_rdev *rdev)
+static int linear_add(struct mddev *mddev, struct md_rdev *rdev,
+ struct queue_limits *lim)
{
/* Adding a drive to a linear array allows the array to grow.
* It is permitted if the new drive has a matching superblock
@@ -204,7 +220,7 @@ static int linear_add(struct mddev *mddev, struct md_rdev *rdev)
rdev->raid_disk = rdev->saved_raid_disk;
rdev->saved_raid_disk = -1;
- newconf = linear_conf(mddev, mddev->raid_disks + 1);
+ newconf = linear_conf(mddev, mddev->raid_disks + 1, lim);
if (IS_ERR(newconf))
return PTR_ERR(newconf);
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..28fc903ffeea 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -94,8 +94,8 @@ static DECLARE_WAIT_QUEUE_HEAD(resync_wait);
*/
static struct workqueue_struct *md_misc_wq;
-static int remove_and_add_spares(struct mddev *mddev,
- struct md_rdev *this);
+static int remove_and_add_spares(struct mddev *mddev, struct md_rdev *this,
+ struct queue_limits *lim);
static void mddev_detach(struct mddev *mddev);
static void export_rdev(struct md_rdev *rdev);
static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
@@ -2994,7 +2994,7 @@ static int add_bound_rdev(struct md_rdev *rdev)
*/
super_types[mddev->major_version].
validate_super(mddev, NULL/*freshest*/, rdev);
- err = mddev->pers->hot_add_disk(mddev, rdev);
+ err = mddev->pers->hot_add_disk(mddev, rdev, NULL);
if (err) {
md_kick_rdev_from_array(rdev);
return err;
@@ -3110,7 +3110,7 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len)
} else if (cmd_match(buf, "remove")) {
if (rdev->mddev->pers) {
clear_bit(Blocked, &rdev->flags);
- remove_and_add_spares(rdev->mddev, rdev);
+ remove_and_add_spares(rdev->mddev, rdev, NULL);
}
if (rdev->raid_disk >= 0)
err = -EBUSY;
@@ -3314,7 +3314,7 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len)
if (rdev->mddev->pers->hot_remove_disk == NULL)
return -EINVAL;
clear_bit(Blocked, &rdev->flags);
- remove_and_add_spares(rdev->mddev, rdev);
+ remove_and_add_spares(rdev->mddev, rdev, NULL);
if (rdev->raid_disk >= 0)
return -EBUSY;
set_bit(MD_RECOVERY_NEEDED, &rdev->mddev->recovery);
@@ -3344,7 +3344,8 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len)
rdev->saved_raid_disk = -1;
clear_bit(In_sync, &rdev->flags);
clear_bit(Bitmap_sync, &rdev->flags);
- err = rdev->mddev->pers->hot_add_disk(rdev->mddev, rdev);
+ err = rdev->mddev->pers->hot_add_disk(rdev->mddev, rdev,
+ NULL);
if (err) {
rdev->raid_disk = -1;
return err;
@@ -6275,6 +6276,40 @@ int mddev_stack_new_rdev(struct mddev *mddev, struct md_rdev *rdev)
}
EXPORT_SYMBOL_GPL(mddev_stack_new_rdev);
+/*
+ * Stack a new rdev into limits the caller already holds limits_lock for and
+ * will commit itself. Used from paths that must take limits_lock before
+ * quiescing the array, see md_start_sync().
+ */
+int mddev_stack_rdev_into(struct mddev *mddev, struct md_rdev *rdev,
+ struct queue_limits *lim)
+{
+ struct queue_limits tmp = *lim;
+
+ if (mddev_is_dm(mddev))
+ return 0;
+
+ if (queue_logical_block_size(rdev->bdev->bd_disk->queue) >
+ queue_logical_block_size(mddev->gendisk->queue)) {
+ pr_err("%s: incompatible logical_block_size, can not add\n",
+ mdname(mddev));
+ return -EINVAL;
+ }
+
+ queue_limits_stack_bdev(&tmp, rdev->bdev, rdev->data_offset,
+ mddev->gendisk->disk_name);
+
+ if (!queue_limits_stack_integrity_bdev(&tmp, rdev->bdev)) {
+ pr_err("%s: incompatible integrity profile for %pg\n",
+ mdname(mddev), rdev->bdev);
+ return -ENXIO;
+ }
+
+ *lim = tmp;
+ return 0;
+}
+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)
{
@@ -7706,7 +7741,7 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev)
goto kick_rdev;
clear_bit(Blocked, &rdev->flags);
- remove_and_add_spares(mddev, rdev);
+ remove_and_add_spares(mddev, rdev, NULL);
if (rdev->raid_disk >= 0)
goto busy;
@@ -10167,8 +10202,8 @@ static int remove_spares(struct mddev *mddev, struct md_rdev *this)
return removed;
}
-static int remove_and_add_spares(struct mddev *mddev,
- struct md_rdev *this)
+static int remove_and_add_spares(struct mddev *mddev, struct md_rdev *this,
+ struct queue_limits *lim)
{
struct md_rdev *rdev;
int spares = 0;
@@ -10191,7 +10226,7 @@ static int remove_and_add_spares(struct mddev *mddev,
continue;
if (!test_bit(Journal, &rdev->flags))
rdev->recovery_offset = 0;
- if (mddev->pers->hot_add_disk(mddev, rdev) == 0) {
+ if (mddev->pers->hot_add_disk(mddev, rdev, lim) == 0) {
/* failure here is OK */
sysfs_link_rdev(mddev, rdev);
if (!test_bit(Journal, &rdev->flags))
@@ -10206,7 +10241,8 @@ static int remove_and_add_spares(struct mddev *mddev,
return spares;
}
-static bool md_choose_sync_action(struct mddev *mddev, int *spares)
+static bool md_choose_sync_action(struct mddev *mddev, int *spares,
+ struct queue_limits *lim)
{
/* Check if reshape is in progress first. */
if (mddev->reshape_position != MaxSector) {
@@ -10234,7 +10270,7 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
* also removed and re-added, to allow the personality to fail the
* re-add.
*/
- *spares = remove_and_add_spares(mddev, NULL);
+ *spares = remove_and_add_spares(mddev, NULL, lim);
if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
@@ -10294,11 +10330,11 @@ static void md_start_sync(struct work_struct *ws)
* As we only add devices that are already in-sync, we can
* activate the spares immediately.
*/
- remove_and_add_spares(mddev, NULL);
+ remove_and_add_spares(mddev, NULL, NULL);
goto not_running;
}
- if (!md_choose_sync_action(mddev, &spares))
+ if (!md_choose_sync_action(mddev, &spares, NULL))
goto not_running;
if (!mddev->pers->sync_request)
@@ -10849,7 +10885,7 @@ static void check_sb_changes(struct mddev *mddev, struct md_rdev *rdev)
rdev2->saved_raid_disk = -1;
else
rdev2->saved_raid_disk = role;
- ret = remove_and_add_spares(mddev, rdev2);
+ ret = remove_and_add_spares(mddev, rdev2, NULL);
pr_info("Activated spare: %pg\n",
rdev2->bdev);
/* wakeup mddev->thread here, so array could
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b6d2e8929a0f..ebdd57677062 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -765,7 +765,8 @@ struct md_personality
* if appropriate, and should abort recovery if needed
*/
void (*error_handler)(struct mddev *mddev, struct md_rdev *rdev);
- int (*hot_add_disk) (struct mddev *mddev, struct md_rdev *rdev);
+ int (*hot_add_disk)(struct mddev *mddev, struct md_rdev *rdev,
+ struct queue_limits *lim);
int (*hot_remove_disk) (struct mddev *mddev, struct md_rdev *rdev);
int (*spare_active) (struct mddev *mddev);
sector_t (*sync_request)(struct mddev *mddev, sector_t sector_nr,
@@ -1047,6 +1048,14 @@ int do_md_run(struct mddev *mddev);
int mddev_stack_rdev_limits(struct mddev *mddev, struct queue_limits *lim,
unsigned int flags);
int mddev_stack_new_rdev(struct mddev *mddev, struct md_rdev *rdev);
+int mddev_stack_rdev_into(struct mddev *mddev, struct md_rdev *rdev,
+ struct queue_limits *lim);
+/*
+ * Sentinel for the queue_limits argument of ->hot_add_disk(). The caller has
+ * no update to stack into and must not take q->limits_lock itself, so the leg
+ * 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);
extern const struct block_device_operations md_fops;
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index f0646fb24371..78effcac138d 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1898,7 +1898,8 @@ static bool raid1_remove_conf(struct r1conf *conf, int disk)
return true;
}
-static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
+static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev,
+ struct queue_limits *lim)
{
struct r1conf *conf = mddev->private;
int err = -EEXIST;
@@ -1923,7 +1924,12 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
for (mirror = first; mirror <= last; mirror++) {
p = conf->mirrors + mirror;
if (!p->rdev) {
- err = mddev_stack_new_rdev(mddev, rdev);
+ if (lim == MDDEV_STACK_SKIP)
+ err = 0;
+ else if (lim)
+ err = mddev_stack_rdev_into(mddev, rdev, lim);
+ else
+ err = mddev_stack_new_rdev(mddev, rdev);
if (err)
return err;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 1093c798d9dd..222bd7badcff 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2095,7 +2095,8 @@ static int raid10_spare_active(struct mddev *mddev)
return count;
}
-static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
+static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev,
+ struct queue_limits *lim)
{
struct r10conf *conf = mddev->private;
int err = -EEXIST;
@@ -2130,7 +2131,12 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
continue;
}
- err = mddev_stack_new_rdev(mddev, rdev);
+ if (lim == MDDEV_STACK_SKIP)
+ err = 0;
+ else if (lim)
+ err = mddev_stack_rdev_into(mddev, rdev, lim);
+ else
+ err = mddev_stack_new_rdev(mddev, rdev);
if (err)
return err;
p->head_position = 0;
@@ -2147,7 +2153,12 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
clear_bit(In_sync, &rdev->flags);
set_bit(Replacement, &rdev->flags);
rdev->raid_disk = repl_slot;
- err = mddev_stack_new_rdev(mddev, rdev);
+ if (lim == MDDEV_STACK_SKIP)
+ err = 0;
+ else if (lim)
+ err = mddev_stack_rdev_into(mddev, rdev, lim);
+ else
+ err = mddev_stack_new_rdev(mddev, rdev);
if (err)
return err;
conf->fullsync = 1;
@@ -4484,7 +4495,7 @@ static int raid10_start_reshape(struct mddev *mddev)
rdev_for_each(rdev, mddev)
if (rdev->raid_disk < 0 &&
!test_bit(Faulty, &rdev->flags)) {
- if (raid10_add_disk(mddev, rdev) == 0) {
+ if (raid10_add_disk(mddev, rdev, NULL) == 0) {
if (rdev->raid_disk >=
conf->prev.raid_disks)
set_bit(In_sync, &rdev->flags);
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b91545ce090d..0ec555ada64a 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8441,7 +8441,8 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
return err;
}
-static int raid5_add_disk(struct mddev *mddev, struct md_rdev *rdev)
+static int raid5_add_disk(struct mddev *mddev, struct md_rdev *rdev,
+ struct queue_limits *lim)
{
struct r5conf *conf = mddev->private;
int ret, err = -EEXIST;
@@ -8728,7 +8729,7 @@ static int raid5_start_reshape(struct mddev *mddev)
rdev_for_each(rdev, mddev)
if (rdev->raid_disk < 0 &&
!test_bit(Faulty, &rdev->flags)) {
- if (raid5_add_disk(mddev, rdev) == 0) {
+ if (raid5_add_disk(mddev, rdev, NULL) == 0) {
if (rdev->raid_disk
>= conf->previous_raid_disks)
set_bit(In_sync, &rdev->flags);
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/8] md: don't wait for q->limits_lock in check_sb_changes()
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
2026-09-10 8:11 ` [PATCH v2 1/8] md: pass a queue_limits down to ->hot_add_disk() Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
2026-09-10 8:11 ` [PATCH v2 3/8] md: pass a queue_limits through the rdev sysfs stores Jack Wang
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
check_sb_changes() activates a spare another node added, reached from
md_reload_sb() -> process_metadata_update() with reconfig_mutex held.
Stacking the device's limits there waits for q->limits_lock under that
mutex, which deadlocks: the lock's holder waits for the queue to drain,
and that I/O can be waiting for a superblock update needing
reconfig_mutex.
The device is already a member, so its limits are stacked. Add it with
MDDEV_STACK_SKIP and leave them alone.
Assisted-by: LLM
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/md/md.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 28fc903ffeea..e60dc2c7eb90 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10885,7 +10885,14 @@ static void check_sb_changes(struct mddev *mddev, struct md_rdev *rdev)
rdev2->saved_raid_disk = -1;
else
rdev2->saved_raid_disk = role;
- ret = remove_and_add_spares(mddev, rdev2, NULL);
+ /*
+ * reconfig_mutex is held, so q->limits_lock
+ * cannot be taken here. The device is
+ * already a member, its limits are stacked,
+ * so add it without touching them.
+ */
+ ret = remove_and_add_spares(mddev, rdev2,
+ MDDEV_STACK_SKIP);
pr_info("Activated spare: %pg\n",
rdev2->bdev);
/* wakeup mddev->thread here, so array could
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/8] md: pass a queue_limits through the rdev sysfs stores
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
2026-09-10 8:11 ` [PATCH v2 1/8] md: pass a queue_limits down to ->hot_add_disk() Jack Wang
2026-09-10 8:11 ` [PATCH v2 2/8] md: don't wait for q->limits_lock in check_sb_changes() Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
2026-09-10 8:11 ` [PATCH v2 4/8] md: defer the io_opt update out of the sync thread Jack Wang
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
state_store() and slot_store() can add a leg back to the array, which
stacks its limits, and q->limits_lock has to be taken before the array
is locked and suspended.
Give the rdev sysfs store callback a struct queue_limits argument.
rdev_attr_store() passes NULL, so there is no functional change; the
user follows.
Assisted-by: LLM
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/md/md.c | 45 ++++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index e60dc2c7eb90..3067ea05ba27 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3033,7 +3033,13 @@ static int cmd_match(const char *cmd, const char *str)
struct rdev_sysfs_entry {
struct attribute attr;
ssize_t (*show)(struct md_rdev *, char *);
- ssize_t (*store)(struct md_rdev *, const char *, size_t);
+ /*
+ * @lim: a queue limits update the caller owns, or NULL. Stores that
+ * can add a leg to the array must stack into it rather than take
+ * q->limits_lock themselves, see md_start_sync().
+ */
+ ssize_t (*store)(struct md_rdev *rdev, const char *page, size_t len,
+ struct queue_limits *lim);
};
static ssize_t
@@ -3079,7 +3085,8 @@ state_show(struct md_rdev *rdev, char *page)
}
static ssize_t
-state_store(struct md_rdev *rdev, const char *buf, size_t len)
+state_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
/* can write
* faulty - simulates an error
@@ -3257,7 +3264,8 @@ errors_show(struct md_rdev *rdev, char *page)
}
static ssize_t
-errors_store(struct md_rdev *rdev, const char *buf, size_t len)
+errors_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
unsigned int n;
int rv;
@@ -3283,7 +3291,8 @@ slot_show(struct md_rdev *rdev, char *page)
}
static ssize_t
-slot_store(struct md_rdev *rdev, const char *buf, size_t len)
+slot_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
int slot;
int err;
@@ -3378,7 +3387,8 @@ offset_show(struct md_rdev *rdev, char *page)
}
static ssize_t
-offset_store(struct md_rdev *rdev, const char *buf, size_t len)
+offset_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
unsigned long long offset;
if (kstrtoull(buf, 10, &offset) < 0)
@@ -3404,7 +3414,8 @@ static ssize_t new_offset_show(struct md_rdev *rdev, char *page)
}
static ssize_t new_offset_store(struct md_rdev *rdev,
- const char *buf, size_t len)
+ const char *buf, size_t len,
+ struct queue_limits *lim)
{
unsigned long long new_offset;
struct mddev *mddev = rdev->mddev;
@@ -3511,7 +3522,8 @@ static int strict_blocks_to_sectors(const char *buf, sector_t *sectors)
}
static ssize_t
-rdev_size_store(struct md_rdev *rdev, const char *buf, size_t len)
+rdev_size_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
struct mddev *my_mddev = rdev->mddev;
sector_t oldsectors = rdev->sectors;
@@ -3573,7 +3585,8 @@ static ssize_t recovery_start_show(struct md_rdev *rdev, char *page)
return sprintf(page, "%llu\n", recovery_start);
}
-static ssize_t recovery_start_store(struct md_rdev *rdev, const char *buf, size_t len)
+static ssize_t recovery_start_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
unsigned long long recovery_start;
@@ -3612,7 +3625,9 @@ static ssize_t bb_show(struct md_rdev *rdev, char *page)
{
return badblocks_show(&rdev->badblocks, page, 0);
}
-static ssize_t bb_store(struct md_rdev *rdev, const char *page, size_t len)
+
+static ssize_t bb_store(struct md_rdev *rdev, const char *page, size_t len,
+ struct queue_limits *lim)
{
int rv = badblocks_store(&rdev->badblocks, page, len, 0);
/* Maybe that ack was all we needed */
@@ -3627,7 +3642,9 @@ static ssize_t ubb_show(struct md_rdev *rdev, char *page)
{
return badblocks_show(&rdev->badblocks, page, 1);
}
-static ssize_t ubb_store(struct md_rdev *rdev, const char *page, size_t len)
+
+static ssize_t ubb_store(struct md_rdev *rdev, const char *page, size_t len,
+ struct queue_limits *lim)
{
return badblocks_store(&rdev->badblocks, page, len, 1);
}
@@ -3641,7 +3658,8 @@ ppl_sector_show(struct md_rdev *rdev, char *page)
}
static ssize_t
-ppl_sector_store(struct md_rdev *rdev, const char *buf, size_t len)
+ppl_sector_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
unsigned long long sector;
@@ -3680,7 +3698,8 @@ ppl_size_show(struct md_rdev *rdev, char *page)
}
static ssize_t
-ppl_size_store(struct md_rdev *rdev, const char *buf, size_t len)
+ppl_size_store(struct md_rdev *rdev, const char *buf, size_t len,
+ struct queue_limits *lim)
{
unsigned int size;
@@ -3766,7 +3785,7 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
if (rdev->mddev == NULL)
rv = -ENODEV;
else
- rv = entry->store(rdev, page, length);
+ rv = entry->store(rdev, page, length, NULL);
suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/8] md: defer the io_opt update out of the sync thread
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
` (2 preceding siblings ...)
2026-09-10 8:11 ` [PATCH v2 3/8] md: pass a queue_limits through the rdev sysfs stores Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
2026-09-10 8:11 ` [PATCH v2 5/8] md: take q->limits_lock before locking and suspending the array Jack Wang
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
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 <jinpu.wang@cloud.ionos.com>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/8] md: take q->limits_lock before locking and suspending the array
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
` (3 preceding siblings ...)
2026-09-10 8:11 ` [PATCH v2 4/8] md: defer the io_opt update out of the sync thread Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
2026-09-10 8:11 ` [PATCH v2 6/8] md: pass a queue_limits through ->run() Jack Wang
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
Writing a queue limits attribute while a spare is re-added deadlocks the
array:
udev-worker queue_attr_store() holds q->limits_lock, waits in
blk_mq_freeze_queue() for q_usage_counter to drain
fio holds a q_usage_counter reference, parked in
md_handle_request()'s is_suspended() loop
mdadm suspended the array, waits for reconfig_mutex
md_start_sync holds reconfig_mutex, waits for q->limits_lock
Blocking on q->limits_lock while holding reconfig_mutex, or with the
array suspended, is waiting for normal I/O, which mddev_suspend()
already warns about with lockdep_assert_not_held(). So q->limits_lock
has to nest outside both.
Take the update before the array is locked and suspended, and pass it
down so the personality stacks into it:
- md_start_sync(), at both suspend points
- md_ioctl() for ADD_NEW_DISK and HOT_REMOVE_DISK
- rdev_attr_store(), for slot and for state "remove"/"re-add"
- raid5 skip_copy_store(), which took the lock while suspended
They are converted together because a mix of the two orders is an ABBA.
All of them commit while the array is still quiesced.
Two callers still take the lock inside reconfig_mutex with the array
suspended: ->start_reshape() from action_store(), which suspends before
flushing sync_work so the update cannot be held across it, and
raid*_run() from level_store(), which a later patch converts.
Verified with a raid1 of two ram devices, fio in flight and a loop
writing queue/max_sectors_kb: 20 fail/remove/add cycles complete, where
the same test wedges the array before the change.
Fixes: c99f66e4084a ("block: fix queue freeze vs limits lock order in sysfs store methods")
Assisted-by: LLM
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/md/md-autodetect.c | 2 +-
drivers/md/md.c | 120 +++++++++++++++++++++++++++++++------
drivers/md/md.h | 3 +-
drivers/md/raid5.c | 31 +++++++---
4 files changed, 128 insertions(+), 28 deletions(-)
diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
index 4b80165afd23..929513109657 100644
--- a/drivers/md/md-autodetect.c
+++ b/drivers/md/md-autodetect.c
@@ -213,7 +213,7 @@ static void __init md_setup_drive(struct md_setup_args *args)
(1 << MD_DISK_ACTIVE) | (1 << MD_DISK_SYNC);
}
- md_add_new_disk(mddev, &dinfo);
+ md_add_new_disk(mddev, &dinfo, NULL);
}
if (!err)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 87e17ba86d93..0668a048db71 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2983,7 +2983,7 @@ void md_update_sb(struct mddev *mddev, int force_change)
}
EXPORT_SYMBOL(md_update_sb);
-static int add_bound_rdev(struct md_rdev *rdev)
+static int add_bound_rdev(struct md_rdev *rdev, struct queue_limits *lim)
{
struct mddev *mddev = rdev->mddev;
int err = 0;
@@ -2996,7 +2996,7 @@ static int add_bound_rdev(struct md_rdev *rdev)
*/
super_types[mddev->major_version].
validate_super(mddev, NULL/*freshest*/, rdev);
- err = mddev->pers->hot_add_disk(mddev, rdev, NULL);
+ err = mddev->pers->hot_add_disk(mddev, rdev, lim);
if (err) {
md_kick_rdev_from_array(rdev);
return err;
@@ -3119,7 +3119,7 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len,
} else if (cmd_match(buf, "remove")) {
if (rdev->mddev->pers) {
clear_bit(Blocked, &rdev->flags);
- remove_and_add_spares(rdev->mddev, rdev, NULL);
+ remove_and_add_spares(rdev->mddev, rdev, lim);
}
if (rdev->raid_disk >= 0)
err = -EBUSY;
@@ -3238,7 +3238,7 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len,
if (!mddev_is_clustered(rdev->mddev) ||
(err = mddev->cluster_ops->gather_bitmaps(rdev)) == 0) {
clear_bit(Faulty, &rdev->flags);
- err = add_bound_rdev(rdev);
+ err = add_bound_rdev(rdev, lim);
}
} else
err = -EBUSY;
@@ -3325,7 +3325,7 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len,
if (rdev->mddev->pers->hot_remove_disk == NULL)
return -EINVAL;
clear_bit(Blocked, &rdev->flags);
- remove_and_add_spares(rdev->mddev, rdev, NULL);
+ remove_and_add_spares(rdev->mddev, rdev, lim);
if (rdev->raid_disk >= 0)
return -EBUSY;
set_bit(MD_RECOVERY_NEEDED, &rdev->mddev->recovery);
@@ -3356,7 +3356,7 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len,
clear_bit(In_sync, &rdev->flags);
clear_bit(Bitmap_sync, &rdev->flags);
err = rdev->mddev->pers->hot_add_disk(rdev->mddev, rdev,
- NULL);
+ lim);
if (err) {
rdev->raid_disk = -1;
return err;
@@ -3762,6 +3762,9 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
struct kernfs_node *kn = NULL;
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
bool suspend = false;
ssize_t rv;
struct mddev *mddev = READ_ONCE(rdev->mddev);
@@ -3782,15 +3785,41 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
suspend = true;
}
+ /*
+ * These can add a leg back, which stacks its limits; the other
+ * state_store() values never reach ->hot_add_disk(). q->limits_lock
+ * nests outside the lock and the suspend, see md_start_sync().
+ */
+ if ((entry->store == slot_store ||
+ (entry->store == state_store &&
+ (cmd_match(page, "remove") || cmd_match(page, "re-add")))) &&
+ !mddev_is_dm(mddev)) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
+
rv = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
if (!rv) {
if (rdev->mddev == NULL)
rv = -ENODEV;
else
- rv = entry->store(rdev, page, length, NULL);
+ rv = entry->store(rdev, page, length, limp);
+ /* apply the limits before the array takes I/O again */
+ if (limp) {
+ int err = queue_limits_commit_update(q, limp);
+
+ limp = NULL;
+ if (err && rv >= 0)
+ rv = err;
+ }
suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
}
+ /* only reached when the lock failed, so nothing was stacked */
+ if (limp)
+ queue_limits_cancel_update(q);
+
if (kn)
sysfs_unbreak_active_protection(kn);
@@ -7575,7 +7604,8 @@ static int get_disk_info(struct mddev *mddev, void __user * arg)
return 0;
}
-int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
+int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
+ struct queue_limits *lim)
{
struct md_rdev *rdev;
dev_t dev = MKDEV(info->major,info->minor);
@@ -7723,11 +7753,11 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
if (err)
mddev->cluster_ops->add_new_disk_cancel(mddev);
else
- err = add_bound_rdev(rdev);
+ err = add_bound_rdev(rdev, lim);
}
} else if (!err)
- err = add_bound_rdev(rdev);
+ err = add_bound_rdev(rdev, lim);
return err;
}
@@ -7780,7 +7810,8 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
return 0;
}
-static int hot_remove_disk(struct mddev *mddev, dev_t dev)
+static int hot_remove_disk(struct mddev *mddev, dev_t dev,
+ struct queue_limits *lim)
{
struct md_rdev *rdev;
@@ -7795,7 +7826,7 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev)
goto kick_rdev;
clear_bit(Blocked, &rdev->flags);
- remove_and_add_spares(mddev, rdev, NULL);
+ remove_and_add_spares(mddev, rdev, lim);
if (rdev->raid_disk >= 0)
goto busy;
@@ -8380,6 +8411,22 @@ static inline int md_ioctl_valid(unsigned int cmd)
}
}
+/*
+ * Commands that can reach ->hot_add_disk(). ADD_NEW_DISK only does so for a
+ * journal device or a personality without ->hot_remove_disk, but that depends
+ * on disk info still in user memory here, so it is included as a whole.
+ */
+static bool md_ioctl_may_add_disk(unsigned int cmd)
+{
+ switch (cmd) {
+ case ADD_NEW_DISK:
+ case HOT_REMOVE_DISK:
+ return true;
+ default:
+ return false;
+ }
+}
+
static bool md_ioctl_need_suspend(unsigned int cmd)
{
switch (cmd) {
@@ -8435,6 +8482,9 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
unsigned int noio_flags = 0;
void __user *argp = (void __user *)arg;
struct mddev *mddev = NULL;
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
bool suspend;
err = md_ioctl_valid(cmd);
@@ -8485,11 +8535,20 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
if (!md_is_rdwr(mddev))
flush_work(&mddev->sync_work);
+ /* q->limits_lock nests outside both, see md_start_sync() */
+ if (md_ioctl_may_add_disk(cmd) && !mddev_is_dm(mddev)) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
+
suspend = md_ioctl_need_suspend(cmd);
err = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
if (err) {
pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
err, cmd);
+ if (limp)
+ queue_limits_cancel_update(q);
goto out;
}
if (suspend)
@@ -8531,7 +8590,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
goto unlock;
case HOT_REMOVE_DISK:
- err = hot_remove_disk(mddev, new_decode_dev(arg));
+ err = hot_remove_disk(mddev, new_decode_dev(arg), limp);
goto unlock;
case ADD_NEW_DISK:
@@ -8547,7 +8606,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
/* Need to clear read-only for this */
break;
else
- err = md_add_new_disk(mddev, &info);
+ err = md_add_new_disk(mddev, &info, limp);
goto unlock;
}
break;
@@ -8585,7 +8644,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
if (copy_from_user(&info, argp, sizeof(info)))
err = -EFAULT;
else
- err = md_add_new_disk(mddev, &info);
+ err = md_add_new_disk(mddev, &info, limp);
goto unlock;
}
@@ -8618,6 +8677,9 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
err != -EINVAL)
mddev->hold_active = 0;
+ if (limp)
+ err = queue_limits_commit_update(q, limp) ?: err;
+
if (suspend) {
memalloc_noio_restore(noio_flags);
mddev_unlock_and_resume(mddev);
@@ -10346,6 +10408,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares,
static void md_start_sync(struct work_struct *ws)
{
struct mddev *mddev = container_of(ws, struct mddev, sync_work);
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
int spares = 0;
bool suspend = false;
unsigned int noio_flags = 0;
@@ -10357,6 +10422,17 @@ static void md_start_sync(struct work_struct *ws)
*/
if ((mddev->reshape_position == MaxSector || !md_is_rdwr(mddev)) &&
md_spares_need_change(mddev)) {
+ /*
+ * Adding a spare below stacks its limits, which needs
+ * q->limits_lock. Take it before suspending: its holder
+ * waits in blk_mq_freeze_queue() for I/O that
+ * mddev->suspended holds back, so the other order deadlocks.
+ */
+ if (!mddev_is_dm(mddev)) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
suspend = true;
mddev_suspend(mddev, false);
noio_flags = memalloc_noio_save();
@@ -10371,6 +10447,12 @@ static void md_start_sync(struct work_struct *ws)
if (!suspend && (mddev->reshape_position == MaxSector || !md_is_rdwr(mddev)) &&
md_spares_need_change(mddev)) {
mddev_unlock(mddev);
+ /* see above: q->limits_lock nests outside both */
+ if (!mddev_is_dm(mddev)) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
mddev_suspend_and_lock_nointr(mddev);
suspend = true;
noio_flags = memalloc_noio_save();
@@ -10384,11 +10466,11 @@ static void md_start_sync(struct work_struct *ws)
* As we only add devices that are already in-sync, we can
* activate the spares immediately.
*/
- remove_and_add_spares(mddev, NULL, NULL);
+ remove_and_add_spares(mddev, NULL, limp);
goto not_running;
}
- if (!md_choose_sync_action(mddev, &spares, NULL))
+ if (!md_choose_sync_action(mddev, &spares, limp))
goto not_running;
if (!mddev->pers->sync_request)
@@ -10419,6 +10501,8 @@ static void md_start_sync(struct work_struct *ws)
* https://bugzilla.kernel.org/show_bug.cgi?id=218200
* Therefore, use __mddev_resume(mddev, false).
*/
+ if (limp && queue_limits_commit_update(q, limp))
+ pr_err("%s: could not apply queue limits\n", mdname(mddev));
if (suspend) {
memalloc_noio_restore(noio_flags);
__mddev_resume(mddev, false);
@@ -10441,6 +10525,8 @@ static void md_start_sync(struct work_struct *ws)
* https://bugzilla.kernel.org/show_bug.cgi?id=218200
* Therefore, use __mddev_resume(mddev, false).
*/
+ if (limp && queue_limits_commit_update(q, limp))
+ pr_err("%s: could not apply queue limits\n", mdname(mddev));
if (suspend) {
memalloc_noio_restore(noio_flags);
__mddev_resume(mddev, false);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 1b2e8720f0d1..7f4e3ea8b826 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -1046,7 +1046,8 @@ struct mdu_disk_info_s;
extern int mdp_major;
void md_autostart_arrays(int part);
int md_set_array_info(struct mddev *mddev, struct mdu_array_info_s *info);
-int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info);
+int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
+ struct queue_limits *lim);
int do_md_run(struct mddev *mddev);
#define MDDEV_STACK_INTEGRITY (1u << 0)
int mddev_stack_rdev_limits(struct mddev *mddev, struct queue_limits *lim,
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 3faa2a94c03b..22759c631c4d 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7288,6 +7288,9 @@ static ssize_t
raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
{
struct r5conf *conf;
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
unsigned long new;
int err;
@@ -7297,23 +7300,33 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
return -EINVAL;
new = !!new;
+ /* q->limits_lock nests outside both, see md_start_sync() */
+ if (!mddev_is_dm(mddev)) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
+
err = mddev_suspend_and_lock(mddev);
- if (err)
+ if (err) {
+ if (limp)
+ queue_limits_cancel_update(q);
return err;
+ }
conf = mddev->private;
if (!conf)
err = -ENODEV;
else if (new != conf->skip_copy) {
- struct request_queue *q = mddev->gendisk->queue;
- struct queue_limits lim = queue_limits_start_update(q);
-
conf->skip_copy = new;
- if (new)
- lim.features |= BLK_FEAT_STABLE_WRITES;
- else
- lim.features &= ~BLK_FEAT_STABLE_WRITES;
- err = queue_limits_commit_update(q, &lim);
+ if (limp) {
+ if (new)
+ limp->features |= BLK_FEAT_STABLE_WRITES;
+ else
+ limp->features &= ~BLK_FEAT_STABLE_WRITES;
+ }
}
+ if (limp)
+ err = queue_limits_commit_update(q, limp) ?: err;
mddev_unlock_and_resume(mddev);
return err ?: len;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 6/8] md: pass a queue_limits through ->run()
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
` (4 preceding siblings ...)
2026-09-10 8:11 ` [PATCH v2 5/8] md: take q->limits_lock before locking and suspending the array Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
2026-09-11 10:54 ` Nilay Shroff
2026-09-10 8:11 ` [PATCH v2 7/8] md: open new legs before locking the array Jack Wang
2026-09-10 8:11 ` [PATCH v2 8/8] md: link a new leg's holder " Jack Wang
7 siblings, 1 reply; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
raid*_run() -> queue_limits_set() takes q->limits_lock with
reconfig_mutex held, the order the previous patches inverted. With
lockdep on, creating an array and then adding a leg reports it:
-> #1 (&q->limits_lock): -> #0 (&mddev->reconfig_mutex):
queue_limits_set md_ioctl <- ADD_NEW_DISK
raid1_run
do_md_run
md_ioctl <- RUN_ARRAY
The earlier patch left this for level_store() alone; RUN_ARRAY reaches
it too, so every array creation records the wrong order.
Give ->run() a queue_limits argument and take the update at the entry
points that start an array: md_ioctl() for RUN_ARRAY, level_store(),
autorun_devices(), md_setup_drive(), and array_state_store() for
readonly, read_auto and active -- but only while mddev->pers is NULL,
as with the array running those states go to md_set_readonly(), which
waits in stop_sync_thread() for the work that takes the same lock.
dm-raid passes NULL: with no gendisk the personalities return before
touching any limits.
Assisted-by: LLM
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/md/dm-raid.c | 2 +-
drivers/md/md-autodetect.c | 21 ++++++-
drivers/md/md-linear.c | 4 +-
drivers/md/md.c | 116 +++++++++++++++++++++++++++++++------
drivers/md/md.h | 11 +++-
drivers/md/raid0.c | 16 ++++-
drivers/md/raid1.c | 16 ++++-
drivers/md/raid10.c | 16 ++++-
drivers/md/raid5.c | 16 ++++-
9 files changed, 182 insertions(+), 36 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 21a1922bee4f..d043a5c49608 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3258,7 +3258,7 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
/* Keep array frozen until resume. */
md_frozen_sync_thread(&rs->md);
- r = md_run(&rs->md);
+ r = md_run(&rs->md, NULL);
rs->md.in_sync = 0; /* Assume already marked dirty */
if (r) {
ti->error = "Failed to run raid array";
diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
index 929513109657..e15ae2fb58a2 100644
--- a/drivers/md/md-autodetect.c
+++ b/drivers/md/md-autodetect.c
@@ -126,6 +126,9 @@ static void __init md_setup_drive(struct md_setup_args *args)
dev_t devices[MD_SB_DISKS + 1], mdev;
struct mdu_array_info_s ainfo = { };
struct mddev *mddev;
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
int err = 0, i;
char name[16];
@@ -216,11 +219,27 @@ static void __init md_setup_drive(struct md_setup_args *args)
md_add_new_disk(mddev, &dinfo, NULL);
}
+ /*
+ * do_md_run() restacks the array's limits, and q->limits_lock must
+ * not nest inside reconfig_mutex, so start the update with the array
+ * unlocked. This is __init and the array is not reachable yet.
+ */
+ if (!err && !mddev_is_dm(mddev)) {
+ mddev_unlock(mddev);
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ mddev_lock_nointr(mddev);
+ }
+
if (!err)
- err = do_md_run(mddev);
+ err = do_md_run(mddev, limp);
if (err)
pr_warn("md: starting %s failed\n", name);
out_unlock:
+ /* apply the limits before the array takes I/O */
+ if (limp)
+ queue_limits_commit_update(q, limp);
mddev_unlock_and_resume(mddev);
out_mddev_put:
mddev_put(mddev);
diff --git a/drivers/md/md-linear.c b/drivers/md/md-linear.c
index da82c313d459..5438c23a7242 100644
--- a/drivers/md/md-linear.c
+++ b/drivers/md/md-linear.c
@@ -178,7 +178,7 @@ static struct linear_conf *linear_conf(struct mddev *mddev, int raid_disks,
return ERR_PTR(ret);
}
-static int linear_run(struct mddev *mddev)
+static int linear_run(struct mddev *mddev, struct queue_limits *lim)
{
struct linear_conf *conf;
int ret;
@@ -186,7 +186,7 @@ static int linear_run(struct mddev *mddev)
if (md_check_no_bitmap(mddev))
return -EINVAL;
- conf = linear_conf(mddev, mddev->raid_disks, NULL);
+ conf = linear_conf(mddev, mddev->raid_disks, lim);
if (IS_ERR(conf))
return PTR_ERR(conf);
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0668a048db71..5be956e80563 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4105,13 +4105,30 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
long level;
void *priv, *oldpriv;
struct md_rdev *rdev;
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
if (slen == 0 || slen >= sizeof(clevel))
return -EINVAL;
+ /*
+ * The new personality restacks the array's queue limits in ->run(),
+ * and q->limits_lock has to be taken before the array is locked and
+ * suspended, see md_start_sync().
+ */
+ if (!mddev_is_dm(mddev)) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
+
rv = mddev_suspend_and_lock(mddev);
- if (rv)
+ if (rv) {
+ if (limp)
+ queue_limits_cancel_update(q);
return rv;
+ }
noio_flags = memalloc_noio_save();
if (mddev->pers == NULL) {
@@ -4280,7 +4297,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
mddev->in_sync = 1;
timer_delete_sync(&mddev->safemode_timer);
}
- pers->run(mddev);
+ pers->run(mddev, limp);
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
if (!mddev->thread)
md_update_sb(mddev, 1);
@@ -4288,6 +4305,9 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
md_new_event();
rv = len;
out_unlock:
+ /* apply the limits before the array takes I/O again */
+ if (limp)
+ rv = queue_limits_commit_update(q, limp) ?: rv;
memalloc_noio_restore(noio_flags);
mddev_unlock_and_resume(mddev);
return rv;
@@ -4708,6 +4728,10 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
{
int err = 0;
enum array_state st = match_word(buf, array_states);
+ bool starts_array, need_lim;
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
/* No lock dependent actions */
switch (st) {
@@ -4753,9 +4777,39 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
spin_unlock(&mddev->lock);
return err ?: len;
}
+
+ /*
+ * These states start the array when it is not running, and ->run()
+ * restacks its limits, so take q->limits_lock first. Only then:
+ * with mddev->pers set they go to md_set_readonly(), which waits for
+ * the very work that takes the same lock.
+ */
+ starts_array = (st == readonly || st == read_auto || st == active) &&
+ !mddev_is_dm(mddev);
+retry:
+ need_lim = starts_array && !READ_ONCE(mddev->pers);
+ if (need_lim) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
+
err = mddev_lock(mddev);
- if (err)
+ if (err) {
+ if (limp)
+ queue_limits_cancel_update(q);
return err;
+ }
+
+ /* mddev->pers was read without the lock, so redo it if it changed */
+ if (need_lim != (starts_array && !mddev->pers)) {
+ mddev_unlock(mddev);
+ if (limp) {
+ queue_limits_cancel_update(q);
+ limp = NULL;
+ }
+ goto retry;
+ }
switch (st) {
case inactive:
@@ -4772,7 +4826,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
else {
mddev->ro = MD_RDONLY;
set_disk_ro(mddev->gendisk, 1);
- err = do_md_run(mddev);
+ err = do_md_run(mddev, limp);
}
break;
case read_auto:
@@ -4787,7 +4841,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
}
} else {
mddev->ro = MD_AUTO_READ;
- err = do_md_run(mddev);
+ err = do_md_run(mddev, limp);
}
break;
case clean:
@@ -4813,7 +4867,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
} else {
mddev->ro = MD_RDWR;
set_disk_ro(mddev->gendisk, 0);
- err = do_md_run(mddev);
+ err = do_md_run(mddev, limp);
}
break;
default:
@@ -4826,6 +4880,9 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
mddev->hold_active = 0;
sysfs_notify_dirent_safe(mddev->sysfs_state);
}
+ /* apply the limits before the array takes I/O */
+ if (limp)
+ err = queue_limits_commit_update(q, limp) ?: err;
mddev_unlock(mddev);
if (st == readonly || st == read_auto || st == inactive ||
@@ -6763,7 +6820,7 @@ static void md_bitmap_set_none(struct mddev *mddev)
md_bitmap_sysfs_add(mddev);
}
-int md_run(struct mddev *mddev)
+int md_run(struct mddev *mddev, struct queue_limits *lim)
{
int err;
struct md_rdev *rdev;
@@ -6892,7 +6949,7 @@ int md_run(struct mddev *mddev)
if (start_readonly && md_is_rdwr(mddev))
mddev->ro = MD_AUTO_READ; /* read-only, but switch on first write */
- err = pers->run(mddev);
+ err = pers->run(mddev, lim);
if (err)
pr_warn("md: pers->run() failed ...\n");
else if (pers->size(mddev, 0, 0) < mddev->array_sectors) {
@@ -6988,12 +7045,12 @@ int md_run(struct mddev *mddev)
}
EXPORT_SYMBOL_GPL(md_run);
-int do_md_run(struct mddev *mddev)
+int do_md_run(struct mddev *mddev, struct queue_limits *lim)
{
int err;
set_bit(MD_NOT_READY, &mddev->flags);
- err = md_run(mddev);
+ err = md_run(mddev, lim);
if (err)
goto out;
@@ -7349,7 +7406,7 @@ static int do_md_stop(struct mddev *mddev, int mode)
}
#ifndef MODULE
-static void autorun_array(struct mddev *mddev)
+static void autorun_array(struct mddev *mddev, struct queue_limits *lim)
{
struct md_rdev *rdev;
int err;
@@ -7364,7 +7421,7 @@ static void autorun_array(struct mddev *mddev)
}
pr_cont("\n");
- err = do_md_run(mddev);
+ err = do_md_run(mddev, lim);
if (err) {
pr_warn("md: do_md_run() returned %d\n", err);
do_md_stop(mddev, 0);
@@ -7387,6 +7444,9 @@ static void autorun_devices(int part)
{
struct md_rdev *rdev0, *rdev, *tmp;
struct mddev *mddev;
+ struct request_queue *q = NULL;
+ struct queue_limits lim;
+ struct queue_limits *limp = NULL;
pr_info("md: autorun ...\n");
while (!list_empty(&pending_raid_disks)) {
@@ -7427,12 +7487,29 @@ static void autorun_devices(int part)
if (IS_ERR(mddev))
break;
- if (mddev_suspend_and_lock(mddev))
+ /*
+ * autorun_array() runs the array, which restacks its limits;
+ * q->limits_lock has to be taken before the array is locked
+ * and suspended, see md_start_sync().
+ */
+ if (!mddev_is_dm(mddev)) {
+ q = mddev->gendisk->queue;
+ lim = queue_limits_start_update(q);
+ limp = &lim;
+ }
+
+ if (mddev_suspend_and_lock(mddev)) {
pr_warn("md: %s locked, cannot run\n", mdname(mddev));
- else if (mddev->raid_disks || mddev->major_version
+ if (limp) {
+ queue_limits_cancel_update(q);
+ limp = NULL;
+ }
+ } else if (mddev->raid_disks || mddev->major_version
|| !list_empty(&mddev->disks)) {
pr_warn("md: %s already running, cannot run %pg\n",
mdname(mddev), rdev0->bdev);
+ if (limp)
+ queue_limits_cancel_update(q);
mddev_unlock_and_resume(mddev);
} else {
pr_debug("md: created %s\n", mdname(mddev));
@@ -7442,9 +7519,13 @@ static void autorun_devices(int part)
if (bind_rdev_to_array(rdev, mddev))
export_rdev(rdev);
}
- autorun_array(mddev);
+ autorun_array(mddev, limp);
+ if (limp && queue_limits_commit_update(q, limp))
+ pr_warn("md: %s: could not apply queue limits\n",
+ mdname(mddev));
mddev_unlock_and_resume(mddev);
}
+ limp = NULL;
/* on success, candidates will be empty, on error
* it won't...
*/
@@ -8536,7 +8617,8 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
flush_work(&mddev->sync_work);
/* q->limits_lock nests outside both, see md_start_sync() */
- if (md_ioctl_may_add_disk(cmd) && !mddev_is_dm(mddev)) {
+ if ((md_ioctl_may_add_disk(cmd) || cmd == RUN_ARRAY) &&
+ !mddev_is_dm(mddev)) {
q = mddev->gendisk->queue;
lim = queue_limits_start_update(q);
limp = &lim;
@@ -8660,7 +8742,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
goto unlock;
case RUN_ARRAY:
- err = do_md_run(mddev);
+ err = do_md_run(mddev, limp);
goto unlock;
case SET_BITMAP_FILE:
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 7f4e3ea8b826..73f6ef20f266 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -760,7 +760,12 @@ struct md_personality
* start up works that do NOT require md_thread. tasks that
* requires md_thread should go into start()
*/
- int (*run)(struct mddev *mddev);
+ /*
+ * @lim: a queue limits update the caller owns, or NULL. Non-NULL
+ * means stack into it rather than take q->limits_lock, which has to
+ * nest outside reconfig_mutex, see md_start_sync().
+ */
+ int (*run)(struct mddev *mddev, struct queue_limits *lim);
/* start up works that require md threads */
int (*start)(struct mddev *mddev);
void (*free)(struct mddev *mddev, void *priv);
@@ -959,7 +964,7 @@ extern void mddev_destroy(struct mddev *mddev);
void md_init_stacking_limits(struct queue_limits *lim);
struct mddev *md_alloc(dev_t dev, char *name);
void mddev_put(struct mddev *mddev);
-extern int md_run(struct mddev *mddev);
+extern int md_run(struct mddev *mddev, struct queue_limits *lim);
extern int md_start(struct mddev *mddev);
extern void md_stop(struct mddev *mddev);
extern void md_stop_writes(struct mddev *mddev);
@@ -1048,7 +1053,7 @@ void md_autostart_arrays(int part);
int md_set_array_info(struct mddev *mddev, struct mdu_array_info_s *info);
int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
struct queue_limits *lim);
-int do_md_run(struct mddev *mddev);
+int do_md_run(struct mddev *mddev, struct queue_limits *lim);
#define MDDEV_STACK_INTEGRITY (1u << 0)
int mddev_stack_rdev_limits(struct mddev *mddev, struct queue_limits *lim,
unsigned int flags);
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 35e103f0c2c3..59141e4299a8 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -379,7 +379,8 @@ static void raid0_free(struct mddev *mddev, void *priv)
kfree(conf);
}
-static int raid0_set_limits(struct mddev *mddev)
+static int raid0_set_limits(struct mddev *mddev,
+ struct queue_limits *caller_lim)
{
struct queue_limits lim;
int err;
@@ -398,10 +399,19 @@ static int raid0_set_limits(struct mddev *mddev)
err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
if (err)
return err;
+ /*
+ * The caller owns an update and commits it itself; taking
+ * q->limits_lock here would take it a second time.
+ */
+ if (caller_lim) {
+ *caller_lim = lim;
+ return 0;
+ }
+
return queue_limits_set(mddev->gendisk->queue, &lim);
}
-static int raid0_run(struct mddev *mddev)
+static int raid0_run(struct mddev *mddev, struct queue_limits *lim)
{
struct r0conf *conf;
int ret;
@@ -414,7 +424,7 @@ static int raid0_run(struct mddev *mddev)
return -EINVAL;
if (!mddev_is_dm(mddev)) {
- ret = raid0_set_limits(mddev);
+ ret = raid0_set_limits(mddev, lim);
if (ret)
return ret;
}
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 78effcac138d..6713a53fd460 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -3170,7 +3170,8 @@ static struct r1conf *setup_conf(struct mddev *mddev)
return ERR_PTR(err);
}
-static int raid1_set_limits(struct mddev *mddev)
+static int raid1_set_limits(struct mddev *mddev,
+ struct queue_limits *caller_lim)
{
struct queue_limits lim;
int err;
@@ -3185,10 +3186,19 @@ static int raid1_set_limits(struct mddev *mddev)
err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
if (err)
return err;
+ /*
+ * The caller owns an update and commits it itself; taking
+ * q->limits_lock here would take it a second time.
+ */
+ if (caller_lim) {
+ *caller_lim = lim;
+ return 0;
+ }
+
return queue_limits_set(mddev->gendisk->queue, &lim);
}
-static int raid1_run(struct mddev *mddev)
+static int raid1_run(struct mddev *mddev, struct queue_limits *lim)
{
struct r1conf *conf;
int i;
@@ -3219,7 +3229,7 @@ static int raid1_run(struct mddev *mddev)
return PTR_ERR(conf);
if (!mddev_is_dm(mddev)) {
- ret = raid1_set_limits(mddev);
+ ret = raid1_set_limits(mddev, lim);
if (ret) {
md_unregister_thread(mddev, &conf->thread);
if (!mddev->private)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5580ca77ef1e..16143db6085b 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3930,7 +3930,8 @@ static unsigned int raid10_nr_stripes(struct r10conf *conf)
return raid_disks / conf->geo.near_copies;
}
-static int raid10_set_queue_limits(struct mddev *mddev)
+static int raid10_set_queue_limits(struct mddev *mddev,
+ struct queue_limits *caller_lim)
{
struct r10conf *conf = mddev->private;
struct queue_limits lim;
@@ -3948,10 +3949,19 @@ static int raid10_set_queue_limits(struct mddev *mddev)
err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
if (err)
return err;
+ /*
+ * The caller owns an update and commits it itself; taking
+ * q->limits_lock here would take it a second time.
+ */
+ if (caller_lim) {
+ *caller_lim = lim;
+ return 0;
+ }
+
return queue_limits_set(mddev->gendisk->queue, &lim);
}
-static int raid10_run(struct mddev *mddev)
+static int raid10_run(struct mddev *mddev, struct queue_limits *lim)
{
struct r10conf *conf;
int i, disk_idx;
@@ -4020,7 +4030,7 @@ static int raid10_run(struct mddev *mddev)
}
if (!mddev_is_dm(conf->mddev)) {
- int err = raid10_set_queue_limits(mddev);
+ int err = raid10_set_queue_limits(mddev, lim);
if (err) {
ret = err;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 22759c631c4d..28bd81de86c1 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7944,7 +7944,8 @@ static int raid5_create_ctx_pool(struct r5conf *conf)
return conf->ctx_pool ? 0 : -ENOMEM;
}
-static int raid5_set_limits(struct mddev *mddev)
+static int raid5_set_limits(struct mddev *mddev,
+ struct queue_limits *caller_lim)
{
struct r5conf *conf = mddev->private;
struct queue_limits lim;
@@ -7996,10 +7997,19 @@ static int raid5_set_limits(struct mddev *mddev)
/* No restrictions on the number of segments in the request */
lim.max_segments = USHRT_MAX;
+ /*
+ * The caller owns an update and commits it itself; taking
+ * q->limits_lock here would take it a second time.
+ */
+ if (caller_lim) {
+ *caller_lim = lim;
+ return 0;
+ }
+
return queue_limits_set(mddev->gendisk->queue, &lim);
}
-static int raid5_run(struct mddev *mddev)
+static int raid5_run(struct mddev *mddev, struct queue_limits *lim)
{
struct r5conf *conf;
int dirty_parity_disks = 0;
@@ -8259,7 +8269,7 @@ static int raid5_run(struct mddev *mddev)
md_set_array_sectors(mddev, raid5_size(mddev, 0, 0));
if (!mddev_is_dm(mddev)) {
- ret = raid5_set_limits(mddev);
+ ret = raid5_set_limits(mddev, lim);
if (ret)
goto abort;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 7/8] md: open new legs before locking the array
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
` (5 preceding siblings ...)
2026-09-10 8:11 ` [PATCH v2 6/8] md: pass a queue_limits through ->run() Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
2026-09-10 8:11 ` [PATCH v2 8/8] md: link a new leg's holder " Jack Wang
7 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
Opening a leg takes disk->open_mutex, and the scsi disk probe path nests
q->limits_lock inside it (sd_open() -> sd_revalidate_disk()). md opens
legs under reconfig_mutex, which this series makes q->limits_lock nest
outside, closing a cycle. Booting with lockdep on an md root reports it
during assembly:
-> #2 (&q->limits_lock): sd_revalidate_disk / sd_open
-> #1 (&disk->open_mutex): md_import_device
md_add_new_disk
md_ioctl <- ADD_NEW_DISK
-> #0 (&mddev->reconfig_mutex): md_ioctl <- RUN_ARRAY
Move every open out from under the lock. md_import_new_disk() mirrors
md_add_new_disk()'s branch selection so all three of its branches take a
pre-opened leg, and hot_add_disk(), new_dev_store() and md_setup_drive()
open before they lock as well.
The mddev fields the open depends on are read without reconfig_mutex, so
each caller rechecks them once the array is locked and rejects the add
with -EBUSY if the branch or the superblock format would have changed.
md_autostart_arrays() needs no change: it opens under
detected_devices_mutex.
Assisted-by: LLM
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/md/md-autodetect.c | 17 ++-
drivers/md/md.c | 238 ++++++++++++++++++++++++++-----------
drivers/md/md.h | 21 +++-
3 files changed, 203 insertions(+), 73 deletions(-)
diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
index e15ae2fb58a2..e592577356ad 100644
--- a/drivers/md/md-autodetect.c
+++ b/drivers/md/md-autodetect.c
@@ -208,6 +208,7 @@ static void __init md_setup_drive(struct md_setup_args *args)
.major = MAJOR(devices[i]),
.minor = MINOR(devices[i]),
};
+ struct md_new_disk nd;
if (args->level != LEVEL_NONE) {
dinfo.number = i;
@@ -216,7 +217,21 @@ static void __init md_setup_drive(struct md_setup_args *args)
(1 << MD_DISK_ACTIVE) | (1 << MD_DISK_SYNC);
}
- md_add_new_disk(mddev, &dinfo, NULL);
+ /*
+ * Opening a leg takes disk->open_mutex, which must not nest
+ * inside reconfig_mutex, see md_import_new_disk(). Drop the
+ * array lock around it; this is __init and the array is not
+ * reachable yet, so nothing else can touch it in between.
+ */
+ mddev_unlock(mddev);
+ if (md_import_new_disk(mddev, &dinfo, &nd)) {
+ mddev_lock_nointr(mddev);
+ continue;
+ }
+ mddev_lock_nointr(mddev);
+
+ md_add_new_disk(mddev, &dinfo, &nd, NULL);
+ md_put_new_disk(&nd);
}
/*
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 5be956e80563..fa033d7d3831 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4942,6 +4942,7 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
struct md_rdev *rdev;
unsigned int noio_flags;
int err;
+ int persistent, external, major_version, minor_version;
if (!*buf || *e != ':' || !e[1] || e[1] == '\n')
return -EINVAL;
@@ -4953,32 +4954,52 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
minor != MINOR(dev))
return -EOVERFLOW;
- err = mddev_suspend_and_lock(mddev);
- if (err)
- return err;
- noio_flags = memalloc_noio_save();
- if (mddev->persistent) {
- rdev = md_import_device(dev, mddev->major_version,
- mddev->minor_version);
- if (!IS_ERR(rdev) && !list_empty(&mddev->disks)) {
- struct md_rdev *rdev0
- = list_entry(mddev->disks.next,
- struct md_rdev, same_set);
- err = super_types[mddev->major_version]
- .load_super(rdev, rdev0, mddev->minor_version);
- if (err < 0)
- goto out;
- }
- } else if (mddev->external)
+ /*
+ * Open before locking the array: bdev_open() takes disk->open_mutex,
+ * which must not nest inside reconfig_mutex, see md_import_new_disk().
+ * The fields below are read without the lock and rechecked under it.
+ */
+ persistent = READ_ONCE(mddev->persistent);
+ external = READ_ONCE(mddev->external);
+ major_version = READ_ONCE(mddev->major_version);
+ minor_version = READ_ONCE(mddev->minor_version);
+
+ if (persistent)
+ rdev = md_import_device(dev, major_version, minor_version);
+ else if (external)
rdev = md_import_device(dev, -2, -1);
else
rdev = md_import_device(dev, -1, -1);
- if (IS_ERR(rdev)) {
- memalloc_noio_restore(noio_flags);
- mddev_unlock_and_resume(mddev);
+ if (IS_ERR(rdev))
return PTR_ERR(rdev);
+
+ err = mddev_suspend_and_lock(mddev);
+ if (err) {
+ export_rdev(rdev);
+ return err;
}
+ noio_flags = memalloc_noio_save();
+
+ if (persistent != mddev->persistent || external != mddev->external ||
+ major_version != mddev->major_version ||
+ minor_version != mddev->minor_version) {
+ pr_warn("%s: array reconfigured while opening %pg\n",
+ mdname(mddev), rdev->bdev);
+ err = -EBUSY;
+ goto out;
+ }
+
+ if (mddev->persistent && !list_empty(&mddev->disks)) {
+ struct md_rdev *rdev0
+ = list_entry(mddev->disks.next,
+ struct md_rdev, same_set);
+ err = super_types[mddev->major_version]
+ .load_super(rdev, rdev0, mddev->minor_version);
+ if (err < 0)
+ goto out;
+ }
+
err = bind_rdev_to_array(rdev, mddev);
out:
if (err)
@@ -7685,12 +7706,35 @@ static int get_disk_info(struct mddev *mddev, void __user * arg)
return 0;
}
+/*
+ * @nd carries an rdev the caller opened before locking the array, for the
+ * branch its snapshot selected. Every caller must open first; doing it
+ * here would nest disk->open_mutex inside reconfig_mutex.
+ */
int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
- struct queue_limits *lim)
+ struct md_new_disk *nd, struct queue_limits *lim)
{
struct md_rdev *rdev;
dev_t dev = MKDEV(info->major,info->minor);
+ /*
+ * The open ran unlocked, so anything that selects a different branch
+ * below, or a different superblock format, means it was done against
+ * an array that no longer looks like this one.
+ */
+ if (nd && nd->rdev &&
+ (nd->have_raid_disks != (mddev->raid_disks != 0) ||
+ nd->have_pers != !!mddev->pers ||
+ nd->persistent != mddev->persistent ||
+ nd->major_version != mddev->major_version ||
+ nd->minor_version != mddev->minor_version)) {
+ pr_warn("%s: array reconfigured while opening %pg\n",
+ mdname(mddev), nd->rdev->bdev);
+ export_rdev(nd->rdev);
+ nd->rdev = NULL;
+ return -EBUSY;
+ }
+
if (mddev_is_clustered(mddev) &&
!(info->state & ((1 << MD_DISK_CLUSTER_ADD) | (1 << MD_DISK_CANDIDATE)))) {
pr_warn("%s: Cannot add to clustered mddev.\n",
@@ -7703,13 +7747,12 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
if (!mddev->raid_disks) {
int err;
+
/* expecting a device which has a superblock */
- rdev = md_import_device(dev, mddev->major_version, mddev->minor_version);
- if (IS_ERR(rdev)) {
- pr_warn("md: md_import_device returned %ld\n",
- PTR_ERR(rdev));
- return PTR_ERR(rdev);
- }
+ if (WARN_ON_ONCE(!nd || !nd->rdev))
+ return -EINVAL;
+ rdev = nd->rdev;
+ nd->rdev = NULL;
if (!list_empty(&mddev->disks)) {
struct md_rdev *rdev0
= list_entry(mddev->disks.next,
@@ -7742,16 +7785,10 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
mdname(mddev));
return -EINVAL;
}
- if (mddev->persistent)
- rdev = md_import_device(dev, mddev->major_version,
- mddev->minor_version);
- else
- rdev = md_import_device(dev, -1, -1);
- if (IS_ERR(rdev)) {
- pr_warn("md: md_import_device returned %ld\n",
- PTR_ERR(rdev));
- return PTR_ERR(rdev);
- }
+ if (WARN_ON_ONCE(!nd || !nd->rdev))
+ return -EINVAL;
+ rdev = nd->rdev;
+ nd->rdev = NULL;
/* set saved_raid_disk if appropriate */
if (!mddev->persistent) {
if (info->state & (1<<MD_DISK_SYNC) &&
@@ -7853,12 +7890,11 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
if (!(info->state & (1<<MD_DISK_FAULTY))) {
int err;
- rdev = md_import_device(dev, -1, 0);
- if (IS_ERR(rdev)) {
- pr_warn("md: error, md_import_device() returned %ld\n",
- PTR_ERR(rdev));
- return PTR_ERR(rdev);
- }
+
+ if (WARN_ON_ONCE(!nd || !nd->rdev))
+ return -EINVAL;
+ rdev = nd->rdev;
+ nd->rdev = NULL;
rdev->desc_nr = info->number;
if (info->raid_disk < mddev->raid_disks)
rdev->raid_disk = info->raid_disk;
@@ -7930,7 +7966,8 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev,
return -EBUSY;
}
-static int hot_add_disk(struct mddev *mddev, dev_t dev)
+/* @nd carries a leg the caller opened before the array was locked */
+static int hot_add_disk(struct mddev *mddev, struct md_new_disk *nd)
{
int err;
struct md_rdev *rdev;
@@ -7949,12 +7986,10 @@ static int hot_add_disk(struct mddev *mddev, dev_t dev)
return -EINVAL;
}
- rdev = md_import_device(dev, -1, 0);
- if (IS_ERR(rdev)) {
- pr_warn("md: error, md_import_device() returned %ld\n",
- PTR_ERR(rdev));
+ if (WARN_ON_ONCE(!nd->rdev))
return -EINVAL;
- }
+ rdev = nd->rdev;
+ nd->rdev = NULL;
if (mddev->persistent)
rdev->sb_start = calc_dev_sboffset(rdev);
@@ -8497,14 +8532,61 @@ static inline int md_ioctl_valid(unsigned int cmd)
* journal device or a personality without ->hot_remove_disk, but that depends
* on disk info still in user memory here, so it is included as a whole.
*/
-static bool md_ioctl_may_add_disk(unsigned int cmd)
+
+/*
+ * Open the leg before the array is locked; bdev_open() takes
+ * disk->open_mutex, which must not nest inside reconfig_mutex. mddev is
+ * read unlocked on purpose, and md_add_new_disk() rechecks the snapshot.
+ */
+int md_import_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
+ struct md_new_disk *nd)
{
- switch (cmd) {
- case ADD_NEW_DISK:
- case HOT_REMOVE_DISK:
- return true;
- default:
- return false;
+ dev_t dev = MKDEV(info->major, info->minor);
+ struct md_rdev *rdev;
+
+ memset(nd, 0, sizeof(*nd));
+ nd->have_raid_disks = READ_ONCE(mddev->raid_disks) != 0;
+ nd->have_pers = !!READ_ONCE(mddev->pers);
+ nd->persistent = READ_ONCE(mddev->persistent);
+ nd->major_version = READ_ONCE(mddev->major_version);
+ nd->minor_version = READ_ONCE(mddev->minor_version);
+
+ if (!nd->have_raid_disks) {
+ /* a device with a superblock, for an array being assembled */
+ rdev = md_import_device(dev, nd->major_version,
+ nd->minor_version);
+ } else if (nd->have_pers) {
+ /* a hot spare; this is the branch that stacks limits */
+ nd->stacks = true;
+ if (nd->persistent)
+ rdev = md_import_device(dev, nd->major_version,
+ nd->minor_version);
+ else
+ rdev = md_import_device(dev, -1, -1);
+ } else if (nd->major_version == 0) {
+ rdev = md_import_device(dev, -1, 0);
+ } else {
+ /* md_add_new_disk() rejects this, nothing to open */
+ return 0;
+ }
+
+ if (IS_ERR(rdev)) {
+ int err = PTR_ERR(rdev);
+
+ pr_warn("md: md_import_device returned %d\n", err);
+ return err;
+ }
+
+ nd->rdev = rdev;
+ return 0;
+}
+
+/* release a leg md_add_new_disk() did not take ownership of */
+void md_put_new_disk(struct md_new_disk *nd)
+{
+ if (nd->rdev) {
+ export_rdev(nd->rdev);
+ nd->rdev = NULL;
}
}
@@ -8566,6 +8648,8 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
struct request_queue *q = NULL;
struct queue_limits lim;
struct queue_limits *limp = NULL;
+ struct md_new_disk nd = { };
+ mdu_disk_info_t info;
bool suspend;
err = md_ioctl_valid(cmd);
@@ -8616,8 +8700,27 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
if (!md_is_rdwr(mddev))
flush_work(&mddev->sync_work);
+ if (cmd == ADD_NEW_DISK) {
+ if (copy_from_user(&info, argp, sizeof(info))) {
+ err = -EFAULT;
+ goto out;
+ }
+ err = md_import_new_disk(mddev, &info, &nd);
+ if (err)
+ goto out;
+ } else if (cmd == HOT_ADD_DISK) {
+ nd.rdev = md_import_device(new_decode_dev(arg), -1, 0);
+ if (IS_ERR(nd.rdev)) {
+ pr_warn("md: error, md_import_device() returned %ld\n",
+ PTR_ERR(nd.rdev));
+ nd.rdev = NULL;
+ err = -EINVAL;
+ goto out;
+ }
+ }
+
/* q->limits_lock nests outside both, see md_start_sync() */
- if ((md_ioctl_may_add_disk(cmd) || cmd == RUN_ARRAY) &&
+ if ((nd.stacks || cmd == HOT_REMOVE_DISK || cmd == RUN_ARRAY) &&
!mddev_is_dm(mddev)) {
q = mddev->gendisk->queue;
lim = queue_limits_start_update(q);
@@ -8681,14 +8784,10 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
* So require mddev->pers and MD_DISK_SYNC.
*/
if (mddev->pers) {
- mdu_disk_info_t info;
- if (copy_from_user(&info, argp, sizeof(info)))
- err = -EFAULT;
- else if (!(info.state & (1<<MD_DISK_SYNC)))
+ if (!(info.state & (1<<MD_DISK_SYNC)))
/* Need to clear read-only for this */
break;
- else
- err = md_add_new_disk(mddev, &info, limp);
+ err = md_add_new_disk(mddev, &info, &nd, limp);
goto unlock;
}
break;
@@ -8721,14 +8820,8 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
switch (cmd) {
case ADD_NEW_DISK:
- {
- mdu_disk_info_t info;
- if (copy_from_user(&info, argp, sizeof(info)))
- err = -EFAULT;
- else
- err = md_add_new_disk(mddev, &info, limp);
+ err = md_add_new_disk(mddev, &info, &nd, limp);
goto unlock;
- }
case CLUSTERED_DISK_NACK:
if (mddev_is_clustered(mddev))
@@ -8738,7 +8831,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
goto unlock;
case HOT_ADD_DISK:
- err = hot_add_disk(mddev, new_decode_dev(arg));
+ err = hot_add_disk(mddev, &nd);
goto unlock;
case RUN_ARRAY:
@@ -8770,6 +8863,9 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
}
out:
+ /* a leg we opened but nothing took ownership of */
+ md_put_new_disk(&nd);
+
if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
clear_bit(MD_CLOSING, &mddev->flags);
return err;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 73f6ef20f266..73a27d83d65a 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -1051,8 +1051,27 @@ struct mdu_disk_info_s;
extern int mdp_major;
void md_autostart_arrays(int part);
int md_set_array_info(struct mddev *mddev, struct mdu_array_info_s *info);
+/*
+ * A leg opened before the array was locked, with the mddev fields that
+ * selected the branch and the superblock format. Opening takes
+ * disk->open_mutex, which must not nest inside reconfig_mutex; the fields
+ * are read unlocked and md_add_new_disk() rechecks them.
+ */
+struct md_new_disk {
+ struct md_rdev *rdev;
+ bool stacks; /* the add can reach ->hot_add_disk() */
+ bool have_pers;
+ bool have_raid_disks;
+ int persistent;
+ int major_version;
+ int minor_version;
+};
+
+int md_import_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
+ struct md_new_disk *nd);
+void md_put_new_disk(struct md_new_disk *nd);
int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
- struct queue_limits *lim);
+ struct md_new_disk *nd, struct queue_limits *lim);
int do_md_run(struct mddev *mddev, struct queue_limits *lim);
#define MDDEV_STACK_INTEGRITY (1u << 0)
int mddev_stack_rdev_limits(struct mddev *mddev, struct queue_limits *lim,
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 8/8] md: link a new leg's holder before locking the array
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
` (6 preceding siblings ...)
2026-09-10 8:11 ` [PATCH v2 7/8] md: open new legs before locking the array Jack Wang
@ 2026-09-10 8:11 ` Jack Wang
7 siblings, 0 replies; 11+ messages in thread
From: Jack Wang @ 2026-09-10 8:11 UTC (permalink / raw)
To: Song Liu, Yu Kuai, linux-raid, Nilay Shroff, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
From: Jack Wang <jinpu.wang@cloud.ionos.com>
bd_link_disk_holder() takes the leg's disk->open_mutex, and
bind_rdev_to_array() calls it with reconfig_mutex held, so the
dependency the previous patch removed from md_import_device() is still
there by another route:
-> #2 (&q->limits_lock): sd_revalidate_disk / sd_open
-> #1 (&disk->open_mutex): bd_link_disk_holder
bind_rdev_to_array
md_add_new_disk
md_ioctl <- ADD_NEW_DISK
-> #0 (&mddev->reconfig_mutex): md_ioctl <- RUN_ARRAY
bd_unlink_disk_holder() only takes blk_holder_mutex, which is why the
release side needs no change and made the link side easy to miss.
Link the holder where the leg is opened, before the array is locked, and
record it in a new HolderLinked flag so the release side knows whether
there is a link to drop. A failed link is not fatal, as before. A leg
that is linked but not yet bound is released through md_export_rdev(),
which drops the link first.
A leg is now linked before it is known to be acceptable, so a leg the
array goes on to reject shows up in its slaves directory until the
error path releases it.
md then no longer takes disk->open_mutex under reconfig_mutex: of the
functions that take it, md reaches bdev_open() and bd_link_disk_holder()
from the paths above, bdev_release() and bdev_fput() only through fput(),
which defers to task work, del_gendisk() only from mddev teardown, and
never sync_bdevs().
Assisted-by: LLM
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/md/md-autodetect.c | 2 +-
drivers/md/md.c | 70 ++++++++++++++++++++++++++++----------
drivers/md/md.h | 7 +++-
3 files changed, 59 insertions(+), 20 deletions(-)
diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
index e592577356ad..b6f9fb36f1bb 100644
--- a/drivers/md/md-autodetect.c
+++ b/drivers/md/md-autodetect.c
@@ -231,7 +231,7 @@ static void __init md_setup_drive(struct md_setup_args *args)
mddev_lock_nointr(mddev);
md_add_new_disk(mddev, &dinfo, &nd, NULL);
- md_put_new_disk(&nd);
+ md_put_new_disk(mddev, &nd);
}
/*
diff --git a/drivers/md/md.c b/drivers/md/md.c
index fa033d7d3831..235f0d645cea 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2632,7 +2632,7 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
sysfs_get_dirent_safe(rdev->kobj.sd, "bad_blocks");
list_add_rcu(&rdev->same_set, &mddev->disks);
- bd_link_disk_holder(rdev->bdev, mddev->gendisk);
+ /* the holder is linked with the open, see md_link_rdev_holder() */
return 0;
@@ -2648,6 +2648,23 @@ void md_autodetect_dev(dev_t dev);
/* just for claiming the bdev */
static struct md_rdev claim_rdev;
+/*
+ * bd_link_disk_holder() takes the leg's disk->open_mutex, so the link is
+ * made with the open, before the array is locked. bd_unlink_disk_holder()
+ * only takes blk_holder_mutex, so dropping it is safe under any lock.
+ */
+static void md_link_rdev_holder(struct md_rdev *rdev, struct mddev *mddev)
+{
+ if (!bd_link_disk_holder(rdev->bdev, mddev->gendisk))
+ set_bit(HolderLinked, &rdev->flags);
+}
+
+static void md_unlink_rdev_holder(struct md_rdev *rdev, struct mddev *mddev)
+{
+ if (test_and_clear_bit(HolderLinked, &rdev->flags))
+ bd_unlink_disk_holder(rdev->bdev, mddev->gendisk);
+}
+
static void export_rdev(struct md_rdev *rdev)
{
pr_debug("md: export_rdev(%pg)\n", rdev->bdev);
@@ -2661,11 +2678,18 @@ static void export_rdev(struct md_rdev *rdev)
kobject_put(&rdev->kobj);
}
+/* release a leg that was linked before the array was locked */
+static void md_export_rdev(struct mddev *mddev, struct md_rdev *rdev)
+{
+ md_unlink_rdev_holder(rdev, mddev);
+ export_rdev(rdev);
+}
+
static void md_kick_rdev_from_array(struct md_rdev *rdev)
{
struct mddev *mddev = rdev->mddev;
- bd_unlink_disk_holder(rdev->bdev, rdev->mddev->gendisk);
+ md_unlink_rdev_holder(rdev, rdev->mddev);
list_del_rcu(&rdev->same_set);
pr_debug("md: unbind<%pg>\n", rdev->bdev);
mddev_destroy_serial_pool(rdev->mddev, rdev);
@@ -4974,9 +4998,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
if (IS_ERR(rdev))
return PTR_ERR(rdev);
+ md_link_rdev_holder(rdev, mddev);
+
err = mddev_suspend_and_lock(mddev);
if (err) {
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return err;
}
noio_flags = memalloc_noio_save();
@@ -5003,7 +5029,7 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
err = bind_rdev_to_array(rdev, mddev);
out:
if (err)
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
memalloc_noio_restore(noio_flags);
mddev_unlock_and_resume(mddev);
if (!err)
@@ -7519,6 +7545,10 @@ static void autorun_devices(int part)
limp = &lim;
}
+ /* link before locking, see md_link_rdev_holder() */
+ rdev_for_each_list(rdev, tmp, &candidates)
+ md_link_rdev_holder(rdev, mddev);
+
if (mddev_suspend_and_lock(mddev)) {
pr_warn("md: %s locked, cannot run\n", mdname(mddev));
if (limp) {
@@ -7538,7 +7568,7 @@ static void autorun_devices(int part)
rdev_for_each_list(rdev, tmp, &candidates) {
list_del_init(&rdev->same_set);
if (bind_rdev_to_array(rdev, mddev))
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
}
autorun_array(mddev, limp);
if (limp && queue_limits_commit_update(q, limp))
@@ -7552,7 +7582,7 @@ static void autorun_devices(int part)
*/
rdev_for_each_list(rdev, tmp, &candidates) {
list_del_init(&rdev->same_set);
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
}
mddev_put(mddev);
}
@@ -7730,7 +7760,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
nd->minor_version != mddev->minor_version)) {
pr_warn("%s: array reconfigured while opening %pg\n",
mdname(mddev), nd->rdev->bdev);
- export_rdev(nd->rdev);
+ md_export_rdev(mddev, nd->rdev);
nd->rdev = NULL;
return -EBUSY;
}
@@ -7763,13 +7793,13 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
pr_warn("md: %pg has different UUID to %pg\n",
rdev->bdev,
rdev0->bdev);
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return -EINVAL;
}
}
err = bind_rdev_to_array(rdev, mddev);
if (err)
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return err;
}
@@ -7806,7 +7836,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
/* This was a hot-add request, but events doesn't
* match, so reject it.
*/
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return -EINVAL;
}
@@ -7832,7 +7862,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
}
}
if (has_journal || mddev->bitmap) {
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return -EBUSY;
}
set_bit(Journal, &rdev->flags);
@@ -7847,7 +7877,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
/* --add initiated by this node */
err = mddev->cluster_ops->add_new_disk(mddev, rdev);
if (err) {
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return err;
}
}
@@ -7857,7 +7887,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
err = bind_rdev_to_array(rdev, mddev);
if (err)
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
if (mddev_is_clustered(mddev)) {
if (info->state & (1 << MD_DISK_CANDIDATE)) {
@@ -7919,7 +7949,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
err = bind_rdev_to_array(rdev, mddev);
if (err) {
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return err;
}
}
@@ -8031,7 +8061,7 @@ static int hot_add_disk(struct mddev *mddev, struct md_new_disk *nd)
return 0;
abort_export:
- export_rdev(rdev);
+ md_export_rdev(mddev, rdev);
return err;
}
@@ -8577,15 +8607,18 @@ int md_import_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
return err;
}
+ /* link the holder here too, for the same reason */
+ md_link_rdev_holder(rdev, mddev);
+
nd->rdev = rdev;
return 0;
}
/* release a leg md_add_new_disk() did not take ownership of */
-void md_put_new_disk(struct md_new_disk *nd)
+void md_put_new_disk(struct mddev *mddev, struct md_new_disk *nd)
{
if (nd->rdev) {
- export_rdev(nd->rdev);
+ md_export_rdev(mddev, nd->rdev);
nd->rdev = NULL;
}
}
@@ -8717,6 +8750,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
err = -EINVAL;
goto out;
}
+ md_link_rdev_holder(nd.rdev, mddev);
}
/* q->limits_lock nests outside both, see md_start_sync() */
@@ -8864,7 +8898,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
out:
/* a leg we opened but nothing took ownership of */
- md_put_new_disk(&nd);
+ md_put_new_disk(mddev, &nd);
if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
clear_bit(MD_CLOSING, &mddev->flags);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 73a27d83d65a..1a0d57d58ad1 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -294,6 +294,11 @@ enum flag_bits {
* serial bios.
*/
Nonrot, /* non-rotational device (SSD) */
+ HolderLinked, /* bd_link_disk_holder() succeeded for this
+ * leg. The link is made before the array is
+ * locked, as it takes disk->open_mutex,
+ * see md_import_new_disk().
+ */
};
static inline int is_badblock(struct md_rdev *rdev, sector_t s, sector_t sectors,
@@ -1069,7 +1074,7 @@ struct md_new_disk {
int md_import_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
struct md_new_disk *nd);
-void md_put_new_disk(struct md_new_disk *nd);
+void md_put_new_disk(struct mddev *mddev, struct md_new_disk *nd);
int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info,
struct md_new_disk *nd, struct queue_limits *lim);
int do_md_run(struct mddev *mddev, struct queue_limits *lim);
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/8] md: pass a queue_limits down to ->hot_add_disk()
2026-09-10 8:11 ` [PATCH v2 1/8] md: pass a queue_limits down to ->hot_add_disk() Jack Wang
@ 2026-09-11 10:46 ` Nilay Shroff
0 siblings, 0 replies; 11+ messages in thread
From: Nilay Shroff @ 2026-09-11 10:46 UTC (permalink / raw)
To: Jack Wang, Song Liu, Yu Kuai, linux-raid, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
On 9/10/26 1:41 PM, Jack Wang wrote:
> From: Jack Wang <jinpu.wang@cloud.ionos.com>
>
> Adding a leg stacks its queue limits, which mddev_stack_new_rdev() does
> by taking q->limits_lock itself. Callers holding reconfig_mutex or a
> suspended array cannot allow that, and must own the update instead.
>
> Give ->hot_add_disk(), remove_and_add_spares() and
> md_choose_sync_action() a struct queue_limits argument with three
> states: an update to stack into, NULL to let the personality take the
> lock as before, or MDDEV_STACK_SKIP to add the leg without touching the
> limits, for callers that can do neither. mddev_stack_rdev_into() stacks
> into a caller-owned update without the lock.
>
> Every caller still passes NULL and nothing passes the sentinel yet, so
> there is no functional change; the users follow.
>
> Assisted-by: LLM
> Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
> drivers/md/dm-raid.c | 2 +-
> drivers/md/md-linear.c | 28 ++++++++++++++----
> drivers/md/md.c | 66 ++++++++++++++++++++++++++++++++----------
> drivers/md/md.h | 11 ++++++-
> drivers/md/raid1.c | 10 +++++--
> drivers/md/raid10.c | 19 +++++++++---
> drivers/md/raid5.c | 5 ++--
> 7 files changed, 110 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 8f5a5e1342a9..21a1922bee4f 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -3923,7 +3923,7 @@ static void attempt_restore_of_faulty_devices(struct raid_set *rs)
> clear_bit(Faulty, &r->flags);
> clear_bit(WriteErrorSeen, &r->flags);
>
> - if (mddev->pers->hot_add_disk(mddev, r)) {
> + if (mddev->pers->hot_add_disk(mddev, r, NULL)) {
> /* Failed to revive this device, try next */
> r->raid_disk = r->saved_raid_disk = -1;
> r->flags = flags;
> diff --git a/drivers/md/md-linear.c b/drivers/md/md-linear.c
> index 73b367b61b87..da82c313d459 100644
> --- a/drivers/md/md-linear.c
> +++ b/drivers/md/md-linear.c
> @@ -65,11 +65,16 @@ static sector_t linear_size(struct mddev *mddev, sector_t sectors, int raid_disk
> return array_sectors;
> }
>
> -static int linear_set_limits(struct mddev *mddev)
> +static int linear_set_limits(struct mddev *mddev,
> + struct queue_limits *caller_lim)
> {
> struct queue_limits lim;
> int err;
>
> + /* the caller can neither stack nor take q->limits_lock */
> + if (caller_lim == MDDEV_STACK_SKIP)
> + return 0;
> +
> md_init_stacking_limits(&lim);
> lim.features |= BLK_FEAT_NOWAIT;
> lim.max_hw_sectors = mddev->chunk_sectors;
> @@ -82,10 +87,20 @@ static int linear_set_limits(struct mddev *mddev)
> if (err)
> return err;
>
> + /*
> + * The caller owns an update and commits it itself; taking
> + * q->limits_lock here would take it a second time.
> + */
> + if (caller_lim) {
> + *caller_lim = lim;
> + return 0;
> + }
> +
> return queue_limits_set(mddev->gendisk->queue, &lim);
> }
>
This looks overly complicated with three different cases where
linear_set_limits() either ignores the limits update, updates the limits
provided by the caller without committing them, or updates and commits the
limits itself.
Why can't we instead have the callers always pass a struct queue_limits
pointer, and make linear_set_limits() only update the limits provided by
its caller without committing them?
The caller can then decide what to do with the resulting limits: either
ignore them or commit them as appropriate. This also avoids introducing
MDDEV_STACK_SKIP as a special sentinel value.
In this model, linear_set_limits() would only be responsible for preparing
the limits. This also keeps the locking and limits-commit logic in one common
place. The caller is then responsible for acquiring the appropriate locks and
committing the limits in the correct order for its particular context.
So the core logic is: personality should describe what the limits need to
become and the MD core/caller should decide when those limits become visible.
[...]
>
> +/*
> + * Stack a new rdev into limits the caller already holds limits_lock for and
> + * will commit itself. Used from paths that must take limits_lock before
> + * quiescing the array, see md_start_sync().
> + */
> +int mddev_stack_rdev_into(struct mddev *mddev, struct md_rdev *rdev,
> + struct queue_limits *lim)
> +{
> + struct queue_limits tmp = *lim;
> +
> + if (mddev_is_dm(mddev))
> + return 0;
> +
> + if (queue_logical_block_size(rdev->bdev->bd_disk->queue) >
> + queue_logical_block_size(mddev->gendisk->queue)) {
> + pr_err("%s: incompatible logical_block_size, can not add\n",
> + mdname(mddev));
> + return -EINVAL;
> + }
> +
> + queue_limits_stack_bdev(&tmp, rdev->bdev, rdev->data_offset,
> + mddev->gendisk->disk_name);
> +
> + if (!queue_limits_stack_integrity_bdev(&tmp, rdev->bdev)) {
> + pr_err("%s: incompatible integrity profile for %pg\n",
> + mdname(mddev), rdev->bdev);
> + return -ENXIO;
> + }
> +
> + *lim = tmp;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mddev_stack_rdev_into);
> +
This API is correctly moving in that direction which I proposed above.
But rather than adding new API, I'd update mddev_stack_new_rdev() (or
rename it to mddev_stack_rdev_into()) which would stack the rdev into
the caller-provided struct queue_limits without taking q->limits_lock
or committing the limits.
[...]
> +/*
> + * Sentinel for the queue_limits argument of ->hot_add_disk(). The caller has
> + * no update to stack into and must not take q->limits_lock itself, so the leg
> + * is added with the array's current limits.
> + */
> +#define MDDEV_STACK_SKIP ((struct queue_limits *)ERR_PTR(-EAGAIN))
If we follow the design as I suggested above then we can get away with
above sentinel.
[...]
> -static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> +static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev,
> + struct queue_limits *lim)
> {
> struct r1conf *conf = mddev->private;
> int err = -EEXIST;
> @@ -1923,7 +1924,12 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> for (mirror = first; mirror <= last; mirror++) {
> p = conf->mirrors + mirror;
> if (!p->rdev) {
> - err = mddev_stack_new_rdev(mddev, rdev);
> + if (lim == MDDEV_STACK_SKIP)
> + err = 0;
> + else if (lim)
> + err = mddev_stack_rdev_into(mddev, rdev, lim);
> + else
> + err = mddev_stack_new_rdev(mddev, rdev);
> if (err)
> return err;
>
Here as well the same comment as linear_set_limits().
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 1093c798d9dd..222bd7badcff 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2095,7 +2095,8 @@ static int raid10_spare_active(struct mddev *mddev)
> return count;
> }
>
> -static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> +static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev,
> + struct queue_limits *lim)
> {
> struct r10conf *conf = mddev->private;
> int err = -EEXIST;
> @@ -2130,7 +2131,12 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> continue;
> }
>
> - err = mddev_stack_new_rdev(mddev, rdev);
> + if (lim == MDDEV_STACK_SKIP)
> + err = 0;
> + else if (lim)
> + err = mddev_stack_rdev_into(mddev, rdev, lim);
> + else
> + err = mddev_stack_new_rdev(mddev, rdev);
> if (err)
> return err;
> p->head_position = 0;
> @@ -2147,7 +2153,12 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
> clear_bit(In_sync, &rdev->flags);
> set_bit(Replacement, &rdev->flags);
> rdev->raid_disk = repl_slot;
> - err = mddev_stack_new_rdev(mddev, rdev);
> + if (lim == MDDEV_STACK_SKIP)
> + err = 0;
> + else if (lim)
> + err = mddev_stack_rdev_into(mddev, rdev, lim);
> + else
> + err = mddev_stack_new_rdev(mddev, rdev);
> if (err)
> return err;
> conf->fullsync = 1;
Again same comment as linear_set_limits().
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 6/8] md: pass a queue_limits through ->run()
2026-09-10 8:11 ` [PATCH v2 6/8] md: pass a queue_limits through ->run() Jack Wang
@ 2026-09-11 10:54 ` Nilay Shroff
0 siblings, 0 replies; 11+ messages in thread
From: Nilay Shroff @ 2026-09-11 10:54 UTC (permalink / raw)
To: Jack Wang, Song Liu, Yu Kuai, linux-raid, abd.masalkhi
Cc: linux-block, Jens Axboe, Christoph Hellwig, Damien Le Moal,
Ming Lei, Xiao Ni, Li Nan, Mike Snitzer, Mikulas Patocka,
dm-devel, linux-kernel, Jack Wang
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 35e103f0c2c3..59141e4299a8 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -379,7 +379,8 @@ static void raid0_free(struct mddev *mddev, void *priv)
> kfree(conf);
> }
>
> -static int raid0_set_limits(struct mddev *mddev)
> +static int raid0_set_limits(struct mddev *mddev,
> + struct queue_limits *caller_lim)
> {
> struct queue_limits lim;
> int err;
> @@ -398,10 +399,19 @@ static int raid0_set_limits(struct mddev *mddev)
> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
> if (err)
> return err;
> + /*
> + * The caller owns an update and commits it itself; taking
> + * q->limits_lock here would take it a second time.
> + */
> + if (caller_lim) {
> + *caller_lim = lim;
> + return 0;
> + }
> +
> return queue_limits_set(mddev->gendisk->queue, &lim);
> }
>
[...]
> -static int raid1_set_limits(struct mddev *mddev)
> +static int raid1_set_limits(struct mddev *mddev,
> + struct queue_limits *caller_lim)
> {
> struct queue_limits lim;
> int err;
> @@ -3185,10 +3186,19 @@ static int raid1_set_limits(struct mddev *mddev)
> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
> if (err)
> return err;
> + /*
> + * The caller owns an update and commits it itself; taking
> + * q->limits_lock here would take it a second time.
> + */
> + if (caller_lim) {
> + *caller_lim = lim;
> + return 0;
> + }
> +
> return queue_limits_set(mddev->gendisk->queue, &lim);
> }
>
[...]
>
> -static int raid10_set_queue_limits(struct mddev *mddev)
> +static int raid10_set_queue_limits(struct mddev *mddev,
> + struct queue_limits *caller_lim)
> {
> struct r10conf *conf = mddev->private;
> struct queue_limits lim;
> @@ -3948,10 +3949,19 @@ static int raid10_set_queue_limits(struct mddev *mddev)
> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
> if (err)
> return err;
> + /*
> + * The caller owns an update and commits it itself; taking
> + * q->limits_lock here would take it a second time.
> + */
> + if (caller_lim) {
> + *caller_lim = lim;
> + return 0;
> + }
> +
> return queue_limits_set(mddev->gendisk->queue, &lim);
> }
>
[...]
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 22759c631c4d..28bd81de86c1 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7944,7 +7944,8 @@ static int raid5_create_ctx_pool(struct r5conf *conf)
> return conf->ctx_pool ? 0 : -ENOMEM;
> }
>
> -static int raid5_set_limits(struct mddev *mddev)
> +static int raid5_set_limits(struct mddev *mddev,
> + struct queue_limits *caller_lim)
> {
> struct r5conf *conf = mddev->private;
> struct queue_limits lim;
> @@ -7996,10 +7997,19 @@ static int raid5_set_limits(struct mddev *mddev)
> /* No restrictions on the number of segments in the request */
> lim.max_segments = USHRT_MAX;
>
> + /*
> + * The caller owns an update and commits it itself; taking
> + * q->limits_lock here would take it a second time.
> + */
> + if (caller_lim) {
> + *caller_lim = lim;
> + return 0;
> + }
> +
> return queue_limits_set(mddev->gendisk->queue, &lim);
> }
>
[...]
I'd propose the same changes as I suggested in patch 1/8, for
raid5_set_limits(), raid10_set_queue_limits(), raid1_set_limits()
and raid0_set_limits().
In particular, I think these functions should always operate on
a caller-provided struct queue_limits and only prepare/update the
limits, without deciding whether to commit them. The caller should
own the limits update and commit it as appropriate for its locking
context.
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-09-11 10:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-10 8:11 [PATCH v2 0/8] md: don't wait for q->limits_lock while md holds back I/O Jack Wang
2026-09-10 8:11 ` [PATCH v2 1/8] md: pass a queue_limits down to ->hot_add_disk() Jack Wang
2026-09-11 10:46 ` Nilay Shroff
2026-09-10 8:11 ` [PATCH v2 2/8] md: don't wait for q->limits_lock in check_sb_changes() Jack Wang
2026-09-10 8:11 ` [PATCH v2 3/8] md: pass a queue_limits through the rdev sysfs stores Jack Wang
2026-09-10 8:11 ` [PATCH v2 4/8] md: defer the io_opt update out of the sync thread Jack Wang
2026-09-10 8:11 ` [PATCH v2 5/8] md: take q->limits_lock before locking and suspending the array Jack Wang
2026-09-10 8:11 ` [PATCH v2 6/8] md: pass a queue_limits through ->run() Jack Wang
2026-09-11 10:54 ` Nilay Shroff
2026-09-10 8:11 ` [PATCH v2 7/8] md: open new legs before locking the array Jack Wang
2026-09-10 8:11 ` [PATCH v2 8/8] md: link a new leg's holder " Jack Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox