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. > > >> >> > >> > >> >> > >> >> > >> >> >> } >> >> >> } >> >> >> >> >> >> -- >> >> >> 2.25.1 >> >> >> >> >> >>
