On Thu, Jul 01, 2021 at 06:58:35AM +0000, Long Li wrote: > > Subject: Re: [Patch v2 2/3] Drivers: hv: add Azure Blob driver > > > + > > > +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. > 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
