Re: [PATCH v2] block, scsi: sd_zbc: Respect bio vector limits for report zones buffer

[Damien Le Moal <dlemoal@kernel.org>]

On 5/3/25 04:35, Steve Siwinski wrote:
> The report zones buffer size is currently limited by the HBA's
> maximum segment count to ensure the buffer can be mapped. However,
> the block layer further limits the number of iovec entries to
> 1024 when allocating a bio.
> 
> To avoid allocation of buffers too large to be mapped, further
> restrict the maximum buffer size to BIO_MAX_INLINE_VECS.
> 
> Replace the UIO_MAXIOV symbolic name with the more contextually
> appropriate BIO_MAX_INLINE_VECS.
> 
> Signed-off-by: Steve Siwinski <ssiwinski@atto.com>

This needs a "Fixes" tag:

Fixes: b091ac616846 ("sd_zbc: Fix report zones buffer allocation")
Cc: stable@vger.kernel.org

> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index 7a447ff600d2..a5364fdc2824 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -180,12 +180,15 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
>  	 * Furthermore, since the report zone command cannot be split, make
>  	 * sure that the allocated buffer can always be mapped by limiting the
>  	 * number of pages allocated to the HBA max segments limit.
> +	 * Since max segments can be larger than the max inline bio vectors,
> +	 * further limit the allocated buffer to BIO_MAX_INLINE_VECS.
>  	 */
>  	nr_zones = min(nr_zones, sdkp->zone_info.nr_zones);
>  	bufsize = roundup((nr_zones + 1) * 64, SECTOR_SIZE);
>  	bufsize = min_t(size_t, bufsize,
>  			queue_max_hw_sectors(q) << SECTOR_SHIFT);
>  	bufsize = min_t(size_t, bufsize, queue_max_segments(q) << PAGE_SHIFT);
> +	bufsize = min_t(size_t, bufsize, BIO_MAX_INLINE_VECS << PAGE_SHIFT);

I would prefer something like:

	unsigned int max_segments;
	...

	max_segments = min(BIO_MAX_INLINE_VECS, queue_max_segments(q));
	bufsize = min_t(size_t, bufsize, max_segments << PAGE_SHIFT);

>  
>  	while (bufsize >= SECTOR_SIZE) {
>  		buf = kvzalloc(bufsize, GFP_KERNEL | __GFP_NORETRY);
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index cafc7c215de8..7cf9506a6c36 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -11,6 +11,8 @@
>  #include <linux/uio.h>
>  
>  #define BIO_MAX_VECS		256U
> +/* BIO_MAX_INLINE_VECS must be at most the size of UIO_MAXIOV */
> +#define BIO_MAX_INLINE_VECS	1024

This should be:

#define BIO_MAX_INLINE_VECS	UIO_MAXIOV

so that we do not end up with inconsistencies with what user space sees as the
maximum value.

>  
>  struct queue_limits;
>  


-- 
Damien Le Moal
Western Digital Research