[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <omiwday3x75sfre6qp7zp7rzmdwyv27eq2wmzkz6hwt7p6jqvv@n2dx55e2xojf>
Date: Tue, 2 Jul 2024 12:00:00 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: luigi.leonardi@...look.com
Cc: Stefan Hajnoczi <stefanha@...hat.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, kvm@...r.kernel.org,
virtualization@...ts.linux.dev, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
Marco Pinna <marco.pinn95@...il.com>
Subject: Re: [PATCH PATCH net-next v2 2/2] vsock/virtio: avoid enqueue
packets when work queue is empty
On Mon, Jul 01, 2024 at 04:28:03PM GMT, Luigi Leonardi via B4 Relay wrote:
>From: Marco Pinna <marco.pinn95@...il.com>
>
>Introduce an optimization in virtio_transport_send_pkt:
>when the work queue (send_pkt_queue) is empty the packet is
>put directly in the virtqueue reducing latency.
>
>In the following benchmark (pingpong mode) the host sends
>a payload to the guest and waits for the same payload back.
>
>All vCPUs pinned individually to pCPUs.
>vhost process pinned to a pCPU
>fio process pinned both inside the host and the guest system.
>
>Host CPU: Intel i7-10700KF CPU @ 3.80GHz
>Tool: Fio version 3.37-56
>Env: Phys host + L1 Guest
>Payload: 512
>Runtime-per-test: 50s
>Mode: pingpong (h-g-h)
>Test runs: 50
>Type: SOCK_STREAM
>
>Before (Linux 6.8.11)
>------
>mean(1st percentile): 380.56 ns
>mean(overall): 780.83 ns
>mean(99th percentile): 8300.24 ns
>
>After
>------
>mean(1st percentile): 370.59 ns
>mean(overall): 720.66 ns
>mean(99th percentile): 7600.27 ns
>
>Same setup, using 4K payload:
>
>Before (Linux 6.8.11)
>------
>mean(1st percentile): 458.84 ns
>mean(overall): 1650.17 ns
>mean(99th percentile): 42240.68 ns
>
>After
>------
>mean(1st percentile): 450.12 ns
>mean(overall): 1460.84 ns
>mean(99th percentile): 37632.45 ns
>
>virtqueue.
>
>Throughput: iperf-vsock
>
>Before (Linux 6.8.11)
>G2H 28.7 Gb/s
>
>After
>G2H 40.8 Gb/s
Cool!
I'd suggest to add the length of buffer (-l param) used, and also
check more lenghts, like at least 4k, 64k, 128k.
>
>The performance improvement is related to this optimization,
>I checked that each packet was put directly on the vq
>avoiding the work queue.
How?
>
>Co-developed-by: Luigi Leonardi <luigi.leonardi@...look.com>
>Signed-off-by: Luigi Leonardi <luigi.leonardi@...look.com>
>Signed-off-by: Marco Pinna <marco.pinn95@...il.com>
I think you might want to change the author of this patch, since it's
changed a lot from Marco's original one. Obviously if you both agree on
this.
Thanks,
Stefano
>---
> net/vmw_vsock/virtio_transport.c | 38 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 36 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index a74083d28120..3815aa8d956b 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -213,6 +213,7 @@ virtio_transport_send_pkt(struct sk_buff *skb)
> {
> struct virtio_vsock_hdr *hdr;
> struct virtio_vsock *vsock;
>+ bool use_worker = true;
> int len = skb->len;
>
> hdr = virtio_vsock_hdr(skb);
>@@ -234,8 +235,41 @@ virtio_transport_send_pkt(struct sk_buff *skb)
> if (virtio_vsock_skb_reply(skb))
> atomic_inc(&vsock->queued_replies);
>
>- virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb);
>- queue_work(virtio_vsock_workqueue, &vsock->send_pkt_work);
>+ /* If the workqueue (send_pkt_queue) is empty there is no need to enqueue the packet.
>+ * Just put it on the virtqueue using virtio_transport_send_skb.
>+ */
>+ if (skb_queue_empty_lockless(&vsock->send_pkt_queue)) {
>+ bool restart_rx = false;
>+ struct virtqueue *vq;
>+ int ret;
>+
>+ /* Inside RCU, can't sleep! */
>+ ret = mutex_trylock(&vsock->tx_lock);
>+ if (unlikely(ret == 0))
>+ goto out_worker;
>+
>+ /* Driver is being removed, no need to enqueue the packet */
>+ if (!vsock->tx_run)
>+ goto out_rcu;
>+
>+ vq = vsock->vqs[VSOCK_VQ_TX];
>+
>+ if (!virtio_transport_send_skb(skb, vq, vsock, &restart_rx)) {
>+ use_worker = false;
>+ virtqueue_kick(vq);
>+ }
>+
>+ mutex_unlock(&vsock->tx_lock);
>+
>+ if (restart_rx)
>+ queue_work(virtio_vsock_workqueue, &vsock->rx_work);
>+ }
>+
>+out_worker:
>+ if (use_worker) {
>+ virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb);
>+ queue_work(virtio_vsock_workqueue, &vsock->send_pkt_work);
>+ }
>
> out_rcu:
> rcu_read_unlock();
>
>-- 2.45.2
>
>
Powered by blists - more mailing lists