Messages in this thread | | | From | Jesper Dangaard Brouer <> | Date | Mon, 6 Feb 2023 11:18:49 +0100 | Subject | Re: [PATCH net] net: page_pool: use in_softirq() instead |
| |
On 03/02/2023 14.05, DENG Qingfang wrote: > Hi Jesper, > > On Fri, Feb 3, 2023 at 7:15 PM Jesper Dangaard Brouer > <jbrouer@redhat.com> wrote: >> How can I enable threaded NAPI on my system? > > dev_set_threaded(napi_dev, true); > > You can also enable it at runtime by writing 1 to > /sys/class/net/<devname>/threaded, but it only works if the driver > does _not_ use a dummy netdev for NAPI poll. >
Thanks for providing this setup info.
I quickly tested driver i40e with a XDP_DROP workload, and switch between the threaded and normal NAPI, and no performance difference. (p.s. driver i40e doesn't use page_pool)
>> I think other cases (above) are likely safe, but I worry a little about >> this case, as the page_pool_recycle_in_cache() rely on RX-NAPI protection. >> Meaning it is only the CPU that handles RX-NAPI for this RX-queue that >> is allowed to access this lockless array. > > The major changes to the threaded NAPI is that instead of scheduling a > NET_RX softirq, it wakes up a kthread which then does the polling, > allowing it to be scheduled to another CPU. The driver's poll function > is called with BH disabled so it's still considered BH context. >
As long as drivers NAPI poll function doesn't migrate between CPUs while it is running this should be fine. (This guarantee is needed as XDP + TC have per_cpu bpf_redirect_info).
Looking at the code napi_threaded_poll() in net/core/dev.c I can see this is guarantee is provided by the local_bh_disable() + local_bh_enable around the call to __napi_poll().
>> We do have the 'allow_direct' boolean, and if every driver/user uses >> this correctly, then this should be safe. Changing this makes it >> possible for drivers to use page_pool API incorrectly and this leads to >> hard-to-debug errors. > > "incorrectly", like, calling it outside RX-NAPI?
Yes.
--Jesper
| |