* [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable
@ 2021-07-21 14:30 ` Lee Jones
0 siblings, 0 replies; 7+ messages in thread
From: Lee Jones @ 2021-07-21 14:30 UTC (permalink / raw)
To: lee.jones
Cc: linux-kernel, Ram Muthiah, Michael S. Tsirkin, Jason Wang,
Stefan Hajnoczi, Stefano Garzarella, David S. Miller,
Jakub Kicinski, virtualization, kvm, netdev
From: Ram Muthiah <rammuthiah@google.com>
After a virtual device has been running for some time, the SLAB
sustains ever increasing fragmentation. Contributing to this
fragmentation are the virtio packet buffer allocations which
are a drain on 64Kb compound pages. Eventually these can't be
allocated due to fragmentation.
To enable successful allocations for this packet buffer, the
packet buffer's size needs to be reduced.
In order to enable a reduction without impacting current users,
this variable is being exposed as a command line parameter.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: virtualization@lists.linux-foundation.org
Cc: kvm@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Ram Muthiah <rammuthiah@google.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
include/linux/virtio_vsock.h | 4 +++-
net/vmw_vsock/virtio_transport_common.c | 4 ++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 35d7eedb5e8e4..8c77d60a74d34 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -7,9 +7,11 @@
#include <net/sock.h>
#include <net/af_vsock.h>
+extern uint virtio_transport_max_vsock_pkt_buf_size;
+
#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4)
#define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL
-#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
+#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE virtio_transport_max_vsock_pkt_buf_size
enum {
VSOCK_VQ_RX = 0, /* for host to guest data */
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 169ba8b72a630..d0d913afec8b6 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -26,6 +26,10 @@
/* Threshold for detecting small packets to copy */
#define GOOD_COPY_LEN 128
+uint virtio_transport_max_vsock_pkt_buf_size = 1024 * 64;
+module_param(virtio_transport_max_vsock_pkt_buf_size, uint, 0444);
+EXPORT_SYMBOL_GPL(virtio_transport_max_vsock_pkt_buf_size);
+
static const struct virtio_transport *
virtio_transport_get_ops(struct vsock_sock *vsk)
{
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable
@ 2021-07-21 14:30 ` Lee Jones
0 siblings, 0 replies; 7+ messages in thread
From: Lee Jones @ 2021-07-21 14:30 UTC (permalink / raw)
To: lee.jones
Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, Ram Muthiah,
Stefan Hajnoczi, Jakub Kicinski, virtualization, David S. Miller
From: Ram Muthiah <rammuthiah@google.com>
After a virtual device has been running for some time, the SLAB
sustains ever increasing fragmentation. Contributing to this
fragmentation are the virtio packet buffer allocations which
are a drain on 64Kb compound pages. Eventually these can't be
allocated due to fragmentation.
To enable successful allocations for this packet buffer, the
packet buffer's size needs to be reduced.
In order to enable a reduction without impacting current users,
this variable is being exposed as a command line parameter.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: virtualization@lists.linux-foundation.org
Cc: kvm@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Ram Muthiah <rammuthiah@google.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
include/linux/virtio_vsock.h | 4 +++-
net/vmw_vsock/virtio_transport_common.c | 4 ++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 35d7eedb5e8e4..8c77d60a74d34 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -7,9 +7,11 @@
#include <net/sock.h>
#include <net/af_vsock.h>
+extern uint virtio_transport_max_vsock_pkt_buf_size;
+
#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4)
#define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL
-#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
+#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE virtio_transport_max_vsock_pkt_buf_size
enum {
VSOCK_VQ_RX = 0, /* for host to guest data */
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 169ba8b72a630..d0d913afec8b6 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -26,6 +26,10 @@
/* Threshold for detecting small packets to copy */
#define GOOD_COPY_LEN 128
+uint virtio_transport_max_vsock_pkt_buf_size = 1024 * 64;
+module_param(virtio_transport_max_vsock_pkt_buf_size, uint, 0444);
+EXPORT_SYMBOL_GPL(virtio_transport_max_vsock_pkt_buf_size);
+
static const struct virtio_transport *
virtio_transport_get_ops(struct vsock_sock *vsk)
{
--
2.32.0.402.g57bb445576-goog
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable
2021-07-21 14:30 ` Lee Jones
@ 2021-07-22 12:55 ` Stefano Garzarella
-1 siblings, 0 replies; 7+ messages in thread
From: Stefano Garzarella @ 2021-07-22 12:55 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, Ram Muthiah, Michael S. Tsirkin, Jason Wang,
Stefan Hajnoczi, David S. Miller, Jakub Kicinski, virtualization,
kvm, netdev, jiang.wang
On Wed, Jul 21, 2021 at 03:30:00PM +0100, Lee Jones wrote:
>From: Ram Muthiah <rammuthiah@google.com>
>
>After a virtual device has been running for some time, the SLAB
>sustains ever increasing fragmentation. Contributing to this
>fragmentation are the virtio packet buffer allocations which
>are a drain on 64Kb compound pages. Eventually these can't be
>allocated due to fragmentation.
>
>To enable successful allocations for this packet buffer, the
>packet buffer's size needs to be reduced.
>
>In order to enable a reduction without impacting current users,
>this variable is being exposed as a command line parameter.
>
>Cc: "Michael S. Tsirkin" <mst@redhat.com>
>Cc: Jason Wang <jasowang@redhat.com>
>Cc: Stefan Hajnoczi <stefanha@redhat.com>
>Cc: Stefano Garzarella <sgarzare@redhat.com>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: virtualization@lists.linux-foundation.org
>Cc: kvm@vger.kernel.org
>Cc: netdev@vger.kernel.org
>Signed-off-by: Ram Muthiah <rammuthiah@google.com>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> include/linux/virtio_vsock.h | 4 +++-
> net/vmw_vsock/virtio_transport_common.c | 4 ++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 35d7eedb5e8e4..8c77d60a74d34 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -7,9 +7,11 @@
> #include <net/sock.h>
> #include <net/af_vsock.h>
>
>+extern uint virtio_transport_max_vsock_pkt_buf_size;
>+
> #define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4)
> #define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL
>-#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
>+#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE virtio_transport_max_vsock_pkt_buf_size
>
> enum {
> VSOCK_VQ_RX = 0, /* for host to guest data */
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 169ba8b72a630..d0d913afec8b6 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -26,6 +26,10 @@
> /* Threshold for detecting small packets to copy */
> #define GOOD_COPY_LEN 128
>
>+uint virtio_transport_max_vsock_pkt_buf_size = 1024 * 64;
>+module_param(virtio_transport_max_vsock_pkt_buf_size, uint, 0444);
>+EXPORT_SYMBOL_GPL(virtio_transport_max_vsock_pkt_buf_size);
>+
Maybe better to add an entry under sysfs similar to what Jiang proposed
here:
https://lists.linuxfoundation.org/pipermail/virtualization/2021-June/054769.html
Thanks,
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable
@ 2021-07-22 12:55 ` Stefano Garzarella
0 siblings, 0 replies; 7+ messages in thread
From: Stefano Garzarella @ 2021-07-22 12:55 UTC (permalink / raw)
To: Lee Jones
Cc: jiang.wang, kvm, Michael S. Tsirkin, netdev, linux-kernel,
Ram Muthiah, Stefan Hajnoczi, Jakub Kicinski, virtualization,
David S. Miller
On Wed, Jul 21, 2021 at 03:30:00PM +0100, Lee Jones wrote:
>From: Ram Muthiah <rammuthiah@google.com>
>
>After a virtual device has been running for some time, the SLAB
>sustains ever increasing fragmentation. Contributing to this
>fragmentation are the virtio packet buffer allocations which
>are a drain on 64Kb compound pages. Eventually these can't be
>allocated due to fragmentation.
>
>To enable successful allocations for this packet buffer, the
>packet buffer's size needs to be reduced.
>
>In order to enable a reduction without impacting current users,
>this variable is being exposed as a command line parameter.
>
>Cc: "Michael S. Tsirkin" <mst@redhat.com>
>Cc: Jason Wang <jasowang@redhat.com>
>Cc: Stefan Hajnoczi <stefanha@redhat.com>
>Cc: Stefano Garzarella <sgarzare@redhat.com>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: virtualization@lists.linux-foundation.org
>Cc: kvm@vger.kernel.org
>Cc: netdev@vger.kernel.org
>Signed-off-by: Ram Muthiah <rammuthiah@google.com>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> include/linux/virtio_vsock.h | 4 +++-
> net/vmw_vsock/virtio_transport_common.c | 4 ++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 35d7eedb5e8e4..8c77d60a74d34 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -7,9 +7,11 @@
> #include <net/sock.h>
> #include <net/af_vsock.h>
>
>+extern uint virtio_transport_max_vsock_pkt_buf_size;
>+
> #define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4)
> #define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL
>-#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
>+#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE virtio_transport_max_vsock_pkt_buf_size
>
> enum {
> VSOCK_VQ_RX = 0, /* for host to guest data */
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 169ba8b72a630..d0d913afec8b6 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -26,6 +26,10 @@
> /* Threshold for detecting small packets to copy */
> #define GOOD_COPY_LEN 128
>
>+uint virtio_transport_max_vsock_pkt_buf_size = 1024 * 64;
>+module_param(virtio_transport_max_vsock_pkt_buf_size, uint, 0444);
>+EXPORT_SYMBOL_GPL(virtio_transport_max_vsock_pkt_buf_size);
>+
Maybe better to add an entry under sysfs similar to what Jiang proposed
here:
https://lists.linuxfoundation.org/pipermail/virtualization/2021-June/054769.html
Thanks,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable
2021-07-22 12:55 ` Stefano Garzarella
(?)
@ 2022-12-09 19:48 ` Carlos Llamas
2022-12-12 13:10 ` Stefano Garzarella
-1 siblings, 1 reply; 7+ messages in thread
From: Carlos Llamas @ 2022-12-09 19:48 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Lee Jones, linux-kernel, Ram Muthiah, Michael S. Tsirkin,
Jason Wang, Stefan Hajnoczi, David S. Miller, Jakub Kicinski,
virtualization, kvm, netdev, jiang.wang
On Thu, Jul 22, 2021 at 02:55:19PM +0200, Stefano Garzarella wrote:
> >
> > +uint virtio_transport_max_vsock_pkt_buf_size = 1024 * 64;
> > +module_param(virtio_transport_max_vsock_pkt_buf_size, uint, 0444);
> > +EXPORT_SYMBOL_GPL(virtio_transport_max_vsock_pkt_buf_size);
> > +
I'm interested on this functionality, so I could take this on.
>
> Maybe better to add an entry under sysfs similar to what Jiang proposed
> here:
> https://lists.linuxfoundation.org/pipermail/virtualization/2021-June/054769.html
Having a look at Jiang's RFC patch it seems the proposed sysfs node
hangs off from the main kernel object e.g. /sys/kernel. So I wonder if
there is a more appropriate parent for this knob?
Also, I noticed that Ram's patch here is using read-only permissions for
the module parameter and switching to sysfs would mean opening this knob
up to be dynamically configured? I'd need to be careful here.
--
Carlos Llamas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable
2022-12-09 19:48 ` Carlos Llamas
@ 2022-12-12 13:10 ` Stefano Garzarella
0 siblings, 0 replies; 7+ messages in thread
From: Stefano Garzarella @ 2022-12-12 13:10 UTC (permalink / raw)
To: Carlos Llamas
Cc: jiang.wang, kvm, Michael S. Tsirkin, netdev, linux-kernel,
Ram Muthiah, Stefan Hajnoczi, Jakub Kicinski, virtualization,
Lee Jones, David S. Miller
On Fri, Dec 09, 2022 at 07:48:02PM +0000, Carlos Llamas wrote:
>On Thu, Jul 22, 2021 at 02:55:19PM +0200, Stefano Garzarella wrote:
>> >
>> > +uint virtio_transport_max_vsock_pkt_buf_size = 1024 * 64;
>> > +module_param(virtio_transport_max_vsock_pkt_buf_size, uint, 0444);
>> > +EXPORT_SYMBOL_GPL(virtio_transport_max_vsock_pkt_buf_size);
>> > +
>
>I'm interested on this functionality, so I could take this on.
Great!
We are changing the packet handling using sk_buff [1], so I think it's
better to rebase on that work that should be merged in net-next after
the current merge window will close.
>
>>
>> Maybe better to add an entry under sysfs similar to what Jiang proposed
>> here:
>> https://lists.linuxfoundation.org/pipermail/virtualization/2021-June/054769.html
>
>Having a look at Jiang's RFC patch it seems the proposed sysfs node
>hangs off from the main kernel object e.g. /sys/kernel. So I wonder if
>there is a more appropriate parent for this knob?
Agree, what about /sys/devices ?
I would take a closer look at what is recommend in this case.
>
>Also, I noticed that Ram's patch here is using read-only permissions for
>the module parameter and switching to sysfs would mean opening this knob
>up to be dynamically configured? I'd need to be careful here.
>
True, but even if it's changed while we're running, I don't think it's a
big problem.
Maybe the problem here would be the allocation of RX buffers made during
the probe. Could this be a good reason to use a module parameter?
Thanks,
Stefano
[1]
https://lore.kernel.org/lkml/20221202173520.10428-1-bobby.eshleman@bytedance.com/
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable
@ 2022-12-12 13:10 ` Stefano Garzarella
0 siblings, 0 replies; 7+ messages in thread
From: Stefano Garzarella @ 2022-12-12 13:10 UTC (permalink / raw)
To: Carlos Llamas
Cc: Lee Jones, linux-kernel, Ram Muthiah, Michael S. Tsirkin,
Jason Wang, Stefan Hajnoczi, David S. Miller, Jakub Kicinski,
virtualization, kvm, netdev, jiang.wang
On Fri, Dec 09, 2022 at 07:48:02PM +0000, Carlos Llamas wrote:
>On Thu, Jul 22, 2021 at 02:55:19PM +0200, Stefano Garzarella wrote:
>> >
>> > +uint virtio_transport_max_vsock_pkt_buf_size = 1024 * 64;
>> > +module_param(virtio_transport_max_vsock_pkt_buf_size, uint, 0444);
>> > +EXPORT_SYMBOL_GPL(virtio_transport_max_vsock_pkt_buf_size);
>> > +
>
>I'm interested on this functionality, so I could take this on.
Great!
We are changing the packet handling using sk_buff [1], so I think it's
better to rebase on that work that should be merged in net-next after
the current merge window will close.
>
>>
>> Maybe better to add an entry under sysfs similar to what Jiang proposed
>> here:
>> https://lists.linuxfoundation.org/pipermail/virtualization/2021-June/054769.html
>
>Having a look at Jiang's RFC patch it seems the proposed sysfs node
>hangs off from the main kernel object e.g. /sys/kernel. So I wonder if
>there is a more appropriate parent for this knob?
Agree, what about /sys/devices ?
I would take a closer look at what is recommend in this case.
>
>Also, I noticed that Ram's patch here is using read-only permissions for
>the module parameter and switching to sysfs would mean opening this knob
>up to be dynamically configured? I'd need to be careful here.
>
True, but even if it's changed while we're running, I don't think it's a
big problem.
Maybe the problem here would be the allocation of RX buffers made during
the probe. Could this be a good reason to use a module parameter?
Thanks,
Stefano
[1]
https://lore.kernel.org/lkml/20221202173520.10428-1-bobby.eshleman@bytedance.com/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-12 13:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 14:30 [PATCH 1/1] virtio/vsock: Make vsock virtio packet buff size configurable Lee Jones
2021-07-21 14:30 ` Lee Jones
2021-07-22 12:55 ` Stefano Garzarella
2021-07-22 12:55 ` Stefano Garzarella
2022-12-09 19:48 ` Carlos Llamas
2022-12-12 13:10 ` Stefano Garzarella
2022-12-12 13:10 ` Stefano Garzarella
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.