On Tue, 8 Dec 2020 16:54:43 -0800 Wei Wang wrote:
> This patch allows running each napi poll loop inside its own
> kernel thread.
> The threaded mode could be enabled through napi_set_threaded()
> api, and does not require a device up/down. The kthread gets
> created on demand when napi_set_threaded() is called, and gets
> shut down eventually in napi_disable().
>
> Once that threaded mode is enabled and the kthread is
> started, napi_schedule() will wake-up such thread instead
> of scheduling the softirq.
>
> The threaded poll loop behaves quite likely the net_rx_action,
> but it does not have to manipulate local irqs and uses
> an explicit scheduling point based on netdev_budget.
>
> Co-developed-by: Paolo Abeni <[email protected]>
> Signed-off-by: Paolo Abeni <[email protected]>
> Co-developed-by: Hannes Frederic Sowa <[email protected]>
> Signed-off-by: Hannes Frederic Sowa <[email protected]>
> Co-developed-by: Jakub Kicinski <[email protected]>
> Signed-off-by: Jakub Kicinski <[email protected]>
> Signed-off-by: Wei Wang <[email protected]>
> Reviewed-by: Eric Dumazet <[email protected]>
> @@ -4234,6 +4265,11 @@ int gro_normal_batch __read_mostly = 8;
> static inline void ____napi_schedule(struct softnet_data *sd,
> struct napi_struct *napi)
> {
> + if (test_bit(NAPI_STATE_THREADED, &napi->state)) {
> + wake_up_process(napi->thread);
FTR your implementation depends on the fact that this is the only
place that can wake the worker and not set kthread_should_stop().
Which I trust you is the case :) maybe I already mentioned this..
> + return;
> + }
> +
> list_add_tail(&napi->poll_list, &sd->poll_list);
> __raise_softirq_irqoff(NET_RX_SOFTIRQ);
> }
> void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
> int (*poll)(struct napi_struct *, int), int weight)
> {
> @@ -6731,6 +6790,7 @@ void napi_disable(struct napi_struct *n)
> msleep(1);
>
> hrtimer_cancel(&n->timer);
> + napi_kthread_stop(n);
I'm surprised that we stop the thread on napi_disable() but there is no
start/create in napi_enable(). NAPIs can (and do get) disabled and
enabled again. But that'd make your code crash with many popular
drivers if you tried to change rings with threaded napi enabled so I
feel like I must be missing something..
> clear_bit(NAPI_STATE_PREFER_BUSY_POLL, &n->state);
> clear_bit(NAPI_STATE_DISABLE, &n->state);
