* [PATCH] uio_hv_generic: Add a check for HV_NIC for send, receive buffers setup
@ 2024-11-25 12:50 Naman Jain
2024-11-26 16:33 ` Saurabh Singh Sengar
0 siblings, 1 reply; 3+ messages in thread
From: Naman Jain @ 2024-11-25 12:50 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Greg Kroah-Hartman
Cc: linux-hyperv, linux-kernel, Saurabh Sengar
Support for send and receive buffers was added for networking usecases
in UIO devices. There is no known usecase of these buffers for devices
other than HV_NIC. Add a check for HV_NIC in probe function to avoid
memory allocation and GPADL setup which would save 47 MB memory per
device type.
While at it, fix some of the syntax related issues in the touched code
which are reported by "--strict" option of checkpatch.
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
drivers/uio/uio_hv_generic.c | 86 ++++++++++++++++++------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 3976360d0096..1b19b5647495 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -296,51 +296,51 @@ 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;
+ if (channel->device_id == HV_NIC) {
+ pdata->recv_buf = vzalloc(RECV_BUFFER_SIZE);
+ if (!pdata->recv_buf) {
+ ret = -ENOMEM;
+ goto fail_free_ring;
+ }
+
+ ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
+ RECV_BUFFER_SIZE, &pdata->recv_gpadl);
+ if (ret) {
+ if (!pdata->recv_gpadl.decrypted)
+ 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) {
+ ret = -ENOMEM;
+ goto fail_close;
+ }
+
+ ret = vmbus_establish_gpadl(channel, pdata->send_buf,
+ SEND_BUFFER_SIZE, &pdata->send_gpadl);
+ if (ret) {
+ if (!pdata->send_gpadl.decrypted)
+ 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_BUFFER_SIZE;
+ pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
}
- ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
- RECV_BUFFER_SIZE, &pdata->recv_gpadl);
- if (ret) {
- if (!pdata->recv_gpadl.decrypted)
- 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;
- }
-
- ret = vmbus_establish_gpadl(channel, pdata->send_buf,
- SEND_BUFFER_SIZE, &pdata->send_gpadl);
- if (ret) {
- if (!pdata->send_gpadl.decrypted)
- 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_BUFFER_SIZE;
- pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
-
pdata->info.priv = pdata;
pdata->device = dev;
base-commit: 85a2dd7d7c8152cb125712a1ecae1d0a6ccac250
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] uio_hv_generic: Add a check for HV_NIC for send, receive buffers setup
2024-11-25 12:50 [PATCH] uio_hv_generic: Add a check for HV_NIC for send, receive buffers setup Naman Jain
@ 2024-11-26 16:33 ` Saurabh Singh Sengar
2024-11-27 6:51 ` Naman Jain
0 siblings, 1 reply; 3+ messages in thread
From: Saurabh Singh Sengar @ 2024-11-26 16:33 UTC (permalink / raw)
To: Naman Jain
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Greg Kroah-Hartman, linux-hyperv, linux-kernel
On Mon, Nov 25, 2024 at 12:50:15PM +0000, Naman Jain wrote:
> Support for send and receive buffers was added for networking usecases
> in UIO devices. There is no known usecase of these buffers for devices
> other than HV_NIC. Add a check for HV_NIC in probe function to avoid
> memory allocation and GPADL setup which would save 47 MB memory per
> device type.
Thanks for the patch. How about rephrasing the commit message like this:
Receive and send buffer allocation was originally introduced to support
DPDK's networking use case. These buffer sizes were further increased to
meet DPDK performance requirements. However, these large buffers are
unnecessary for any other UIO use cases.
Restrict the allocation of receive and send buffers only for HV_NIC device
type, saving 47 MB of memory per device.
>
> While at it, fix some of the syntax related issues in the touched code
> which are reported by "--strict" option of checkpatch.
>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> drivers/uio/uio_hv_generic.c | 86 ++++++++++++++++++------------------
> 1 file changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 3976360d0096..1b19b5647495 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -296,51 +296,51 @@ 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;
> + if (channel->device_id == HV_NIC) {
> + pdata->recv_buf = vzalloc(RECV_BUFFER_SIZE);
> + if (!pdata->recv_buf) {
> + ret = -ENOMEM;
> + goto fail_free_ring;
> + }
> +
> + ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
> + RECV_BUFFER_SIZE, &pdata->recv_gpadl);
> + if (ret) {
> + if (!pdata->recv_gpadl.decrypted)
> + 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) {
> + ret = -ENOMEM;
> + goto fail_close;
> + }
> +
> + ret = vmbus_establish_gpadl(channel, pdata->send_buf,
> + SEND_BUFFER_SIZE, &pdata->send_gpadl);
> + if (ret) {
> + if (!pdata->send_gpadl.decrypted)
> + 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_BUFFER_SIZE;
> + pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
> }
>
> - ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
> - RECV_BUFFER_SIZE, &pdata->recv_gpadl);
> - if (ret) {
> - if (!pdata->recv_gpadl.decrypted)
> - 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;
> - }
> -
> - ret = vmbus_establish_gpadl(channel, pdata->send_buf,
> - SEND_BUFFER_SIZE, &pdata->send_gpadl);
> - if (ret) {
> - if (!pdata->send_gpadl.decrypted)
> - 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_BUFFER_SIZE;
> - pdata->info.mem[SEND_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
> -
> pdata->info.priv = pdata;
> pdata->device = dev;
>
>
> base-commit: 85a2dd7d7c8152cb125712a1ecae1d0a6ccac250
overall change looks good to me.
- Saurabh
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] uio_hv_generic: Add a check for HV_NIC for send, receive buffers setup
2024-11-26 16:33 ` Saurabh Singh Sengar
@ 2024-11-27 6:51 ` Naman Jain
0 siblings, 0 replies; 3+ messages in thread
From: Naman Jain @ 2024-11-27 6:51 UTC (permalink / raw)
To: Saurabh Singh Sengar
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Greg Kroah-Hartman, linux-hyperv, linux-kernel
On 11/26/2024 10:03 PM, Saurabh Singh Sengar wrote:
> On Mon, Nov 25, 2024 at 12:50:15PM +0000, Naman Jain wrote:
>> Support for send and receive buffers was added for networking usecases
>> in UIO devices. There is no known usecase of these buffers for devices
>> other than HV_NIC. Add a check for HV_NIC in probe function to avoid
>> memory allocation and GPADL setup which would save 47 MB memory per
>> device type.
>
> Thanks for the patch. How about rephrasing the commit message like this:
>
> Receive and send buffer allocation was originally introduced to support
> DPDK's networking use case. These buffer sizes were further increased to
> meet DPDK performance requirements. However, these large buffers are
> unnecessary for any other UIO use cases.
>
> Restrict the allocation of receive and send buffers only for HV_NIC device
> type, saving 47 MB of memory per device.
Thanks for the review. I'll rephrase it and push it in a new patch after
a few days, considering ongoing merge window.
Regards,
Naman
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-27 6:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-25 12:50 [PATCH] uio_hv_generic: Add a check for HV_NIC for send, receive buffers setup Naman Jain
2024-11-26 16:33 ` Saurabh Singh Sengar
2024-11-27 6:51 ` Naman Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox