> > On Sun, Sep 06, 2026 at 09:36:59AM +0800, Jia Jia wrote: > > vhost_transport_do_send_pkt() calls vhost_add_used() for every Guest RX > > buffer even though it delays the Guest signal until the worker finishes. > > Each call publishes one used entry and updates the used index separately. > > > > Collect the completed buffer heads in the arrays already allocated for the > > virtqueue and publish them with vhost_add_used_n(). Bound the batch by the > > ring size and array capacity. Flush when the batch reaches its limit or > > before leaving the worker. > > > > Each used entry describes one completed RX buffer and keeps its actual used > > length, so set nheads to 1 for every entry. This patch does not change > > negotiated features or compress multiple buffers into one used entry. > > > > This patch is limited to the current skb-based vhost-vsock RX path. > > > > Performance: > > > > The test used vsock_perf with a fresh Guest for each state and 20 paired > > runs. The Guest receiver used: > > > > vsock_perf --port PORT --buf-size 64M --vsk-size 64M --rcvlowat 1 > > > > The Host sender used: > > > > vsock_perf --sender 3 --port PORT --bytes 1G \ > > --buf-size SEND_BUF --vsk-size 64M > > > > The table reports geometric mean Guest RX throughput. > > > > SEND_BUF baseline RX batching RX change > > (Gbit/s) (Gbit/s) > > 256 B 0.0795724 0.0831509 +4.497% > > 512 B 0.1194885 0.1210297 +1.290% > > 4 KiB 0.7208273 0.7242053 +0.469% > > 64 KiB 2.1712797 2.1951941 +1.101% > > > I will be frank I don't find the numbers compelling enough > to bother with this trickery. > > vsock wasn't optimized all that much for small packets - > first of all, it is doing most of its work in a work item > so we are already at the mercy of the scheduler. > > That's more work but you will see a much bigger win for bw and > latency by just handling small packets directly in the cb > if you can. > > Looking at batching: wakeup per packet in rx_work, lock_sock per packet, > sk->sk_write_space per rx packet are all high overhead things we are > doing in the data path that are likely easier to handle > and will give you more bang for the buck. > > > > >
Thank you very much for giving such a clear direction! > > > > As supplementary data, perf stat measured the vhost worker cycles and > > instructions per GiB in 10 paired runs. The patched implementation > > reduced cycles by 3.846%, 2.162%, and 4.109%, and instructions by > > 2.548%, 2.084%, and 4.637% for 256 B, 4 KiB, and 64 KiB, respectively. > > > > An AF_VSOCK request-response latency test with 10 AB/BA pairs showed no > > consistent RTT change: -0.102% for 256-byte messages (4/10 pairs lower) and > > +3.281% for 4-KiB messages (5/10 pairs lower), using arithmetic mean RTTs. > > > > Signed-off-by: Jia Jia <[email protected]> > > Acked-by: Eugenio Pérez <[email protected]> > > --- > > Changes in v4: > > - Simplify the performance description. > > - Add a brief AF_VSOCK RTT measurement. > > - Keep the batch limit within the used ring and scratch arrays, without > > duplicating the worker weight limit. > > - Flush after adding a used entry when the batch reaches the limit, and > > remove the redundant flush in the empty-queue path. > > --- > > drivers/vhost/vsock.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 45 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c > > index 9aaab6bb8061..7a13abe73345 100644 > > --- a/drivers/vhost/vsock.c > > +++ b/drivers/vhost/vsock.c > > @@ -103,12 +103,35 @@ static bool vhost_transport_has_remote_cid(struct > > vsock_sock *vsk, u32 cid) > > return found; > > } > > > > +static bool vhost_vsock_flush_used(struct vhost_virtqueue *vq, > > + unsigned int used_count) > > +{ > > + if (!used_count) > > + return false; > > + > > + vhost_add_used_n(vq, vq->heads, vq->nheads, used_count); > > + return true; > > +} > > + > > +static void vhost_vsock_add_used(struct vhost_virtqueue *vq, > > + unsigned int used_count, > > + unsigned int head, unsigned int len) > > +{ > > + struct vring_used_elem *used = &vq->heads[used_count]; > > + > > + used->id = cpu_to_vhost32(vq, head); > > + used->len = cpu_to_vhost32(vq, len); > > + vq->nheads[used_count] = 1; > > +} > > + > > > flush where? add where? return what? these apis don't make it > easier to read code. > Thanks for pointing this out. If I understand correctly, the responsibilities, call sites, and return values of these helpers make the control flow harder to follow. I will be careful not to make the path more complicated for the sake of abstraction in the future. > > static void > > vhost_transport_do_send_pkt(struct vhost_vsock *vsock, > > struct vhost_virtqueue *vq) > > { > > struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX]; > > int pkts = 0, total_len = 0; > > + unsigned int used_count = 0; > > + unsigned int used_limit; > > bool added = false; > > bool restart_tx = false; > > > > @@ -120,6 +143,11 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock, > > if (!vq_meta_prefetch(vq)) > > goto out; > > > > + /* Keep the batch within the used ring and the scratch arrays. */ > > what "the batch"? this is the 1st time code mentions any batch. > In the future, I will try not to introduce concepts used in a patch midway through the code. They should be explained when used. > > + used_limit = min_t(unsigned int, vq->num, vq->dev->iov_limit); > > + if (unlikely(!used_limit)) > > + goto out; > > + > > /* Avoid further vmexits, we're already processing the virtqueue */ > > vhost_disable_notify(&vsock->dev, vq); > > > > @@ -150,9 +178,16 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock, > > > > if (head == vq->num) { > > virtio_vsock_skb_queue_head(&vsock->send_pkt_queue, > > skb); > > + > > + /* Flush completed buffers before re-enabling > > notifications. */ > > > redundant - i can see this is what it does but why? and what does flush mean > here? > I will pay attention to making the purpose of comments clear. > > + if (vhost_vsock_flush_used(vq, used_count)) { > > + added = true; > > + used_count = 0; > > + } > > + > > /* We cannot finish yet if more buffers snuck in while > > * re-enabling notify. > > */ > > if (unlikely(vhost_enable_notify(&vsock->dev, vq))) { > > vhost_disable_notify(&vsock->dev, vq); > > continue; > > @@ -230,8 +265,13 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock, > > */ > > virtio_transport_deliver_tap_pkt(skb); > > > > - vhost_add_used(vq, head, sizeof(*hdr) + payload_len); > > - added = true; > > + vhost_vsock_add_used(vq, used_count, head, > > + sizeof(*hdr) + payload_len); > > + used_count++; > > + if (used_count == used_limit) { > > + added |= vhost_vsock_flush_used(vq, used_count); > > > bitwise or on a boolean likely not what was intended. > Understood. > > + used_count = 0; > > + } > > > > VIRTIO_VSOCK_SKB_CB(skb)->offset += payload_len; > > total_len += payload_len; > > @@ -264,6 +304,9 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock, > > virtio_transport_consume_skb_sent(skb, true); > > } > > } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len))); > > + > > + added |= vhost_vsock_flush_used(vq, used_count); > > + > > > and here. > I will drop the current patch and try a new optimization direction. Thank you. > > if (added) > > vhost_signal(&vsock->dev, vq); > > > > -- > > 2.34.1 >
