At 2026-08-10 15:13:14, "Michael S. Tsirkin" <[email protected]> wrote: >On Wed, Jun 17, 2026 at 10:08:55AM +0800, Lange Tang wrote: >> At 2026-06-16 14:49:25, "Xuan Zhuo" <[email protected]> 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. >> >> Based on your code analysis above, I got . thanks. >> disable_cb and __napi_schedule in the virtqueue_napi_schedule func indeed >> cannot be separated. >> >> Regarding the issue of not being able to disable cb in a busy-poll context, >> do you have any suggestions? >> >> Any comments are welcome. Thanks. > > >Add a netdev core API that basically does: > >test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state) > >add documentation, either netdev maintainers merge it, or >alternatively they will tell you what to do)
This issue has already been fixed with https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1eb8fc67ca41db71c90866ff76c990d85247daef, and it was merged. so maybe netdev core API no longer needed. thanks. Lange > >> > >> > >> >> >> >> > >> >> > >> >> >> > >> >> >> > >> >> >> >> } >> >> >> >> } >> >> >> >> >> >> >> >> -- >> >> >> >> 2.25.1 >> >> >> >> >> >> >> >> >> >>
