Re: [PATCH] scsi: sd_zbc: Limit the report zones buffer size to UIO_MAXIOV

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

On 4/25/25 00:33, Siwinski, Steve wrote:
> My issue is not with passthough report zones.
> 
> The report zones command is failing on driver load and causing the drive 
> to fail to appear as a block device. If queue_max_segments is set to a 
> value over 1024, then nr_vecs in bio_alloc() will be greater than 
> UIO_MAXIOV and bio_alloc() will return NULL.

OK... A remainder about the path:

sd_zbc_do_report_zones() -> scsi_execute_cmd() -> blk_rq_map_kern() ->
bio_map_kern() -> bio_kmalloc()

and the fact that bio_kmalloc() does not allow more than UIO_MAXIOV segments
would have made things clear from the beginning. I had to look it up again to
understand why UIO_MAXIOV matters.

> This causes the error.
> ```
> sd 8:0:0:0: [sdb] REPORT ZONES start lba 0 failed
> sd 8:0:0:0: [sdb] REPORT ZONES: Result: hostbyte=0xff driverbyte=DRIVER_OK
> sdb: failed to revalidate zones
> ```
> 
> You can reproduce this by setting the max_sgl_entries parameter to 2k or 
> greater in the mpt3sas driver. Other drivers can also reproduce this 
> behavior.

Well, I think that the problem you uncovered here is a lot more fundamental than
just ZBC report zones. If the drive has a queue_max_segments() value larger than
UIO_MAXIOV, any attempt to map a large buffer for any command (e.g. a read) will
also fail. So this limit inconsistency seems wrong...

Christoph ? Since you were touching the vmalloc-ed BIO mapping code, do you have
any idea about this ? The quick and dirty fix would be to do:

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 7a447ff600d2..3cb897b25878 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -169,6 +169,7 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
                                        unsigned int nr_zones, size_t *buflen)
 {
        struct request_queue *q = sdkp->disk->queue;
+       size_t max_segs;
        size_t bufsize;
        void *buf;

@@ -185,7 +186,8 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
        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);
+       max_segs = min(queue_max_segments(q), UIO_MAXIOV);
+       bufsize = min_t(size_t, bufsize, max_segs << PAGE_SHIFT);

        while (bufsize >= SECTOR_SIZE) {
                buf = kvzalloc(bufsize, GFP_KERNEL | __GFP_NORETRY);


But that feels wrong...

-- 
Damien Le Moal
Western Digital Research