[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z92kRKwkDmcRbc41@LQ3V64L9R2>
Date: Fri, 21 Mar 2025 10:39:16 -0700
From: Joe Damato <jdamato@...tly.com>
To: Samiullah Khawaja <skhawaja@...gle.com>
Cc: Jakub Kicinski <kuba@...nel.org>,
"David S . Miller " <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
almasrymina@...gle.com, willemb@...gle.com, mkarsten@...terloo.ca,
netdev@...r.kernel.org
Subject: Re: [PATCH net-next v4 3/4] Extend napi threaded polling to allow
kthread based busy polling
On Fri, Mar 21, 2025 at 02:15:20AM +0000, Samiullah Khawaja wrote:
> Add a new state to napi state enum:
>
> - STATE_THREADED_BUSY_POLL
> Threaded busy poll is enabled/running for this napi.
>
> Following changes are introduced in the napi scheduling and state logic:
>
> - When threaded busy poll is enabled through sysfs it also enables
> NAPI_STATE_THREADED so a kthread is created per napi. It also sets
> NAPI_STATE_THREADED_BUSY_POLL bit on each napi to indicate that we are
> supposed to busy poll for each napi.
>
> - When napi is scheduled with STATE_SCHED_THREADED and associated
> kthread is woken up, the kthread owns the context. If
> NAPI_STATE_THREADED_BUSY_POLL and NAPI_SCHED_THREADED both are set
> then it means that we can busy poll.
>
> - To keep busy polling and to avoid scheduling of the interrupts, the
> napi_complete_done returns false when both SCHED_THREADED and
> THREADED_BUSY_POLL flags are set. Also napi_complete_done returns
> early to avoid the STATE_SCHED_THREADED being unset.
>
> - If at any point STATE_THREADED_BUSY_POLL is unset, the
> napi_complete_done will run and unset the SCHED_THREADED bit also.
> This will make the associated kthread go to sleep as per existing
> logic.
>
> Signed-off-by: Samiullah Khawaja <skhawaja@...gle.com>
> ---
> Documentation/ABI/testing/sysfs-class-net | 3 +-
> Documentation/netlink/specs/netdev.yaml | 12 ++-
> Documentation/networking/napi.rst | 67 ++++++++++++-
> .../net/ethernet/atheros/atl1c/atl1c_main.c | 2 +-
> drivers/net/ethernet/mellanox/mlxsw/pci.c | 2 +-
> drivers/net/ethernet/renesas/ravb_main.c | 2 +-
> drivers/net/wireless/ath/ath10k/snoc.c | 2 +-
> include/linux/netdevice.h | 20 +++-
> include/uapi/linux/netdev.h | 6 ++
> net/core/dev.c | 93 ++++++++++++++++---
> net/core/net-sysfs.c | 2 +-
> net/core/netdev-genl-gen.c | 2 +-
> net/core/netdev-genl.c | 2 +-
> tools/include/uapi/linux/netdev.h | 6 ++
> 14 files changed, 188 insertions(+), 33 deletions(-)
I think this should be split into two patches which would ease
review and bisection:
- First patch: introduce enum netdev_napi_threaded and
NETDEV_NAPI_THREADED_ENABLE and the associated driver changes.
- Second patch: introduce NETDEV_NAPI_THREADED_BUSY_POLL_ENABLE
I'll have to take a closer look at all the changes here after I've
read the cover letter and have reproduced the results, but one issue
stands out:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 3c244fd9ae6d..b990cbe76f86 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
[...]
> @@ -2432,7 +2442,7 @@ struct net_device {
> struct sfp_bus *sfp_bus;
> struct lock_class_key *qdisc_tx_busylock;
> bool proto_down;
> - bool threaded;
> + u8 threaded;
Doesn't
Documentation/networking/net_cachelines/net_device.rst
Also need to be updated if you are changing the width of this field
from bool to u8?
Powered by blists - more mailing lists