* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-25 18:00 [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays Jon Kohler
@ 2025-11-25 23:50 ` Michael S. Tsirkin
2025-11-26 16:49 ` Jon Kohler
2025-11-26 6:15 ` Jason Wang
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2025-11-25 23:50 UTC (permalink / raw)
To: Jon Kohler
Cc: Jason Wang, Eugenio Pérez, kvm, virtualization, netdev,
linux-kernel
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@nutanix.com>
Given this is supposed to be a performance improvement,
pls include info on the effect this has on performance. Thanks!
> ---
> 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);
> --
> 2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-25 23:50 ` Michael S. Tsirkin
@ 2025-11-26 16:49 ` Jon Kohler
2025-12-26 14:50 ` Michael S. Tsirkin
0 siblings, 1 reply; 10+ messages in thread
From: Jon Kohler @ 2025-11-26 16:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Eugenio Pérez, kvm@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
> On Nov 25, 2025, at 6:50 PM, Michael S. Tsirkin <mst@redhat.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@nutanix.com>
>
> Given this is supposed to be a performance improvement,
> pls include info on the effect this has on performance. Thanks!
I had already mentioned we’re avoiding ~20k schedulers/IPIs in that
example, but I can add more detail. Let’s resolve the other parts of
the thread first and go from there?
>
>> ---
>> 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);
>> --
>> 2.43.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-26 16:49 ` Jon Kohler
@ 2025-12-26 14:50 ` Michael S. Tsirkin
0 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2025-12-26 14:50 UTC (permalink / raw)
To: Jon Kohler
Cc: Jason Wang, Eugenio Pérez, kvm@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
On Wed, Nov 26, 2025 at 04:49:11PM +0000, Jon Kohler wrote:
>
>
> > On Nov 25, 2025, at 6:50 PM, Michael S. Tsirkin <mst@redhat.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@nutanix.com>
> >
> > Given this is supposed to be a performance improvement,
> > pls include info on the effect this has on performance. Thanks!
>
> I had already mentioned we’re avoiding ~20k schedulers/IPIs in that
> example, but I can add more detail. Let’s resolve the other parts of
> the thread first and go from there?
the discussion seems to have died down.
I suggest reposting with perf data you have
(which test, how much improvement, what cpu usage)
collected in the commit log.
thanks!
> >
> >> ---
> >> 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);
> >> --
> >> 2.43.0
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-25 18:00 [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays Jon Kohler
2025-11-25 23:50 ` Michael S. Tsirkin
@ 2025-11-26 6:15 ` Jason Wang
2025-11-26 16:48 ` Jon Kohler
2025-11-26 6:41 ` Michael S. Tsirkin
2025-11-26 8:56 ` Michael S. Tsirkin
3 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2025-11-26 6:15 UTC (permalink / raw)
To: Jon Kohler
Cc: Michael S. Tsirkin, Eugenio Pérez, kvm, virtualization,
netdev, linux-kernel
On Wed, Nov 26, 2025 at 1:18 AM Jon Kohler <jon@nutanix.com> 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@nutanix.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;
> + }
> + }
I'm not sure I will get here.
Once vhost_net_rx_peek_head_len() returns 0, we exit the loop to:
if (unlikely(busyloop_intr))
vhost_poll_queue(&vq->poll);
else if (!sock_len)
vhost_net_enable_vq(net, vq);
out:
vhost_net_signal_used(nvq, count);
Are you suggesting signalling before enabling vq actually?
Thanks
> +
> if (!len && rvq->busyloop_timeout) {
> /* Flush batched heads first */
> vhost_net_signal_used(rnvq, *count);
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-26 6:15 ` Jason Wang
@ 2025-11-26 16:48 ` Jon Kohler
0 siblings, 0 replies; 10+ messages in thread
From: Jon Kohler @ 2025-11-26 16:48 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
> On Nov 26, 2025, at 1:15 AM, Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Nov 26, 2025 at 1:18 AM Jon Kohler <jon@nutanix.com> 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@nutanix.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;
>> + }
>> + }
>
> I'm not sure I will get here.
>
> Once vhost_net_rx_peek_head_len() returns 0, we exit the loop to:
>
> if (unlikely(busyloop_intr))
> vhost_poll_queue(&vq->poll);
> else if (!sock_len)
> vhost_net_enable_vq(net, vq);
> out:
> vhost_net_signal_used(nvq, count);
>
> Are you suggesting signalling before enabling vq actually?
See my other note I just sent, yes, thats exactly what I’m suggesting
Signaling takes some time, and if we do that before we do our last
peek for work, we can pick up racing additions to the ring, and avoid
a trip to scheduler and IPIs, etc
>
> Thanks
>
>> +
>> if (!len && rvq->busyloop_timeout) {
>> /* Flush batched heads first */
>> vhost_net_signal_used(rnvq, *count);
>> --
>> 2.43.0
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-25 18:00 [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays Jon Kohler
2025-11-25 23:50 ` Michael S. Tsirkin
2025-11-26 6:15 ` Jason Wang
@ 2025-11-26 6:41 ` Michael S. Tsirkin
2025-11-26 16:47 ` Jon Kohler
2025-11-26 8:56 ` Michael S. Tsirkin
3 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2025-11-26 6:41 UTC (permalink / raw)
To: Jon Kohler
Cc: Jason Wang, Eugenio Pérez, kvm, virtualization, netdev,
linux-kernel
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@nutanix.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?
> --
> 2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-26 6:41 ` Michael S. Tsirkin
@ 2025-11-26 16:47 ` Jon Kohler
0 siblings, 0 replies; 10+ messages in thread
From: Jon Kohler @ 2025-11-26 16:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Eugenio Pérez, kvm@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
> On Nov 26, 2025, at 1:41 AM, Michael S. Tsirkin <mst@redhat.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@nutanix.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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-25 18:00 [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays Jon Kohler
` (2 preceding siblings ...)
2025-11-26 6:41 ` Michael S. Tsirkin
@ 2025-11-26 8:56 ` Michael S. Tsirkin
2025-11-26 16:43 ` Jon Kohler
3 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2025-11-26 8:56 UTC (permalink / raw)
To: Jon Kohler
Cc: Jason Wang, Eugenio Pérez, kvm, virtualization, netdev,
linux-kernel
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@nutanix.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)) {
I also wonder why don't we assign len here.
I get the point about being fair to TX but it's not
indefinite poll, just a single peek ...
> + /* 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);
> --
> 2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] vhost/net: check peek_head_len after signal to guest to avoid delays
2025-11-26 8:56 ` Michael S. Tsirkin
@ 2025-11-26 16:43 ` Jon Kohler
0 siblings, 0 replies; 10+ messages in thread
From: Jon Kohler @ 2025-11-26 16:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Eugenio Pérez, kvm@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
> On Nov 26, 2025, at 3:56 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> 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@nutanix.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)) {
>
>
> I also wonder why don't we assign len here.
> I get the point about being fair to TX but it's not
> indefinite poll, just a single peek …
The first version I made of this patch did this. It works,
but I liked the idea of having baked in fairness design
wise. It could go either way though?
The nice bit about deferring to the TX handler (or other
work) is that you’d then naturally batch up more work,
so the ebb n flow should be nicer in a mixed load environment
Thoughts?
>> + /* 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);
>> --
>> 2.43.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread