Subject: Re: [Patch v2 2/3] Drivers: hv: add Azure Blob driver

On Thu, Jul 01, 2021 at 06:58:35AM +0000, Long Li wrote:

quoted

quoted

Subject: Re: [Patch v2 2/3] Drivers: hv: add Azure Blob driver

quoted

+
+static struct az_blob_device az_blob_dev;
+
+static int az_blob_ringbuffer_size = (128 * 1024);
+module_param(az_blob_ringbuffer_size, int, 0444);
+MODULE_PARM_DESC(az_blob_ringbuffer_size, "Ring buffer size
+(bytes)");

This is NOT the 1990's, please do not create new module parameters.
Just make this work properly for everyone.

The default value is chosen so that it works best for most workloads while

not taking too much system resources. It should work out of box for nearly all
customers.

Please wrap your email lines :(

Anyway, great, it will work for everyone, no need for this to be changed.
Then just drop it.

quoted

But what we see is that from time to time, some customers still want to

change those values to work best for their workloads. It's hard to predict their
workload. They have to recompile the kernel if there is no module parameter
to do it. So the intent of this module parameter is that if the default value
works for you, don't mess up with it.

Can you not just determine at runtime that these workloads are overflowing
the buffer and increase the size of it?  That would be best otherwise you are
forced to try to deal with a module parameter that is for code, not for devices
(which is why we do not like module parameters for drivers.)

Please remove this for now and if you _REALLY_ need it in the future, make it
auto-tunable.  Or if that is somehow impossible, then make it a per-device
option, through something like sysfs so that it will work correctly per-device.

thanks,

greg k-h

I got your point, will be removing this. 

Thanks,
Long