[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89iJE01V4TBCXg=w=M8=75TXypuYMJ_pXBUrN9NdRRAtAZg@mail.gmail.com>
Date: Fri, 23 Aug 2024 19:39:56 +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, peter@...eblog.net,
m2shafiei@...terloo.ca, 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>,
Daniel Borkmann <daniel@...earbox.net>, Breno Leitao <leitao@...ian.org>,
Johannes Berg <johannes.berg@...el.com>, Heiner Kallweit <hkallweit1@...il.com>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/6] net: Add sysfs parameter irq_suspend_timeout
On Fri, Aug 23, 2024 at 7:31 PM Joe Damato <jdamato@...tly.com> wrote:
>
> From: Martin Karsten <mkarsten@...terloo.ca>
>
> This patch doesn't change any behavior but prepares the code for other
> changes in the following commits which use irq_suspend_timeout as a
> timeout for IRQ suspension.
>
> Signed-off-by: Martin Karsten <mkarsten@...terloo.ca>
> Co-developed-by: Joe Damato <jdamato@...tly.com>
> Signed-off-by: Joe Damato <jdamato@...tly.com>
> Tested-by: Joe Damato <jdamato@...tly.com>
> Tested-by: Martin Karsten <mkarsten@...terloo.ca>
> ---
> rfc -> v1:
> - Removed napi.rst documentation from this patch; added to patch 6.
>
> include/linux/netdevice.h | 2 ++
> net/core/dev.c | 3 ++-
> net/core/net-sysfs.c | 18 ++++++++++++++++++
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 0ef3eaa23f4b..31867bb2ff65 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1857,6 +1857,7 @@ enum netdev_reg_state {
> * @gro_flush_timeout: timeout for GRO layer in NAPI
> * @napi_defer_hard_irqs: If not zero, provides a counter that would
> * allow to avoid NIC hard IRQ, on busy queues.
> + * @irq_suspend_timeout: IRQ suspension timeout
> *
> * @rx_handler: handler for received packets
> * @rx_handler_data: XXX: need comments on this one
> @@ -2060,6 +2061,7 @@ struct net_device {
> struct netdev_rx_queue *_rx;
> unsigned long gro_flush_timeout;
> int napi_defer_hard_irqs;
> + unsigned long irq_suspend_timeout;
> unsigned int gro_max_size;
> unsigned int gro_ipv4_max_size;
> rx_handler_func_t __rcu *rx_handler;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index e7260889d4cb..3bf325ec25a3 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -11945,6 +11945,7 @@ static void __init net_dev_struct_check(void)
> CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, _rx);
> CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, gro_flush_timeout);
> CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, napi_defer_hard_irqs);
> + CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, irq_suspend_timeout);
> CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, gro_max_size);
> CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, gro_ipv4_max_size);
> CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, rx_handler);
> @@ -11956,7 +11957,7 @@ static void __init net_dev_struct_check(void)
> #ifdef CONFIG_NET_XGRESS
> CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_rx, tcx_ingress);
> #endif
> - CACHELINE_ASSERT_GROUP_SIZE(struct net_device, net_device_read_rx, 104);
> + CACHELINE_ASSERT_GROUP_SIZE(struct net_device, net_device_read_rx, 112);
> }
>
> /*
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 0e2084ce7b75..fb6f3327310f 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -440,6 +440,23 @@ static ssize_t napi_defer_hard_irqs_store(struct device *dev,
> }
> NETDEVICE_SHOW_RW(napi_defer_hard_irqs, fmt_dec);
>
> +static int change_irq_suspend_timeout(struct net_device *dev, unsigned long val)
> +{
> + WRITE_ONCE(dev->irq_suspend_timeout, val);
> + return 0;
> +}
> +
> +static ssize_t irq_suspend_timeout_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + return netdev_store(dev, attr, buf, len, change_irq_suspend_timeout);
> +}
> +NETDEVICE_SHOW_RW(irq_suspend_timeout, fmt_ulong);
> +
> static ssize_t ifalias_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t len)
> {
> @@ -664,6 +681,7 @@ static struct attribute *net_class_attrs[] __ro_after_init = {
> &dev_attr_tx_queue_len.attr,
> &dev_attr_gro_flush_timeout.attr,
> &dev_attr_napi_defer_hard_irqs.attr,
> + &dev_attr_irq_suspend_timeout.attr,
> &dev_attr_phys_port_id.attr,
> &dev_attr_phys_port_name.attr,
> &dev_attr_phys_switch_id.attr,
> --
> 2.25.1
Please no more per-device sysfs entry, shared by all the users of the device.
Let's not repeat past mistakes.
Nowadays, we need/want per receive-queue tuning, preferably set with netlink.
Powered by blists - more mailing lists