Re:Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll

[Xuan Zhuo <xuanzhuo@linux.alibaba.com>]

On Tue, 16 Jun 2026 14:07:34 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> At 2026-06-16 11:27:12, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> >> At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
> >> >> From: Longjun Tang <tanglongjun@kylinos.cn>
> >> >>
> >> >> When busy-poll is active, napi_schedule_prep() returns false in
> >> >> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
> >> >> may keep firing irqs until the next poll round reaches
> >> >> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
> >> >> it will lead to a large number of spurious interrupts. Explicitly
> >> >> disable callbacks in this case to prevent spurious interrupts.
> >> >>
> >> >> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
> >> >> ---
> >> >>  drivers/net/virtio_net.c | 2 ++
> >> >>  1 file changed, 2 insertions(+)
> >> >>
> >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> >> index f4adcfee7a80..6d675fddc59b 100644
> >> >> --- a/drivers/net/virtio_net.c
> >> >> +++ b/drivers/net/virtio_net.c
> >> >> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
> >> >>  	if (napi_schedule_prep(napi)) {
> >> >>  		virtqueue_disable_cb(vq);
> >> >>  		__napi_schedule(napi);
> >> >> +	} else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
> >> >> +		virtqueue_disable_cb(vq);
> >> >
> >> >I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
> >> >drivers. The NIC driver should remain agnostic to busy polling. I think we need
> >> >a better way, maybe we should rewrite virtqueue_napi_schedule instead.
> >>
> >> How about rewrite it like this?
> >> static void virtqueue_napi_schedule(struct napi_struct *napi,
> >>                                     struct virtqueue *vq)
> >> {
> >>         virtqueue_disable_cb(vq);
> >>         if (napi_schedule_prep(napi))
> >>                 __napi_schedule(napi);
> >> }
> >> Any comments are welcome.
> >
> >
> >Another CPU could be running NAPI and has just enabled the callbacks (cb).
> >Meanwhile, this side unconditionally disables the cb. Since NAPI on the other
> >CPU hasn't exited yet, the subsequent prep on this side fails, leaving no one to
> >re-enable the cb.
> >
> >Thanks.
>
> Regarding the case you described, when NAPI on another CPU exits, the virtqueue_napi_complete func
> will be executed to re-enable cb.  and if there is still unconsumed data in the virtqueue, virtqueue_napi_schedule
> will be called again to schedule NAPI.
>
> In summary, I think that the disable_cb and __napi_schedule within the virtqueue_napi_schedule func do not need to be bound together.
>
> Any comments are welcome. Thinks.


<Your code>
static void virtqueue_napi_schedule(struct napi_struct *napi,
                                    struct virtqueue *vq)
{

							       |static bool virtqueue_napi_complete(struct napi_struct *napi,
							       |				    struct virtqueue *vq, int processed)
							       |{
							       |	int opaque;
							       |
							       |	opaque = virtqueue_enable_cb_prepare(vq);
                                                               |
        virtqueue_disable_cb(vq);                              |
        if (napi_schedule_prep(napi))                          |
                __napi_schedule(napi);                         |
							       |	if (napi_complete_done(napi, processed)) {
							       |		if (unlikely(virtqueue_poll(vq, opaque)))
							       |			virtqueue_napi_schedule(napi, vq);
							       |		else
							       |			 return true; // return directly
							       |	} else {
							       |		virtqueue_disable_cb(vq);
							       |	}
							       |
							       |	return false;
							       |}
}

1. new packets (notified by irq) are consumed by napi before virtqueue_napi_complete
2. poll is not called by irq, maybe xsk wake up. So irq is not disabled.


Thanks.


>
> >
> >
> >> >
> >> >
> >> >>  	}
> >> >>  }
> >> >>
> >> >> --
> >> >> 2.25.1
> >> >>
> >>
>