LinuxLists.cc - [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

2022-10-12 12:49:16

Subject: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

This patch enables support for slower vmbus channels, which consist
of following 3 changes :
1. Support for hypercalls
2. Module params for recv/send buffer sizes
3. Module param for custom ring buffer sizes

Signed-off-by: Saurabh Sengar <[email protected]>
---
drivers/uio/uio_hv_generic.c | 111 +++++++++++++++++++----------------
1 file changed, 62 insertions(+), 49 deletions(-)

diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index c08a6cfd119f..11a735fbb4f1 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -36,10 +36,21 @@
#define DRIVER_AUTHOR "Stephen Hemminger <sthemmin at microsoft.com>"
#define DRIVER_DESC "Generic UIO driver for VMBus devices"

-#define HV_RING_SIZE 512 /* pages */
+#define DEFAULT_HV_RING_SIZE VMBUS_RING_SIZE(511 * HV_HYP_PAGE_SIZE)
#define SEND_BUFFER_SIZE (16 * 1024 * 1024)
#define RECV_BUFFER_SIZE (31 * 1024 * 1024)

+static size_t recv_buf_size = RECV_BUFFER_SIZE;
+module_param(recv_buf_size, ulong, 0644);
+MODULE_PARM_DESC(recv_buf_size, "receive buffer size in bytes");
+
+static size_t send_buf_size = SEND_BUFFER_SIZE;
+module_param(send_buf_size, ulong, 0644);
+MODULE_PARM_DESC(send_buf_size, "send buffer size in bytes");
+
+static size_t ring_size = DEFAULT_HV_RING_SIZE;
+module_param(ring_size, ulong, 0644);
+MODULE_PARM_DESC(ring_size, "primary channel ring buffer size in bytes");
/*
* List of resources to be mapped to user space
* can be extended up to MAX_UIO_MAPS(5) items
@@ -84,6 +95,9 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
virt_mb();

+ if (!dev->channel->offermsg.monitor_allocated && irq_state)
+ vmbus_setevent(dev->channel);
+
return 0;
}

@@ -143,7 +157,7 @@ static const struct bin_attribute ring_buffer_bin_attr = {
.name = "ring",
.mode = 0600,
},
- .size = 2 * HV_RING_SIZE * PAGE_SIZE,
+ .size = 2 * DEFAULT_HV_RING_SIZE,
.mmap = hv_uio_ring_mmap,
};

@@ -153,7 +167,7 @@ hv_uio_new_channel(struct vmbus_channel *new_sc)
{
struct hv_device *hv_dev = new_sc->primary_channel->device_obj;
struct device *device = &hv_dev->device;
- const size_t ring_bytes = HV_RING_SIZE * PAGE_SIZE;
+ const size_t ring_bytes = DEFAULT_HV_RING_SIZE;
int ret;

/* Create host communication ring */
@@ -239,18 +253,13 @@ hv_uio_probe(struct hv_device *dev,
void *ring_buffer;
int ret;

- /* Communicating with host has to be via shared memory not hypercall */
- if (!channel->offermsg.monitor_allocated) {
- dev_err(&dev->device, "vmbus channel requires hypercall\n");
- return -ENOTSUPP;
- }
+ dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);

pdata = devm_kzalloc(&dev->device, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
return -ENOMEM;

- ret = vmbus_alloc_ring(channel, HV_RING_SIZE * PAGE_SIZE,
- HV_RING_SIZE * PAGE_SIZE);
+ ret = vmbus_alloc_ring(channel, ring_size, ring_size);
if (ret)
return ret;

@@ -286,49 +295,53 @@ hv_uio_probe(struct hv_device *dev,
pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;

- pdata->recv_buf = vzalloc(RECV_BUFFER_SIZE);
- if (pdata->recv_buf == NULL) {
- ret = -ENOMEM;
- goto fail_free_ring;
- }
-
- ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
- RECV_BUFFER_SIZE, &pdata->recv_gpadl);
- if (ret) {
- vfree(pdata->recv_buf);
- goto fail_close;
- }
-
- /* put Global Physical Address Label in name */
- snprintf(pdata->recv_name, sizeof(pdata->recv_name),
- "recv:%u", pdata->recv_gpadl.gpadl_handle);
- pdata->info.mem[RECV_BUF_MAP].name = pdata->recv_name;
- pdata->info.mem[RECV_BUF_MAP].addr
- = (uintptr_t)pdata->recv_buf;
- pdata->info.mem[RECV_BUF_MAP].size = RECV_BUFFER_SIZE;
- pdata->info.mem[RECV_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
-
- pdata->send_buf = vzalloc(SEND_BUFFER_SIZE);
- if (pdata->send_buf == NULL) {
- ret = -ENOMEM;
- goto fail_close;
+ if (recv_buf_size) {
+ dev_dbg(&dev->device, "recv buffer allocation");
+ pdata->recv_buf = vzalloc(recv_buf_size);
+ if (!pdata->recv_buf) {
+ ret = -ENOMEM;
+ goto fail_free_ring;
+ }
+
+ ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
+ recv_buf_size, &pdata->recv_gpadl);
+ if (ret) {
+ vfree(pdata->recv_buf);
+ goto fail_close;
+ }
+
+ /* put Global Physical Address Label in name */
+ snprintf(pdata->recv_name, sizeof(pdata->recv_name),
+ "recv:%u", pdata->recv_gpadl.gpadl_handle);
+ pdata->info.mem[RECV_BUF_MAP].name = pdata->recv_name;
+ pdata->info.mem[RECV_BUF_MAP].addr = (uintptr_t)pdata->recv_buf;
+ pdata->info.mem[RECV_BUF_MAP].size = recv_buf_size;
+ pdata->info.mem[RECV_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
}

- ret = vmbus_establish_gpadl(channel, pdata->send_buf,
- SEND_BUFFER_SIZE, &pdata->send_gpadl);
- if (ret) {
- vfree(pdata->send_buf);
- goto fail_close;
+ if (send_buf_size) {
+ dev_dbg(&dev->device, "send buffer allocation");
+ pdata->send_buf = vzalloc(send_buf_size);
+ if (!pdata->send_buf) {
+ ret = -ENOMEM;
+ goto fail_close;
+ }
+
+ ret = vmbus_establish_gpadl(channel, pdata->send_buf,
+ send_buf_size, &pdata->send_gpadl);
+ if (ret) {
+ vfree(pdata->send_buf);
+ goto fail_close;
+ }
+
+ snprintf(pdata->send_name, sizeof(pdata->send_name),
+ "send:%u", pdata->send_gpadl.gpadl_handle);
+ pdata->info.mem[SEND_BUF_MAP].name = pdata->send_name;
+ pdata->info.mem[SEND_BUF_MAP].addr = (uintptr_t)pdata->send_buf;
+ pdata->info.mem[SEND_BUF_MAP].size = send_buf_size;
+ pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
}

- snprintf(pdata->send_name, sizeof(pdata->send_name),
- "send:%u", pdata->send_gpadl.gpadl_handle);
- pdata->info.mem[SEND_BUF_MAP].name = pdata->send_name;
- pdata->info.mem[SEND_BUF_MAP].addr
- = (uintptr_t)pdata->send_buf;
- pdata->info.mem[SEND_BUF_MAP].size = SEND_BUFFER_SIZE;
- pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
-
pdata->info.priv = pdata;
pdata->device = dev;

--
2.34.1

2022-10-12 16:02:58

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

On Wed, Oct 12, 2022 at 05:52:05PM +0200, Greg KH wrote:
> On Wed, Oct 12, 2022 at 04:56:46AM -0700, Saurabh Sengar wrote:
> > This patch enables support for slower vmbus channels, which consist
> > of following 3 changes :
> > 1. Support for hypercalls
> > 2. Module params for recv/send buffer sizes
> > 3. Module param for custom ring buffer sizes
>
> Even if this all was ok, you are doing 3 things all in one change,
> that's not allowed at all, you all know this.
>
> Anyway, no new module parameters, this is not the 1990's, we have much
> better ways to do this properly (hint, module parameters modify code,
> you want to modify the options of a specific device.)

Also, you give no good reason for why this is needed at all, nor how
anyone would use these options and why they would need to.

The kernel should "just work" and not require manual intervention by a
user. Dynamically fix this based on the device, do NOT force a user to
have to attempt to "tune" anything, that will never work properly over
time, AND you are being lazy and forcing each individual user to do the
work, making more effort needed overall than just doing it properly in
the kernel.

thanks,

greg k-h

2022-10-12 16:50:44

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

On Wed, Oct 12, 2022 at 04:56:46AM -0700, Saurabh Sengar wrote:
> This patch enables support for slower vmbus channels, which consist
> of following 3 changes :
> 1. Support for hypercalls
> 2. Module params for recv/send buffer sizes
> 3. Module param for custom ring buffer sizes

Even if this all was ok, you are doing 3 things all in one change,
that's not allowed at all, you all know this.

Anyway, no new module parameters, this is not the 1990's, we have much
better ways to do this properly (hint, module parameters modify code,
you want to modify the options of a specific device.)

greg k-h

2022-10-12 18:15:31

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

Hi Saurabh,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on linus/master v6.0 next-20221012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Saurabh-Sengar/uio_hv_generic-Enable-support-for-slower-vmbus-device-channels/20221012-195731
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 49da070062390094112b423ba443ea193527b2e4
config: i386-randconfig-a005
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/e7d62290b320f0f50c4f09b8f869b9049ef2c2bd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Saurabh-Sengar/uio_hv_generic-Enable-support-for-slower-vmbus-device-channels/20221012-195731
git checkout e7d62290b320f0f50c4f09b8f869b9049ef2c2bd
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/uio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from include/linux/module.h:22,
from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from drivers/uio/uio_hv_generic.c:22:
drivers/uio/uio_hv_generic.c: In function '__check_recv_buf_size':
include/linux/moduleparam.h:150:34: error: returning 'size_t *' {aka 'unsigned int *'} from a function with incompatible return type 'long unsigned int *' [-Werror=incompatible-pointer-types]
150 | param_check_##type(name, &(value)); \
| ^
include/linux/moduleparam.h:409:75: note: in definition of macro '__param_check'
409 | static inline type __always_unused *__check_##name(void) { return(p); }
| ^
include/linux/moduleparam.h:150:9: note: in expansion of macro 'param_check_ulong'
150 | param_check_##type(name, &(value)); \
| ^~~~~~~~~~~~
include/linux/moduleparam.h:127:9: note: in expansion of macro 'module_param_named'
127 | module_param_named(name, name, type, perm)
| ^~~~~~~~~~~~~~~~~~
drivers/uio/uio_hv_generic.c:44:1: note: in expansion of macro 'module_param'
44 | module_param(recv_buf_size, ulong, 0644);
| ^~~~~~~~~~~~
drivers/uio/uio_hv_generic.c: In function '__check_send_buf_size':
include/linux/moduleparam.h:150:34: error: returning 'size_t *' {aka 'unsigned int *'} from a function with incompatible return type 'long unsigned int *' [-Werror=incompatible-pointer-types]
150 | param_check_##type(name, &(value)); \
| ^
include/linux/moduleparam.h:409:75: note: in definition of macro '__param_check'
409 | static inline type __always_unused *__check_##name(void) { return(p); }
| ^
include/linux/moduleparam.h:150:9: note: in expansion of macro 'param_check_ulong'
150 | param_check_##type(name, &(value)); \
| ^~~~~~~~~~~~
include/linux/moduleparam.h:127:9: note: in expansion of macro 'module_param_named'
127 | module_param_named(name, name, type, perm)
| ^~~~~~~~~~~~~~~~~~
drivers/uio/uio_hv_generic.c:48:1: note: in expansion of macro 'module_param'
48 | module_param(send_buf_size, ulong, 0644);
| ^~~~~~~~~~~~
drivers/uio/uio_hv_generic.c: In function '__check_ring_size':
include/linux/moduleparam.h:150:34: error: returning 'size_t *' {aka 'unsigned int *'} from a function with incompatible return type 'long unsigned int *' [-Werror=incompatible-pointer-types]
150 | param_check_##type(name, &(value)); \
| ^
include/linux/moduleparam.h:409:75: note: in definition of macro '__param_check'
409 | static inline type __always_unused *__check_##name(void) { return(p); }
| ^
include/linux/moduleparam.h:150:9: note: in expansion of macro 'param_check_ulong'
150 | param_check_##type(name, &(value)); \
| ^~~~~~~~~~~~
include/linux/moduleparam.h:127:9: note: in expansion of macro 'module_param_named'
127 | module_param_named(name, name, type, perm)
| ^~~~~~~~~~~~~~~~~~
drivers/uio/uio_hv_generic.c:52:1: note: in expansion of macro 'module_param'
52 | module_param(ring_size, ulong, 0644);
| ^~~~~~~~~~~~
In file included from include/linux/printk.h:566,
from include/linux/kernel.h:29,
from arch/x86/include/asm/percpu.h:27,
from arch/x86/include/asm/current.h:6,
from include/linux/sched.h:12,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from drivers/uio/uio_hv_generic.c:22:
drivers/uio/uio_hv_generic.c: In function 'hv_uio_probe':
>> drivers/uio/uio_hv_generic.c:256:31: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
256 | dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:223:29: note: in definition of macro '__dynamic_func_call_cls'
223 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:249:9: note: in expansion of macro '_dynamic_func_call_cls'
249 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:272:9: note: in expansion of macro '_dynamic_func_call'
272 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:30: note: in expansion of macro 'dev_fmt'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/uio/uio_hv_generic.c:256:9: note: in expansion of macro 'dev_dbg'
256 | dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
| ^~~~~~~
drivers/uio/uio_hv_generic.c:256:62: note: format string is defined here
256 | dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
| ~~^
| |
| long unsigned int
| %x
cc1: some warnings being treated as errors

vim +256 drivers/uio/uio_hv_generic.c

246
247 static int
248 hv_uio_probe(struct hv_device *dev,
249 const struct hv_vmbus_device_id *dev_id)
250 {
251 struct vmbus_channel *channel = dev->channel;
252 struct hv_uio_private_data *pdata;
253 void *ring_buffer;
254 int ret;
255
> 256 dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
257
258 pdata = devm_kzalloc(&dev->device, sizeof(*pdata), GFP_KERNEL);
259 if (!pdata)
260 return -ENOMEM;
261
262 ret = vmbus_alloc_ring(channel, ring_size, ring_size);
263 if (ret)
264 return ret;
265
266 set_channel_read_mode(channel, HV_CALL_ISR);
267
268 /* Fill general uio info */
269 pdata->info.name = "uio_hv_generic";
270 pdata->info.version = DRIVER_VERSION;
271 pdata->info.irqcontrol = hv_uio_irqcontrol;
272 pdata->info.open = hv_uio_open;
273 pdata->info.release = hv_uio_release;
274 pdata->info.irq = UIO_IRQ_CUSTOM;
275 atomic_set(&pdata->refcnt, 0);
276
277 /* mem resources */
278 pdata->info.mem[TXRX_RING_MAP].name = "txrx_rings";
279 ring_buffer = page_address(channel->ringbuffer_page);
280 pdata->info.mem[TXRX_RING_MAP].addr
281 = (uintptr_t)virt_to_phys(ring_buffer);
282 pdata->info.mem[TXRX_RING_MAP].size
283 = channel->ringbuffer_pagecount << PAGE_SHIFT;
284 pdata->info.mem[TXRX_RING_MAP].memtype = UIO_MEM_IOVA;
285
286 pdata->info.mem[INT_PAGE_MAP].name = "int_page";
287 pdata->info.mem[INT_PAGE_MAP].addr
288 = (uintptr_t)vmbus_connection.int_page;
289 pdata->info.mem[INT_PAGE_MAP].size = PAGE_SIZE;
290 pdata->info.mem[INT_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
291
292 pdata->info.mem[MON_PAGE_MAP].name = "monitor_page";
293 pdata->info.mem[MON_PAGE_MAP].addr
294 = (uintptr_t)vmbus_connection.monitor_pages[1];
295 pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
296 pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
297
298 if (recv_buf_size) {
299 dev_dbg(&dev->device, "recv buffer allocation");
300 pdata->recv_buf = vzalloc(recv_buf_size);
301 if (!pdata->recv_buf) {
302 ret = -ENOMEM;
303 goto fail_free_ring;
304 }
305
306 ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
307 recv_buf_size, &pdata->recv_gpadl);
308 if (ret) {
309 vfree(pdata->recv_buf);
310 goto fail_close;
311 }
312
313 /* put Global Physical Address Label in name */
314 snprintf(pdata->recv_name, sizeof(pdata->recv_name),
315 "recv:%u", pdata->recv_gpadl.gpadl_handle);
316 pdata->info.mem[RECV_BUF_MAP].name = pdata->recv_name;
317 pdata->info.mem[RECV_BUF_MAP].addr = (uintptr_t)pdata->recv_buf;
318 pdata->info.mem[RECV_BUF_MAP].size = recv_buf_size;
319 pdata->info.mem[RECV_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
320 }
321
322 if (send_buf_size) {
323 dev_dbg(&dev->device, "send buffer allocation");
324 pdata->send_buf = vzalloc(send_buf_size);
325 if (!pdata->send_buf) {
326 ret = -ENOMEM;
327 goto fail_close;
328 }
329
330 ret = vmbus_establish_gpadl(channel, pdata->send_buf,
331 send_buf_size, &pdata->send_gpadl);
332 if (ret) {
333 vfree(pdata->send_buf);
334 goto fail_close;
335 }
336
337 snprintf(pdata->send_name, sizeof(pdata->send_name),
338 "send:%u", pdata->send_gpadl.gpadl_handle);
339 pdata->info.mem[SEND_BUF_MAP].name = pdata->send_name;
340 pdata->info.mem[SEND_BUF_MAP].addr = (uintptr_t)pdata->send_buf;
341 pdata->info.mem[SEND_BUF_MAP].size = send_buf_size;
342 pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
343 }
344
345 pdata->info.priv = pdata;
346 pdata->device = dev;
347
348 ret = uio_register_device(&dev->device, &pdata->info);
349 if (ret) {
350 dev_err(&dev->device, "hv_uio register failed\n");
351 goto fail_close;
352 }
353
354 ret = sysfs_create_bin_file(&channel->kobj, &ring_buffer_bin_attr);
355 if (ret)
356 dev_notice(&dev->device,
357 "sysfs create ring bin file failed; %d\n", ret);
358
359 hv_set_drvdata(dev, pdata);
360
361 return 0;
362
363 fail_close:
364 hv_uio_cleanup(dev, pdata);
365 fail_free_ring:
366 vmbus_free_ring(dev->channel);
367
368 return ret;
369 }
370

--
0-DAY CI Kernel Test Service
https://01.org/lkp

Attachments:

(No filename) (12.44 kB)
config (157.18 kB)
Download all attachments

2022-10-12 18:57:00

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

Hi Saurabh,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on linus/master v6.0 next-20221012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Saurabh-Sengar/uio_hv_generic-Enable-support-for-slower-vmbus-device-channels/20221012-195731
config: i386-randconfig-a015
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/e7d62290b320f0f50c4f09b8f869b9049ef2c2bd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Saurabh-Sengar/uio_hv_generic-Enable-support-for-slower-vmbus-device-channels/20221012-195731
git checkout e7d62290b320f0f50c4f09b8f869b9049ef2c2bd
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/uio/ drivers/virtio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/uio/uio_hv_generic.c:44:1: error: incompatible pointer types returning 'size_t *' (aka 'unsigned int *') from a function with result type 'unsigned long *' [-Werror,-Wincompatible-pointer-types]
module_param(recv_buf_size, ulong, 0644);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:127:2: note: expanded from macro 'module_param'
module_param_named(name, name, type, perm)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:150:2: note: expanded from macro 'module_param_named'
param_check_##type(name, &(value)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<scratch space>:40:1: note: expanded from here
param_check_ulong
^
include/linux/moduleparam.h:446:36: note: expanded from macro 'param_check_ulong'
#define param_check_ulong(name, p) __param_check(name, p, unsigned long)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:409:67: note: expanded from macro '__param_check'
static inline type __always_unused *__check_##name(void) { return(p); }
^~~
drivers/uio/uio_hv_generic.c:48:1: error: incompatible pointer types returning 'size_t *' (aka 'unsigned int *') from a function with result type 'unsigned long *' [-Werror,-Wincompatible-pointer-types]
module_param(send_buf_size, ulong, 0644);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:127:2: note: expanded from macro 'module_param'
module_param_named(name, name, type, perm)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:150:2: note: expanded from macro 'module_param_named'
param_check_##type(name, &(value)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<scratch space>:59:1: note: expanded from here
param_check_ulong
^
include/linux/moduleparam.h:446:36: note: expanded from macro 'param_check_ulong'
#define param_check_ulong(name, p) __param_check(name, p, unsigned long)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:409:67: note: expanded from macro '__param_check'
static inline type __always_unused *__check_##name(void) { return(p); }
^~~
drivers/uio/uio_hv_generic.c:52:1: error: incompatible pointer types returning 'size_t *' (aka 'unsigned int *') from a function with result type 'unsigned long *' [-Werror,-Wincompatible-pointer-types]
module_param(ring_size, ulong, 0644);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:127:2: note: expanded from macro 'module_param'
module_param_named(name, name, type, perm)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:150:2: note: expanded from macro 'module_param_named'
param_check_##type(name, &(value)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<scratch space>:80:1: note: expanded from here
param_check_ulong
^
include/linux/moduleparam.h:446:36: note: expanded from macro 'param_check_ulong'
#define param_check_ulong(name, p) __param_check(name, p, unsigned long)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/moduleparam.h:409:67: note: expanded from macro '__param_check'
static inline type __always_unused *__check_##name(void) { return(p); }
^~~
>> drivers/uio/uio_hv_generic.c:256:59: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
~~~ ^~~~~~~~~
%zx
include/linux/dev_printk.h:163:47: note: expanded from macro 'dev_dbg'
dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:129:34: note: expanded from macro 'dev_printk'
_dev_printk(level, dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
1 warning and 3 errors generated.

vim +256 drivers/uio/uio_hv_generic.c

246
247 static int
248 hv_uio_probe(struct hv_device *dev,
249 const struct hv_vmbus_device_id *dev_id)
250 {
251 struct vmbus_channel *channel = dev->channel;
252 struct hv_uio_private_data *pdata;
253 void *ring_buffer;
254 int ret;
255
> 256 dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
257
258 pdata = devm_kzalloc(&dev->device, sizeof(*pdata), GFP_KERNEL);
259 if (!pdata)
260 return -ENOMEM;
261
262 ret = vmbus_alloc_ring(channel, ring_size, ring_size);
263 if (ret)
264 return ret;
265
266 set_channel_read_mode(channel, HV_CALL_ISR);
267
268 /* Fill general uio info */
269 pdata->info.name = "uio_hv_generic";
270 pdata->info.version = DRIVER_VERSION;
271 pdata->info.irqcontrol = hv_uio_irqcontrol;
272 pdata->info.open = hv_uio_open;
273 pdata->info.release = hv_uio_release;
274 pdata->info.irq = UIO_IRQ_CUSTOM;
275 atomic_set(&pdata->refcnt, 0);
276
277 /* mem resources */
278 pdata->info.mem[TXRX_RING_MAP].name = "txrx_rings";
279 ring_buffer = page_address(channel->ringbuffer_page);
280 pdata->info.mem[TXRX_RING_MAP].addr
281 = (uintptr_t)virt_to_phys(ring_buffer);
282 pdata->info.mem[TXRX_RING_MAP].size
283 = channel->ringbuffer_pagecount << PAGE_SHIFT;
284 pdata->info.mem[TXRX_RING_MAP].memtype = UIO_MEM_IOVA;
285
286 pdata->info.mem[INT_PAGE_MAP].name = "int_page";
287 pdata->info.mem[INT_PAGE_MAP].addr
288 = (uintptr_t)vmbus_connection.int_page;
289 pdata->info.mem[INT_PAGE_MAP].size = PAGE_SIZE;
290 pdata->info.mem[INT_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
291
292 pdata->info.mem[MON_PAGE_MAP].name = "monitor_page";
293 pdata->info.mem[MON_PAGE_MAP].addr
294 = (uintptr_t)vmbus_connection.monitor_pages[1];
295 pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
296 pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
297
298 if (recv_buf_size) {
299 dev_dbg(&dev->device, "recv buffer allocation");
300 pdata->recv_buf = vzalloc(recv_buf_size);
301 if (!pdata->recv_buf) {
302 ret = -ENOMEM;
303 goto fail_free_ring;
304 }
305
306 ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
307 recv_buf_size, &pdata->recv_gpadl);
308 if (ret) {
309 vfree(pdata->recv_buf);
310 goto fail_close;
311 }
312
313 /* put Global Physical Address Label in name */
314 snprintf(pdata->recv_name, sizeof(pdata->recv_name),
315 "recv:%u", pdata->recv_gpadl.gpadl_handle);
316 pdata->info.mem[RECV_BUF_MAP].name = pdata->recv_name;
317 pdata->info.mem[RECV_BUF_MAP].addr = (uintptr_t)pdata->recv_buf;
318 pdata->info.mem[RECV_BUF_MAP].size = recv_buf_size;
319 pdata->info.mem[RECV_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
320 }
321
322 if (send_buf_size) {
323 dev_dbg(&dev->device, "send buffer allocation");
324 pdata->send_buf = vzalloc(send_buf_size);
325 if (!pdata->send_buf) {
326 ret = -ENOMEM;
327 goto fail_close;
328 }
329
330 ret = vmbus_establish_gpadl(channel, pdata->send_buf,
331 send_buf_size, &pdata->send_gpadl);
332 if (ret) {
333 vfree(pdata->send_buf);
334 goto fail_close;
335 }
336
337 snprintf(pdata->send_name, sizeof(pdata->send_name),
338 "send:%u", pdata->send_gpadl.gpadl_handle);
339 pdata->info.mem[SEND_BUF_MAP].name = pdata->send_name;
340 pdata->info.mem[SEND_BUF_MAP].addr = (uintptr_t)pdata->send_buf;
341 pdata->info.mem[SEND_BUF_MAP].size = send_buf_size;
342 pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
343 }
344
345 pdata->info.priv = pdata;
346 pdata->device = dev;
347
348 ret = uio_register_device(&dev->device, &pdata->info);
349 if (ret) {
350 dev_err(&dev->device, "hv_uio register failed\n");
351 goto fail_close;
352 }
353
354 ret = sysfs_create_bin_file(&channel->kobj, &ring_buffer_bin_attr);
355 if (ret)
356 dev_notice(&dev->device,
357 "sysfs create ring bin file failed; %d\n", ret);
358
359 hv_set_drvdata(dev, pdata);
360
361 return 0;
362
363 fail_close:
364 hv_uio_cleanup(dev, pdata);
365 fail_free_ring:
366 vmbus_free_ring(dev->channel);
367
368 return ret;
369 }
370

--
0-DAY CI Kernel Test Service
https://01.org/lkp

Attachments:

(No filename) (10.73 kB)
config (181.30 kB)
Download all attachments

2022-10-12 20:58:18

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

Hi Saurabh,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on linus/master v6.0 next-20221012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Saurabh-Sengar/uio_hv_generic-Enable-support-for-slower-vmbus-device-channels/20221012-195731
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 49da070062390094112b423ba443ea193527b2e4
config: i386-randconfig-a005
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/e7d62290b320f0f50c4f09b8f869b9049ef2c2bd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Saurabh-Sengar/uio_hv_generic-Enable-support-for-slower-vmbus-device-channels/20221012-195731
git checkout e7d62290b320f0f50c4f09b8f869b9049ef2c2bd
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from include/linux/module.h:22,
from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from drivers/uio/uio_hv_generic.c:22:
drivers/uio/uio_hv_generic.c: In function '__check_recv_buf_size':
>> include/linux/moduleparam.h:150:34: error: returning 'size_t *' {aka 'unsigned int *'} from a function with incompatible return type 'long unsigned int *' [-Werror=incompatible-pointer-types]
150 | param_check_##type(name, &(value)); \
| ^
include/linux/moduleparam.h:409:75: note: in definition of macro '__param_check'
409 | static inline type __always_unused *__check_##name(void) { return(p); }
| ^
include/linux/moduleparam.h:150:9: note: in expansion of macro 'param_check_ulong'
150 | param_check_##type(name, &(value)); \
| ^~~~~~~~~~~~
include/linux/moduleparam.h:127:9: note: in expansion of macro 'module_param_named'
127 | module_param_named(name, name, type, perm)
| ^~~~~~~~~~~~~~~~~~
drivers/uio/uio_hv_generic.c:44:1: note: in expansion of macro 'module_param'
44 | module_param(recv_buf_size, ulong, 0644);
| ^~~~~~~~~~~~
drivers/uio/uio_hv_generic.c: In function '__check_send_buf_size':
>> include/linux/moduleparam.h:150:34: error: returning 'size_t *' {aka 'unsigned int *'} from a function with incompatible return type 'long unsigned int *' [-Werror=incompatible-pointer-types]
150 | param_check_##type(name, &(value)); \
| ^
include/linux/moduleparam.h:409:75: note: in definition of macro '__param_check'
409 | static inline type __always_unused *__check_##name(void) { return(p); }
| ^
include/linux/moduleparam.h:150:9: note: in expansion of macro 'param_check_ulong'
150 | param_check_##type(name, &(value)); \
| ^~~~~~~~~~~~
include/linux/moduleparam.h:127:9: note: in expansion of macro 'module_param_named'
127 | module_param_named(name, name, type, perm)
| ^~~~~~~~~~~~~~~~~~
drivers/uio/uio_hv_generic.c:48:1: note: in expansion of macro 'module_param'
48 | module_param(send_buf_size, ulong, 0644);
| ^~~~~~~~~~~~
drivers/uio/uio_hv_generic.c: In function '__check_ring_size':
>> include/linux/moduleparam.h:150:34: error: returning 'size_t *' {aka 'unsigned int *'} from a function with incompatible return type 'long unsigned int *' [-Werror=incompatible-pointer-types]
150 | param_check_##type(name, &(value)); \
| ^
include/linux/moduleparam.h:409:75: note: in definition of macro '__param_check'
409 | static inline type __always_unused *__check_##name(void) { return(p); }
| ^
include/linux/moduleparam.h:150:9: note: in expansion of macro 'param_check_ulong'
150 | param_check_##type(name, &(value)); \
| ^~~~~~~~~~~~
include/linux/moduleparam.h:127:9: note: in expansion of macro 'module_param_named'
127 | module_param_named(name, name, type, perm)
| ^~~~~~~~~~~~~~~~~~
drivers/uio/uio_hv_generic.c:52:1: note: in expansion of macro 'module_param'
52 | module_param(ring_size, ulong, 0644);
| ^~~~~~~~~~~~
In file included from include/linux/printk.h:566,
from include/linux/kernel.h:29,
from arch/x86/include/asm/percpu.h:27,
from arch/x86/include/asm/current.h:6,
from include/linux/sched.h:12,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from drivers/uio/uio_hv_generic.c:22:
drivers/uio/uio_hv_generic.c: In function 'hv_uio_probe':
drivers/uio/uio_hv_generic.c:256:31: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
256 | dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:223:29: note: in definition of macro '__dynamic_func_call_cls'
223 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:249:9: note: in expansion of macro '_dynamic_func_call_cls'
249 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:272:9: note: in expansion of macro '_dynamic_func_call'
272 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:30: note: in expansion of macro 'dev_fmt'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/uio/uio_hv_generic.c:256:9: note: in expansion of macro 'dev_dbg'
256 | dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
| ^~~~~~~
drivers/uio/uio_hv_generic.c:256:62: note: format string is defined here
256 | dev_dbg(&dev->device, "primary channel ring size = %lx", ring_size);
| ~~^
| |
| long unsigned int
| %x
cc1: some warnings being treated as errors

vim +150 include/linux/moduleparam.h

^1da177e4c3f41 Linus Torvalds 2005-04-16 100
546970bc6afc7f Rusty Russell 2010-08-11 101 /**
546970bc6afc7f Rusty Russell 2010-08-11 102 * module_param - typesafe helper for a module/cmdline parameter
e2854a1054ab17 Zhenzhong Duan 2019-11-04 103 * @name: the variable to alter, and exposed parameter name.
546970bc6afc7f Rusty Russell 2010-08-11 104 * @type: the type of the parameter
546970bc6afc7f Rusty Russell 2010-08-11 105 * @perm: visibility in sysfs.
546970bc6afc7f Rusty Russell 2010-08-11 106 *
e2854a1054ab17 Zhenzhong Duan 2019-11-04 107 * @name becomes the module parameter, or (prefixed by KBUILD_MODNAME and a
546970bc6afc7f Rusty Russell 2010-08-11 108 * ".") the kernel commandline parameter. Note that - is changed to _, so
546970bc6afc7f Rusty Russell 2010-08-11 109 * the user can use "foo-bar=1" even for variable "foo_bar".
546970bc6afc7f Rusty Russell 2010-08-11 110 *
c6a8b84da4c28b Randy Dunlap 2020-07-17 111 * @perm is 0 if the variable is not to appear in sysfs, or 0444
546970bc6afc7f Rusty Russell 2010-08-11 112 * for world-readable, 0644 for root-writable, etc. Note that if it
b51d23e4e9fea6 Dan Streetman 2015-06-17 113 * is writable, you may need to use kernel_param_lock() around
546970bc6afc7f Rusty Russell 2010-08-11 114 * accesses (esp. charp, which can be kfreed when it changes).
546970bc6afc7f Rusty Russell 2010-08-11 115 *
546970bc6afc7f Rusty Russell 2010-08-11 116 * The @type is simply pasted to refer to a param_ops_##type and a
546970bc6afc7f Rusty Russell 2010-08-11 117 * param_check_##type: for convenience many standard types are provided but
546970bc6afc7f Rusty Russell 2010-08-11 118 * you can create your own by defining those variables.
546970bc6afc7f Rusty Russell 2010-08-11 119 *
546970bc6afc7f Rusty Russell 2010-08-11 120 * Standard types are:
7d8365771ffb0e Paul Menzel 2020-07-03 121 * byte, hexint, short, ushort, int, uint, long, ulong
546970bc6afc7f Rusty Russell 2010-08-11 122 * charp: a character pointer
546970bc6afc7f Rusty Russell 2010-08-11 123 * bool: a bool, values 0/1, y/n, Y/N.
546970bc6afc7f Rusty Russell 2010-08-11 124 * invbool: the above, only sense-reversed (N = true).
546970bc6afc7f Rusty Russell 2010-08-11 125 */
546970bc6afc7f Rusty Russell 2010-08-11 126 #define module_param(name, type, perm) \
546970bc6afc7f Rusty Russell 2010-08-11 127 module_param_named(name, name, type, perm)
546970bc6afc7f Rusty Russell 2010-08-11 128
3baee201b06cfa Jani Nikula 2014-08-27 129 /**
3baee201b06cfa Jani Nikula 2014-08-27 130 * module_param_unsafe - same as module_param but taints kernel
b6d0531ec7e2ae Fabien Dessenne 2019-12-02 131 * @name: the variable to alter, and exposed parameter name.
b6d0531ec7e2ae Fabien Dessenne 2019-12-02 132 * @type: the type of the parameter
b6d0531ec7e2ae Fabien Dessenne 2019-12-02 133 * @perm: visibility in sysfs.
3baee201b06cfa Jani Nikula 2014-08-27 134 */
3baee201b06cfa Jani Nikula 2014-08-27 135 #define module_param_unsafe(name, type, perm) \
3baee201b06cfa Jani Nikula 2014-08-27 136 module_param_named_unsafe(name, name, type, perm)
3baee201b06cfa Jani Nikula 2014-08-27 137
546970bc6afc7f Rusty Russell 2010-08-11 138 /**
546970bc6afc7f Rusty Russell 2010-08-11 139 * module_param_named - typesafe helper for a renamed module/cmdline parameter
546970bc6afc7f Rusty Russell 2010-08-11 140 * @name: a valid C identifier which is the parameter name.
546970bc6afc7f Rusty Russell 2010-08-11 141 * @value: the actual lvalue to alter.
546970bc6afc7f Rusty Russell 2010-08-11 142 * @type: the type of the parameter
546970bc6afc7f Rusty Russell 2010-08-11 143 * @perm: visibility in sysfs.
546970bc6afc7f Rusty Russell 2010-08-11 144 *
546970bc6afc7f Rusty Russell 2010-08-11 145 * Usually it's a good idea to have variable names and user-exposed names the
546970bc6afc7f Rusty Russell 2010-08-11 146 * same, but that's harder if the variable must be non-static or is inside a
546970bc6afc7f Rusty Russell 2010-08-11 147 * structure. This allows exposure under a different name.
546970bc6afc7f Rusty Russell 2010-08-11 148 */
546970bc6afc7f Rusty Russell 2010-08-11 149 #define module_param_named(name, value, type, perm) \
546970bc6afc7f Rusty Russell 2010-08-11 @150 param_check_##type(name, &(value)); \
546970bc6afc7f Rusty Russell 2010-08-11 151 module_param_cb(name, &param_ops_##type, &value, perm); \
546970bc6afc7f Rusty Russell 2010-08-11 152 __MODULE_PARM_TYPE(name, #type)
546970bc6afc7f Rusty Russell 2010-08-11 153

--
0-DAY CI Kernel Test Service
https://01.org/lkp

Attachments:

(No filename) (12.63 kB)
config (157.18 kB)
Download all attachments

2022-10-13 04:22:59

by Saurabh Singh Sengar

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

On Wed, Oct 12, 2022 at 05:54:07PM +0200, Greg KH wrote:
> On Wed, Oct 12, 2022 at 05:52:05PM +0200, Greg KH wrote:
> > On Wed, Oct 12, 2022 at 04:56:46AM -0700, Saurabh Sengar wrote:
> > > This patch enables support for slower vmbus channels, which consist
> > > of following 3 changes :
> > > 1. Support for hypercalls
> > > 2. Module params for recv/send buffer sizes
> > > 3. Module param for custom ring buffer sizes
> >
> > Even if this all was ok, you are doing 3 things all in one change,
> > that's not allowed at all, you all know this.
> >
> > Anyway, no new module parameters, this is not the 1990's, we have much
> > better ways to do this properly (hint, module parameters modify code,
> > you want to modify the options of a specific device.)
>
> Also, you give no good reason for why this is needed at all, nor how
> anyone would use these options and why they would need to.
>
> The kernel should "just work" and not require manual intervention by a
> user. Dynamically fix this based on the device, do NOT force a user to
> have to attempt to "tune" anything, that will never work properly over
> time, AND you are being lazy and forcing each individual user to do the
> work, making more effort needed overall than just doing it properly in
> the kernel.

Let me find a method if we can avoid using module parameters, this may result
in hardcoding values in the vmbus driver code for various devices, giving less
flexibilty to user. Meanwhile I figure out this, we can go ahead with
"support for hypercalls", I will send a new patch for it.

Regards,
Saurabh

>
> thanks,
>
> greg k-h

2022-10-13 04:57:34

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

On Wed, Oct 12, 2022 at 09:15:15PM -0700, Saurabh Singh Sengar wrote:
> On Wed, Oct 12, 2022 at 05:54:07PM +0200, Greg KH wrote:
> > On Wed, Oct 12, 2022 at 05:52:05PM +0200, Greg KH wrote:
> > > On Wed, Oct 12, 2022 at 04:56:46AM -0700, Saurabh Sengar wrote:
> > > > This patch enables support for slower vmbus channels, which consist
> > > > of following 3 changes :
> > > > 1. Support for hypercalls
> > > > 2. Module params for recv/send buffer sizes
> > > > 3. Module param for custom ring buffer sizes
> > >
> > > Even if this all was ok, you are doing 3 things all in one change,
> > > that's not allowed at all, you all know this.
> > >
> > > Anyway, no new module parameters, this is not the 1990's, we have much
> > > better ways to do this properly (hint, module parameters modify code,
> > > you want to modify the options of a specific device.)
> >
> > Also, you give no good reason for why this is needed at all, nor how
> > anyone would use these options and why they would need to.
> >
> > The kernel should "just work" and not require manual intervention by a
> > user. Dynamically fix this based on the device, do NOT force a user to
> > have to attempt to "tune" anything, that will never work properly over
> > time, AND you are being lazy and forcing each individual user to do the
> > work, making more effort needed overall than just doing it properly in
> > the kernel.
>
> Let me find a method if we can avoid using module parameters, this may result
> in hardcoding values in the vmbus driver code for various devices, giving less
> flexibilty to user.

A user should not be using the uio driver for any of that, so I doubt
this is a real issue :)

thanks,

greg k-h

2022-10-13 14:28:20

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH] uio_hv_generic: Enable support for slower vmbus device channels

Attachments:

(No filename) (6.42 kB)
config (181.30 kB)
Download all attachments