[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89i+4=Prs24Cn3hTBpb70f9COJyXm8e19vkPs05LnqwFXtA@mail.gmail.com>
Date: Thu, 29 Aug 2024 15:46:31 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Joe Damato <jdamato@...tly.com>
Cc: netdev@...r.kernel.org, amritha.nambiar@...el.com,
sridhar.samudrala@...el.com, sdf@...ichev.me, bjorn@...osinc.com,
hch@...radead.org, willy@...radead.org, willemdebruijn.kernel@...il.com,
skhawaja@...gle.com, kuba@...nel.org, Martin Karsten <mkarsten@...terloo.ca>,
"David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, Jiri Pirko <jiri@...nulli.us>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Lorenzo Bianconi <lorenzo@...nel.org>,
Breno Leitao <leitao@...ian.org>, Johannes Berg <johannes.berg@...el.com>,
Alexander Lobakin <aleksander.lobakin@...el.com>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/5] net: napi: Make napi_defer_hard_irqs per-NAPI
On Thu, Aug 29, 2024 at 3:13 PM Joe Damato <jdamato@...tly.com> wrote:
>
> Allow per-NAPI defer_hard_irqs setting.
>
> The existing sysfs parameter is respected; writes to sysfs will write to
> all NAPI structs for the device and the net_device defer_hard_irq field.
> Reads from sysfs will read from the net_device field.
>
> sysfs code was updated to guard against what appears to be a potential
> overflow as the field is an int, but the value passed in is an unsigned
> long.
>
> The ability to set defer_hard_irqs on specific NAPI instances will be
> added in a later commit, via netdev-genl.
>
> Signed-off-by: Joe Damato <jdamato@...tly.com>
> Reviewed-by: Martin Karsten <mkarsten@...terloo.ca>
> Tested-by: Martin Karsten <mkarsten@...terloo.ca>
> ---
> include/linux/netdevice.h | 23 +++++++++++++++++++++++
> net/core/dev.c | 29 ++++++++++++++++++++++++++---
> net/core/net-sysfs.c | 5 ++++-
> 3 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index fce70990b209..7d53380da4c0 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -371,6 +371,7 @@ struct napi_struct {
> struct list_head rx_list; /* Pending GRO_NORMAL skbs */
> int rx_count; /* length of rx_list */
> unsigned int napi_id;
> + int defer_hard_irqs;
> struct hrtimer timer;
> struct task_struct *thread;
> /* control-path-only fields follow */
> @@ -534,6 +535,28 @@ static inline void napi_schedule_irqoff(struct napi_struct *n)
> __napi_schedule_irqoff(n);
> }
>
Since dev->napi_defer_hard_irqs is no longer used in fast path,
please move it outside of the net_device_read_rx group.
Also update Documentation/networking/net_cachelines/net_device.rst
> +/**
> + * napi_get_defer_hard_irqs - get the NAPI's defer_hard_irqs
> + * @n: napi struct to get the defer_hard_irqs field from
> + *
> + * Returns the per-NAPI value of the defar_hard_irqs field.
> + */
> +int napi_get_defer_hard_irqs(const struct napi_struct *n);
> +
> +/**
> + * napi_set_defer_hard_irqs - set the defer_hard_irqs for a napi
> + * @n: napi_struct to set the defer_hard_irqs field
> + * @defer: the value the field should be set to
> + */
> +void napi_set_defer_hard_irqs(struct napi_struct *n, int defer);
> +
> +/**
> + * netdev_set_defer_hard_irqs - set defer_hard_irqs for all NAPIs of a netdev
> + * @netdev: the net_device for which all NAPIs will have their defer_hard_irqs set
> + * @defer: the defer_hard_irqs value to set
> + */
> +void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer);
> +
> /**
> * napi_complete_done - NAPI processing complete
> * @n: NAPI context
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 63987b8b7c85..f7baff0da057 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6212,6 +6212,28 @@ void __napi_schedule_irqoff(struct napi_struct *n)
> }
> EXPORT_SYMBOL(__napi_schedule_irqoff);
>
> +int napi_get_defer_hard_irqs(const struct napi_struct *n)
> +{
> + return READ_ONCE(n->defer_hard_irqs);
> +}
> +EXPORT_SYMBOL_GPL(napi_get_defer_hard_irqs);
> +
> +void napi_set_defer_hard_irqs(struct napi_struct *n, int defer)
> +{
> + WRITE_ONCE(n->defer_hard_irqs, defer);
> +}
> +EXPORT_SYMBOL_GPL(napi_set_defer_hard_irqs);
> +
> +void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer)
> +{
> + struct napi_struct *napi;
> +
> + WRITE_ONCE(netdev->napi_defer_hard_irqs, defer);
> + list_for_each_entry(napi, &netdev->napi_list, dev_list)
> + napi_set_defer_hard_irqs(napi, defer);
> +}
> +EXPORT_SYMBOL_GPL(netdev_set_defer_hard_irqs);
> +
> bool napi_complete_done(struct napi_struct *n, int work_done)
> {
> unsigned long flags, val, new, timeout = 0;
> @@ -6230,7 +6252,7 @@ bool napi_complete_done(struct napi_struct *n, int work_done)
> if (work_done) {
> if (n->gro_bitmask)
> timeout = READ_ONCE(n->dev->gro_flush_timeout);
> - n->defer_hard_irqs_count = READ_ONCE(n->dev->napi_defer_hard_irqs);
> + n->defer_hard_irqs_count = napi_get_defer_hard_irqs(n);
> }
> if (n->defer_hard_irqs_count > 0) {
> n->defer_hard_irqs_count--;
> @@ -6368,7 +6390,7 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock,
> bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);
>
> if (flags & NAPI_F_PREFER_BUSY_POLL) {
> - napi->defer_hard_irqs_count = READ_ONCE(napi->dev->napi_defer_hard_irqs);
> + napi->defer_hard_irqs_count = napi_get_defer_hard_irqs(napi);
> timeout = READ_ONCE(napi->dev->gro_flush_timeout);
> if (napi->defer_hard_irqs_count && timeout) {
> hrtimer_start(&napi->timer, ns_to_ktime(timeout), HRTIMER_MODE_REL_PINNED);
> @@ -6650,6 +6672,7 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
> INIT_HLIST_NODE(&napi->napi_hash_node);
> hrtimer_init(&napi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
> napi->timer.function = napi_watchdog;
> + napi_set_defer_hard_irqs(napi, READ_ONCE(dev->napi_defer_hard_irqs));
> init_gro_hash(napi);
> napi->skb = NULL;
> INIT_LIST_HEAD(&napi->rx_list);
> @@ -11032,7 +11055,7 @@ void netdev_sw_irq_coalesce_default_on(struct net_device *dev)
>
> if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
> dev->gro_flush_timeout = 20000;
> - dev->napi_defer_hard_irqs = 1;
> + netdev_set_defer_hard_irqs(dev, 1);
> }
> }
> EXPORT_SYMBOL_GPL(netdev_sw_irq_coalesce_default_on);
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 0e2084ce7b75..8272f0144d81 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -425,7 +425,10 @@ NETDEVICE_SHOW_RW(gro_flush_timeout, fmt_ulong);
>
> static int change_napi_defer_hard_irqs(struct net_device *dev, unsigned long val)
> {
> - WRITE_ONCE(dev->napi_defer_hard_irqs, val);
> + if (val > S32_MAX)
> + return -EINVAL;
> +
> + netdev_set_defer_hard_irqs(dev, (int)val);
> return 0;
> }
>
> --
> 2.25.1
>
Powered by blists - more mailing lists