Hi,
On 2021-02-02 15:19:08 -0500, Melanie Plageman wrote:
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 2e4fa77445fd..d72ab6daf9ae 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -378,10 +378,14 @@ static u32 max_outstanding_req_per_channel;
> static int storvsc_change_queue_depth(struct scsi_device *sdev, int queue_depth);
>
> static int storvsc_vcpus_per_sub_channel = 4;
> +static int storvsc_nr_hw_queues = -1;
>
> module_param(storvsc_ringbuffer_size, int, S_IRUGO);
> MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
>
> +module_param(storvsc_nr_hw_queues, int, S_IRUGO);
> +MODULE_PARM_DESC(storvsc_nr_hw_queues, "Number of hardware queues");
Would be nice if the parameter could be changed in a running
system. Obviously that's only going to affect devices that'll be
attached in the future (or detached and reattached), but that's still
nicer than not being able to change at all.
> @@ -2155,6 +2163,7 @@ static struct fc_function_template fc_transport_functions = {
> static int __init storvsc_drv_init(void)
> {
> int ret;
> + int ncpus = num_present_cpus();
>
> /*
> * Divide the ring buffer data size (which is 1 page less
> @@ -2169,6 +2178,14 @@ static int __init storvsc_drv_init(void)
> vmscsi_size_delta,
> sizeof(u64)));
>
> + if (storvsc_nr_hw_queues > ncpus || storvsc_nr_hw_queues == 0 ||
> + storvsc_nr_hw_queues < -1) {
> + printk(KERN_ERR "storvsc: Invalid storvsc_nr_hw_queues value of %d.
> + Valid values include -1 and 1-%d.\n",
> + storvsc_nr_hw_queues, ncpus);
> + return -EINVAL;
> + }
> +
I have a mild preference for just capping the effective value to
num_present_cpus(), rather than erroring out. Would allow you to
e.g. cap the number of queues to 4, with the same param specified on
smaller and bigger systems. Perhaps renaming it to max_hw_queues would
make that more obvious?
Greetings,
Andres Freund
