Re: [PATCH v2 6/8] md: pass a queue_limits through ->run()
From: sashiko-bot@kernel.org
Date: 2026-09-10 08:49:47
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Array starts successfully but fails to apply its queue limits if queue_limits_commit_update() returns an error, leaving a running array with inconsistent block limits. -- commit a296ef119cee93cfa123eae52144130e9c5df88f Author: Jack Wang [off-list ref] md: pass a queue_limits through ->run() This commit delegates the application of queue limits to the top-level callers that start the array. This is done to prevent lock inversion between the reconfig_mutex and q->limits_lock. Personalities now pass the limits up to be committed by the caller.
quoted hunk ↗ jump to hunk
diff --git a/drivers/md/md.c b/drivers/md/md.c --- a/drivers/md/md.c +++ b/drivers/md/md.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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);
[Severity: High] If queue_limits_commit_update() fails here, does it leave the array in an inconsistent state? The limits are committed after do_md_run() has already activated the array. If the commit fails, an error is returned to userspace, but the running array isn't stopped or rolled back. Can this lead to block layer warnings, driver crashes, or data corruption if I/O requests exceed the capabilities of the underlying devices? [ ... ]
quoted hunk ↗ jump to hunk
@@ -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;
[Severity: High] Similarly, when do_md_run() successfully starts the array here, the limits are committed later in the function (under the unlock label). If that commit fails, the array will remain active with unvalidated or default limits. Should there be a rollback mechanism (e.g., calling do_md_stop()) if the limits cannot be successfully applied after the array is started?
quoted hunk ↗ jump to hunk
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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); }
[Severity: High] By unconditionally returning 0 when caller_lim is provided, are we deferring all limit validation until after the array is fully activated? Because the personality's ->run() method copies the limits but does not validate them with the block layer, the first point of failure becomes queue_limits_commit_update() in the caller. Since the caller currently lacks rollback handling, this guarantees the array is left running with invalid limits if the configuration is rejected by blk_validate_limits(). -- Sashiko AI review · https://sashiko.dev/#/patchset/20260910081114.1605746-1-jinpu.wang@ionos.com?part=6