Re: [PATCH 2/2] vhost-scsi: reject invalid max_io_vqs module parameter

On 2026-08-01 2:55 AM, Michael S. Tsirkin wrote:
> 
> On Fri, Jul 31, 2026 at 11:59:21PM -0700, Dongli Zhang wrote:
>> 
>> 
>> On 2026-07-31 5:00 PM, Michael S. Tsirkin wrote:
>> > On Fri, Jul 31, 2026 at 03:10:24PM -0700, Dongli Zhang wrote:
>> >> max_io_vqs is currently validated only when a vhost-scsi device is opened.
>> >> This allows sysfs to show values larger than the driver will actually use,
>> >> e.g. writing 2048 succeeds even though vhost_scsi_open() later clamps it to
>> >> VHOST_SCSI_MAX_IO_VQ. This makes the sysfs value differ from the value that
>> >> will actually be used.
>> >> 
>> >> hv# echo 2048 > /sys/module/vhost_scsi/parameters/max_io_vqs
>> >> 
>> >> hv# cat /sys/module/vhost_scsi/parameters/max_io_vqs
>> >> 2048
>> >> 
>> >> [  315.630495] Invalid max_io_vqs of 2048. Using 1024.
>> >> 
>> >> Use a module parameter callback to reject out-of-range updates when the
>> >> parameter is written. This keeps the visible parameter value consistent
>> >> with the effective queue limit.
>> >> 
>> >> With the parameter value kept in range, remove the duplicate validation
>> >> from vhost_scsi_open().
>> >> 
>> >> Invalid values now fail with -EINVAL at write/load time instead of being
>> >> accepted and silently adjusted later when the device is opened.
>> >> 
>> >> Signed-off-by: Dongli Zhang <[email protected]>
>> > 
>> > why should we change userspace visible behaviour now?
>> > 
>> 
>> Everything started with PATCH 1/1.
>> 
>> The original purpose was to resolve the memory allocation failure addressed by
>> PATCH 1/1.
>> 
>> To validate the bugfix, I increased max_io_vqs to see what would happen.
>> 
>> To my surprise, I was able to increase max_io_vqs to 2048 even though
>> VHOST_SCSI_MAX_IO_VQ is 1024.
>> 
>> hv# echo 2048 > /sys/module/vhost_scsi/parameters/max_io_vqs
>> 
>> hv# cat /sys/module/vhost_scsi/parameters/max_io_vqs
>> 2048
>> 
>> However, a value of 2048 does not work when creating a guest VM with vhost-scsi
>> because vhost_scsi_open() fails.
>> 
>> This behavior is quite confusing.
>> 
>> Therefore, PATCH 2/2 was proposed to avoid confusing users. If 2048 is an
>> invalid value, then setting max_io_vqs to 2048 should fail explicitly.
>> 
>> Is this really a "userspace-visible behavior change"? Without PATCH 1/1,
>> increasing the default value to 256 could already fail because of memory
>> allocation. Perhaps this is a good opportunity to define the expected behavior
>> of max_io_vqs before more users start increasing its value.
>> 
>> This also makes the behavior of max_io_vqs consistent with inline_sg_cnt. For
>> module parameters, it may be better to perform sanity checks when the parameter
>> is configured rather than at runtime.
>> 
>> Thank you very much!
>> 
>> Dongli Zhang
> 
> 
> There's no telling if someone had a large value configured and now the
> module will start failing for them. I can accept making the read return
> the actual value, if that helps.
> 

I am not sure whether there is already any usage of max_io_vqs values greater
than 1024. In my testing, with the other parameters left at their default
values, even increasing max_io_vqs to 256 can hit a memory allocation failure
with the current kmalloc-based vq array allocation.

I will not reject out-of-range values in v2. Instead, the setter will accept
them, clamp to the supported range, and store the effective value, so reading
max_io_vqs returns what vhost-scsi will actually use.

Thank you very much!

Dongli Zhang