Message-ID: <C814D406-DA1A-497B-9A5F-C6ED60BD018A@nutanix.com>
Date: Wed, 26 Nov 2025 16:47:31 +0000
From: Jon Kohler <jon@...anix.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
CC: Jason Wang <jasowang@...hat.com>,
        Eugenio Pérez
	<eperezma@...hat.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] vhost/net: check peek_head_len after signal to
 guest to avoid delays



> On Nov 26, 2025, at 1:41 AM, Michael S. Tsirkin <mst@...hat.com> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> On Tue, Nov 25, 2025 at 11:00:33AM -0700, Jon Kohler wrote:
>> 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 <jon@...anix.com>
>> ---
>> 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

> 
> 
>> -- 
>> 2.43.0
> 

Powered by blists - more mailing lists