Re: [PATCH v2 11/11] md: fix abnormal io_opt from member disks

在 2026/1/3 23:45, Yu Kuai 写道:
> It's reported that mtp3sas can report abnormal io_opt, for consequence,
> md array will end up with abnormal io_opt as well, due to the
> lcm_not_zero() from blk_stack_limits().
> 
> Some personalities will configure optimal IO size, and it's indicate that
> users can get the best IO bandwidth if they issue IO with this size, and
> we don't want io_opt to be covered by member disks with abnormal io_opt.
> 
> Fix this problem by adding a new mddev flags MD_STACK_IO_OPT to indicate
> that io_opt configured by personalities is preferred over member disks
> or not.
> 
> Reported-by: Filippo Giunchedi <[email protected]>
> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1121006
> Reported-by: Coly Li <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Yu Kuai <[email protected]>
> ---
>   drivers/md/md.c     | 35 ++++++++++++++++++++++++++++++++++-
>   drivers/md/md.h     |  5 ++++-
>   drivers/md/raid1.c  |  2 +-
>   drivers/md/raid10.c |  4 ++--
>   4 files changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 7292aedef01b..b46b05cd28fb 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6192,11 +6192,17 @@ static const struct kobj_type md_ktype = {
>   
>   int mdp_major = 0;
>   
> +static bool rdev_is_mddev(struct md_rdev *rdev)
> +{
> +	return rdev->bdev->bd_disk->fops == &md_fops;
> +}
> +
>   /* stack the limit for all rdevs into lim */
>   int mddev_stack_rdev_limits(struct mddev *mddev, struct queue_limits *lim,
>   		unsigned int flags)
>   {
>   	struct md_rdev *rdev;
> +	unsigned int io_opt = lim->io_opt;
>   
>   	rdev_for_each(rdev, mddev) {
>   		queue_limits_stack_bdev(lim, rdev->bdev, rdev->data_offset,
> @@ -6204,6 +6210,9 @@ int mddev_stack_rdev_limits(struct mddev *mddev, struct queue_limits *lim,
>   		if ((flags & MDDEV_STACK_INTEGRITY) &&
>   		    !queue_limits_stack_integrity_bdev(lim, rdev->bdev))
>   			return -EINVAL;
> +
> +		if (rdev_is_mddev(rdev))
> +			set_bit(MD_STACK_IO_OPT, &mddev->flags);
>   	}
>   
>   	/*
> @@ -6217,14 +6226,24 @@ int mddev_stack_rdev_limits(struct mddev *mddev, struct queue_limits *lim,
>   	}
>   	mddev->logical_block_size = lim->logical_block_size;
>   
> +	/*
> +	 * If all member disks are not mdraid array, and the personality
> +	 * already configures io_opt, keep this io_opt and ignore io_opt from
> +	 * member disks.
> +	 */
> +	if (!test_bit(MD_STACK_IO_OPT, &mddev->flags) && io_opt)
> +		lim->io_opt = io_opt;
> +
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(mddev_stack_rdev_limits);
>   
>   /* apply the extra stacking limits from a new rdev into mddev */
> -int mddev_stack_new_rdev(struct mddev *mddev, struct md_rdev *rdev)
> +int mddev_stack_new_rdev(struct mddev *mddev, struct md_rdev *rdev,
> +			 bool io_opt_configured)
>   {
>   	struct queue_limits lim;
> +	unsigned int io_opt = 0;
>   
>   	if (mddev_is_dm(mddev))
>   		return 0;
> @@ -6237,6 +6256,18 @@ int mddev_stack_new_rdev(struct mddev *mddev, struct md_rdev *rdev)
>   	}
>   
>   	lim = queue_limits_start_update(mddev->gendisk->queue);
> +
> +	/*
> +	 * Keep the old io_opt if no member disks are from md array, and
> +	 * the personality configure it's own io_opt.
> +	 */
> +	if (!test_bit(MD_STACK_IO_OPT, &mddev->flags)) {
> +		if (rdev_is_mddev(rdev))
> +			set_bit(MD_STACK_IO_OPT, &mddev->flags);
> +		else if (io_opt_configured)
> +			io_opt = lim.io_opt;
> +	}
> +
>   	queue_limits_stack_bdev(&lim, rdev->bdev, rdev->data_offset,
>   				mddev->gendisk->disk_name);
>   

This looks problematic. When member disks are a mix of RAID and mtp3sas
—— even though this usage scenario is unlikely —— the opt parameter of
mtp3sas will still affect the entire mddev.

-- 
Thanks,
Nan