On 10/07/2019 16:41, Eric Dumazet wrote: > On 7/10/19 4:52 PM, Edward Cree wrote: >> Hmm, I was caught out by the call to napi_poll() actually being a local >> function pointer, not the static function of the same name. How did a >> shadow like that ever get allowed? >> But in that case I _really_ don't understand napi_busy_loop(); nothing >> in it seems to ever flush GRO, so it's relying on either >> (1) stuff getting flushed because the bucket runs out of space, or >> (2) the next napi poll after busy_poll_stop() doing the flush. >> What am I missing, and where exactly in napi_busy_loop() should the >> gro_normal_list() call go? > Please look at busy_poll_stop() I did look there, but now I've looked again and harder, and I think I get it: busy_poll_stop() calls napi->poll(), which (eventually, possibly in the subsequent poll that we schedule if rc == budget) calls napi_complete_done() which does the flush. In which case, the same applies to the napi->rx_list, which similarly gets handled in napi_complete_done(). So I don't think napi_busy_loop() needs a gro_normal_list() adding to it(?)
As a general rule, I think we need to gro_normal_list() in those places, and only those places, that call napi_gro_flush(). But as I mentioned in the patch 3/3 description, I'm still confused by the (few) drivers that call napi_gro_flush() themselves. -Ed
