On 12/07/2019 17:48, Eric Dumazet wrote: >> but the rest is the stuff we're polling for for low latency. >> I'm putting a gro_normal_list() call after the trace_napi_poll() in >> napi_busy_loop() and testing that, let's see how it goes... One thing that's causing me some uncertainty: busy_poll_stop() does a napi->poll(), which can potentially gro_normal_one() something. But when I tried to put a gro_normal_list() just after that, I ran into list corruption because it could race against the one in napi_complete_done(). I'm not entirely sure how, my current theory goes something like: - clear_bit(IN_BUSY_POLL) - task switch, start napi poll - get as far as starting gro_normal_list() - task switch back to busy_poll_stop() - local_bh_disable() - do a napi poll - start gro_normal_list() - list corruption ensues as we have two instances of netif_receive_skb_list_internal() trying to consume the same list But I may be wildly mistaken. Questions that arise from that: 1) Is it safe to potentially be adding to the rx_list (gro_normal_one(), which in theory can end up calling gro_normal_list() as well) within busy_poll_stop()? I haven't ever seen a splat from that, but it seems every bit as possible as what I have been seeing. 2) Why does busy_poll_stop() not do its local_bh_disable() *before* clearing napi state bits, which (if I'm understanding correctly) would ensure an ordinary napi poll can't race with the one in busy_poll_stop()?
Apart from that I have indeed established that with the patches as posted busy-polling latency is awful, but adding a gro_normal_list() into napi_busy_loop() fixes that, as expected. -Ed
