Re: [PATCH 3/6] md: don't wait for q->limits_lock in check_sb_changes()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The return value of queue_limits_commit_update() is ignored, allowing a disk to be permanently added to the array even if its queue limits are rejected.
--

commit 9dfb5955d3bac2bdadfd9671f084d71c343b76e2
Author: Jack Wang <jinpu.wang@cloud.ionos.com>

md: don't wait for q->limits_lock in check_sb_changes()

This patch updates check_sb_changes() to use a non-blocking trylock when
stacking queue limits for a newly added spare. If the lock is acquired, the
limits are stacked inline to avoid blocking on I/O while holding the
reconfig_mutex.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -10885,7 +10900,24 @@ 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 don't wait for
> +				 * q->limits_lock; MD_RECOVERY_NEEDED below
> +				 * leaves a skipped add to md_start_sync().
> +				 */
> +				if (mddev_stack_limits_trylock(mddev, &lim)) {
> +					struct request_queue *q =
> +						mddev->gendisk->queue;
> +
> +					ret = remove_and_add_spares(mddev,
> +								    rdev2, &lim);
> +					queue_limits_commit_update(q, &lim);

[Severity: High]
Is it safe to ignore the return value of queue_limits_commit_update() here?

If blk_validate_limits() fails inside queue_limits_commit_update(), it returns
an error and the limits are not applied. Because the return value is ignored
here, the disk could still be permanently added to the array even if its queue
limits (such as incompatible integrity profiles or alignments) were rejected.

Previously, a failure when committing limits would abort the disk addition
(for example, in mddev_stack_new_rdev()). Does splitting the limit stacking
from the commit and dropping the return value bypass this safety check?

> +				} else if (mddev_is_dm(mddev)) {
> +					ret = remove_and_add_spares(mddev,
> +								    rdev2, NULL);
> +				} else {
> +					ret = remove_spares(mddev, rdev2);
> +				}
>  				pr_info("Activated spare: %pg\n",
>  					rdev2->bdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907133929.1081540-1-jinpu.wang@ionos.com?part=3