On Tue, Jun 16, 2026 at 02:49:25PM +0800, Xuan Zhuo wrote:
> On Tue, 16 Jun 2026 14:07:34 +0800 (CST), Lange Tang <[email protected]>
> wrote:
> > At 2026-06-16 11:27:12, "Xuan Zhuo" <[email protected]> wrote:
> > >On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <[email protected]>
> > >wrote:
> > >> At 2026-06-15 18:01:40, "Xuan Zhuo" <[email protected]> wrote:
> > >> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <[email protected]>
> > >> >wrote:
> > >> >> From: Longjun Tang <[email protected]>
> > >> >>
> > >> >> 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 <[email protected]>
> > >> >> ---
> > >> >> 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.
I thought more about it and - so what? It's a rare race that
might result in more interrupts. Why is that an issue?
This would be a problem:
callbacks disabled
no NAPI
and I do not see how this gets there.
so maybe
virtqueue_disable_cb(vq);
napi_schedule(napi);
is exactly the right thing?
it's certainly simple.
>
> >
> > >
> > >
> > >> >
> > >> >
> > >> >> }
> > >> >> }
> > >> >>
> > >> >> --
> > >> >> 2.25.1
> > >> >>
> > >>
> >
