Thread (10 messages) 10 messages, 3 authors, 2025-12-26

Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays

From: Jon Kohler <hidden>
Date: 2025-11-26 16:47:38
Also in: kvm, lkml, virtualization

On Nov 26, 2025, at 1:41 AM, Michael S. Tsirkin [off-list ref] wrote:

!-------------------------------------------------------------------|
 CAUTION: External Email

|-------------------------------------------------------------------!

On Tue, Nov 25, 2025 at 11:00:33AM -0700, Jon Kohler wrote:
quoted
In non-busypoll handle_rx paths, if peek_head_len returns 0, the RX
loop breaks, the RX wait queue is re-enabled, and vhost_net_signal_used
is called to flush done_idx and notify the guest if needed.

However, signaling the guest can take non-trivial time. During this
window, additional RX payloads may arrive on rx_ring without further
kicks. These new payloads will sit unprocessed until another kick
arrives, increasing latency. In high-rate UDP RX workloads, this was
observed to occur over 20k times per second.

To minimize this window and improve opportunities to process packets
promptly, immediately call peek_head_len after signaling. If new packets
are found, treat it as a busy poll interrupt and requeue handle_rx,
improving fairness to TX handlers and other pending CPU work. This also
helps suppress unnecessary thread wakeups, reducing waker CPU demand.

Signed-off-by: Jon Kohler <redacted>
---
drivers/vhost/net.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 35ded4330431..04cb5f1dc6e4 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1015,6 +1015,27 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
struct vhost_virtqueue *tvq = &tnvq->vq;
int len = peek_head_len(rnvq, sk);

+ if (!len && rnvq->done_idx) {
+ /* When idle, flush signal first, which can take some
+ * time for ring management and guest notification.
+ * Afterwards, check one last time for work, as the ring
+ * may have received new work during the notification
+ * window.
+ */
+ vhost_net_signal_used(rnvq, *count);
+ *count = 0;
+ if (peek_head_len(rnvq, sk)) {
+ /* More work came in during the notification
+ * window. To be fair to the TX handler and other
+ * potentially pending work items, pretend like
+ * this was a busy poll interruption so that
+ * the RX handler will be rescheduled and try
+ * again.
+ */
+ *busyloop_intr = true;
+ }
+ }
+
if (!len && rvq->busyloop_timeout) {
/* Flush batched heads first */
vhost_net_signal_used(rnvq, *count);

Looks like this can easily send more interrupts than originally?
How can this be good?

From the description, I would expect the changes to just add another call to
peek_head_len after the existing vhost_net_signal_used.
What am I missing?
consider the following race, across NUMA nodes where it is most expensive.

Socket 1 (pNIC) <-------- NUMA -------> Socket 2 (vhost worker)
                                        vhost_net_rx_peek_head_len = 0
                                          peek_head_len = 0
                                            vhost_net_buf_peek
                                            vhost_net_buf_produce
tun_net_xmit                                ptr_ring_consume_batched
ptr_ring_produce()=0
                                        else if (!sock_len)
                                          vhost_net_enable_vq(net, vq);
                                            tun_chr_poll
                                              add_wait_queue
tfile->socket.sk->sk_data_ready()
  sock_def_readable
    skwq_has_sleeper=true
      wake_up ...
        vhost_poll_wakeup
          TTWU
                                        vhost_net_signal_used();
                                          ring operations (takes time, SMAP)
                                          signalling guest (takes time)
                                        vhost_task_fn
                                        schedule()!!
          ttwu rq spinlock!               rq lock
                                          schedules out
          IPI!                              
                                        idle path
                                          sched_ttwu_pending

All I’m saying here is that if we simply use the signal during
the time where we are *not* added to the wait queue, that will
give plenty of time for the race above to resolve wherein we
will give the lockless TX’ers time to add more workload to the
rx_ring, we can either a) process it right there (by assigning
len as you suggested in other mail) or b) set busy poll and
process tx handler or other work, with notification disabled

For the sake of argument, let’s pretend signaling took a full
second. A lot can happen in that time, so if signal first, then
check after, we can avoid the IPIs and trips in n out of
scheduler
quoted
-- 
2.43.0
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help