| Message ID | 20240829131214.169977-4-jdamato@fastly.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add support for per-NAPI config via netlink | expand |
On Thu, Aug 29, 2024 at 3:13 PM Joe Damato <jdamato@fastly.com> wrote: > > Allow per-NAPI gro_flush_timeout setting. > > The existing sysfs parameter is respected; writes to sysfs will write to > all NAPI structs for the device and the net_device gro_flush_timeout > field. Reads from sysfs will read from the net_device field. > > The ability to set gro_flush_timeout on specific NAPI instances will be > added in a later commit, via netdev-genl. > > Signed-off-by: Joe Damato <jdamato@fastly.com> > Reviewed-by: Martin Karsten <mkarsten@uwaterloo.ca> > Tested-by: Martin Karsten <mkarsten@uwaterloo.ca> > --- > include/linux/netdevice.h | 26 ++++++++++++++++++++++++++ > net/core/dev.c | 32 ++++++++++++++++++++++++++++---- > net/core/net-sysfs.c | 2 +- > 3 files changed, 55 insertions(+), 5 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 7d53380da4c0..d00024d9f857 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -372,6 +372,7 @@ struct napi_struct { > int rx_count; /* length of rx_list */ > unsigned int napi_id; > int defer_hard_irqs; > + unsigned long gro_flush_timeout; > struct hrtimer timer; > struct task_struct *thread; > /* control-path-only fields follow */ > @@ -557,6 +558,31 @@ void napi_set_defer_hard_irqs(struct napi_struct *n, int defer); > */ > void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer); > Same remark : dev->gro_flush_timeout is no longer read in the fast path. Please move gro_flush_timeout out of net_device_read_txrx and update Documentation/networking/net_cachelines/net_device.rst > +/** > + * napi_get_gro_flush_timeout - get the gro_flush_timeout > + * @n: napi struct to get the gro_flush_timeout from > + * > + * Returns the per-NAPI value of the gro_flush_timeout field. > + */ > +unsigned long napi_get_gro_flush_timeout(const struct napi_struct *n); > + > +/** > + * napi_set_gro_flush_timeout - set the gro_flush_timeout for a napi > + * @n: napi struct to set the gro_flush_timeout > + * @timeout: timeout value to set > + * > + * napi_set_gro_flush_timeout sets the per-NAPI gro_flush_timeout > + */ > +void napi_set_gro_flush_timeout(struct napi_struct *n, unsigned long timeout); > + > +/** > + * netdev_set_gro_flush_timeout - set gro_flush_timeout for all NAPIs of a netdev > + * @netdev: the net_device for which all NAPIs will have their gro_flush_timeout set > + * @timeout: the timeout value to set > + */ > +void netdev_set_gro_flush_timeout(struct net_device *netdev, > + unsigned long timeout); > + > /** > * napi_complete_done - NAPI processing complete > * @n: NAPI context > diff --git a/net/core/dev.c b/net/core/dev.c > index f7baff0da057..3f7cb1085efa 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -6234,6 +6234,29 @@ void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer) > } > EXPORT_SYMBOL_GPL(netdev_set_defer_hard_irqs); > > +unsigned long napi_get_gro_flush_timeout(const struct napi_struct *n) > +{ > + return READ_ONCE(n->gro_flush_timeout); > +} > +EXPORT_SYMBOL_GPL(napi_get_gro_flush_timeout); > + > +void napi_set_gro_flush_timeout(struct napi_struct *n, unsigned long timeout) > +{ > + WRITE_ONCE(n->gro_flush_timeout, timeout); > +} > +EXPORT_SYMBOL_GPL(napi_set_gro_flush_timeout); > + > +void netdev_set_gro_flush_timeout(struct net_device *netdev, > + unsigned long timeout) > +{ > + struct napi_struct *napi; > + > + WRITE_ONCE(netdev->gro_flush_timeout, timeout); > + list_for_each_entry(napi, &netdev->napi_list, dev_list) > + napi_set_gro_flush_timeout(napi, timeout); > +} > +EXPORT_SYMBOL_GPL(netdev_set_gro_flush_timeout); > + > bool napi_complete_done(struct napi_struct *n, int work_done) > { > unsigned long flags, val, new, timeout = 0; > @@ -6251,12 +6274,12 @@ 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); > + timeout = napi_get_gro_flush_timeout(n); > n->defer_hard_irqs_count = napi_get_defer_hard_irqs(n); > } > if (n->defer_hard_irqs_count > 0) { > n->defer_hard_irqs_count--; > - timeout = READ_ONCE(n->dev->gro_flush_timeout); > + timeout = napi_get_gro_flush_timeout(n); > if (timeout) > ret = false; > } > @@ -6391,7 +6414,7 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock, > > if (flags & NAPI_F_PREFER_BUSY_POLL) { > napi->defer_hard_irqs_count = napi_get_defer_hard_irqs(napi); > - timeout = READ_ONCE(napi->dev->gro_flush_timeout); > + timeout = napi_get_gro_flush_timeout(napi); > if (napi->defer_hard_irqs_count && timeout) { > hrtimer_start(&napi->timer, ns_to_ktime(timeout), HRTIMER_MODE_REL_PINNED); > skip_schedule = true; > @@ -6673,6 +6696,7 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi, > 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)); > + napi_set_gro_flush_timeout(napi, READ_ONCE(dev->gro_flush_timeout)); > init_gro_hash(napi); > napi->skb = NULL; > INIT_LIST_HEAD(&napi->rx_list); > @@ -11054,7 +11078,7 @@ void netdev_sw_irq_coalesce_default_on(struct net_device *dev) > WARN_ON(dev->reg_state == NETREG_REGISTERED); > > if (!IS_ENABLED(CONFIG_PREEMPT_RT)) { > - dev->gro_flush_timeout = 20000; > + netdev_set_gro_flush_timeout(dev, 20000); > netdev_set_defer_hard_irqs(dev, 1); > } > } > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > index 8272f0144d81..ff545a422b1f 100644 > --- a/net/core/net-sysfs.c > +++ b/net/core/net-sysfs.c > @@ -408,7 +408,7 @@ NETDEVICE_SHOW_RW(tx_queue_len, fmt_dec); > > static int change_gro_flush_timeout(struct net_device *dev, unsigned long val) > { > - WRITE_ONCE(dev->gro_flush_timeout, val); > + netdev_set_gro_flush_timeout(dev, val); > return 0; > } > > -- > 2.25.1 >
On Thu, Aug 29, 2024 at 03:48:05PM +0200, Eric Dumazet wrote: > On Thu, Aug 29, 2024 at 3:13 PM Joe Damato <jdamato@fastly.com> wrote: > > > > Allow per-NAPI gro_flush_timeout setting. > > > > The existing sysfs parameter is respected; writes to sysfs will write to > > all NAPI structs for the device and the net_device gro_flush_timeout > > field. Reads from sysfs will read from the net_device field. > > > > The ability to set gro_flush_timeout on specific NAPI instances will be > > added in a later commit, via netdev-genl. > > > > Signed-off-by: Joe Damato <jdamato@fastly.com> > > Reviewed-by: Martin Karsten <mkarsten@uwaterloo.ca> > > Tested-by: Martin Karsten <mkarsten@uwaterloo.ca> > > --- > > include/linux/netdevice.h | 26 ++++++++++++++++++++++++++ > > net/core/dev.c | 32 ++++++++++++++++++++++++++++---- > > net/core/net-sysfs.c | 2 +- > > 3 files changed, 55 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > index 7d53380da4c0..d00024d9f857 100644 > > --- a/include/linux/netdevice.h > > +++ b/include/linux/netdevice.h > > @@ -372,6 +372,7 @@ struct napi_struct { > > int rx_count; /* length of rx_list */ > > unsigned int napi_id; > > int defer_hard_irqs; > > + unsigned long gro_flush_timeout; > > struct hrtimer timer; > > struct task_struct *thread; > > /* control-path-only fields follow */ > > @@ -557,6 +558,31 @@ void napi_set_defer_hard_irqs(struct napi_struct *n, int defer); > > */ > > void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer); > > > > Same remark : dev->gro_flush_timeout is no longer read in the fast path. > > Please move gro_flush_timeout out of net_device_read_txrx and update > Documentation/networking/net_cachelines/net_device.rst Thanks, Eric. I will take care of both in the v2.
On Thu, Aug 29, 2024 at 03:48:05PM +0200, Eric Dumazet wrote: > On Thu, Aug 29, 2024 at 3:13 PM Joe Damato <jdamato@fastly.com> wrote: > > > > Allow per-NAPI gro_flush_timeout setting. > > > > The existing sysfs parameter is respected; writes to sysfs will write to > > all NAPI structs for the device and the net_device gro_flush_timeout > > field. Reads from sysfs will read from the net_device field. > > > > The ability to set gro_flush_timeout on specific NAPI instances will be > > added in a later commit, via netdev-genl. > > > > Signed-off-by: Joe Damato <jdamato@fastly.com> > > Reviewed-by: Martin Karsten <mkarsten@uwaterloo.ca> > > Tested-by: Martin Karsten <mkarsten@uwaterloo.ca> > > --- > > include/linux/netdevice.h | 26 ++++++++++++++++++++++++++ > > net/core/dev.c | 32 ++++++++++++++++++++++++++++---- > > net/core/net-sysfs.c | 2 +- > > 3 files changed, 55 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > index 7d53380da4c0..d00024d9f857 100644 > > --- a/include/linux/netdevice.h > > +++ b/include/linux/netdevice.h > > @@ -372,6 +372,7 @@ struct napi_struct { > > int rx_count; /* length of rx_list */ > > unsigned int napi_id; > > int defer_hard_irqs; > > + unsigned long gro_flush_timeout; > > struct hrtimer timer; > > struct task_struct *thread; > > /* control-path-only fields follow */ > > @@ -557,6 +558,31 @@ void napi_set_defer_hard_irqs(struct napi_struct *n, int defer); > > */ > > void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer); > > > > Same remark : dev->gro_flush_timeout is no longer read in the fast path. > > Please move gro_flush_timeout out of net_device_read_txrx and update > Documentation/networking/net_cachelines/net_device.rst Is there some tooling I should use to generate this file? I am asking because it seems like the file is missing two fields in net_device at the end of the struct: struct hlist_head page_pools; struct dim_irq_moder * irq_moder; Both of which seem to have been added just before and long after (respectively) commit 14006f1d8fa2 ("Documentations: Analyze heavily used Networking related structs"). If this is a bug, I can submit one patch (with two fixes tags) which adds both fields to the file? - Joe
On Thu, Aug 29, 2024 at 5:28 PM Joe Damato <jdamato@fastly.com> wrote: > > On Thu, Aug 29, 2024 at 03:48:05PM +0200, Eric Dumazet wrote: > > On Thu, Aug 29, 2024 at 3:13 PM Joe Damato <jdamato@fastly.com> wrote: > > > > > > Allow per-NAPI gro_flush_timeout setting. > > > > > > The existing sysfs parameter is respected; writes to sysfs will write to > > > all NAPI structs for the device and the net_device gro_flush_timeout > > > field. Reads from sysfs will read from the net_device field. > > > > > > The ability to set gro_flush_timeout on specific NAPI instances will be > > > added in a later commit, via netdev-genl. > > > > > > Signed-off-by: Joe Damato <jdamato@fastly.com> > > > Reviewed-by: Martin Karsten <mkarsten@uwaterloo.ca> > > > Tested-by: Martin Karsten <mkarsten@uwaterloo.ca> > > > --- > > > include/linux/netdevice.h | 26 ++++++++++++++++++++++++++ > > > net/core/dev.c | 32 ++++++++++++++++++++++++++++---- > > > net/core/net-sysfs.c | 2 +- > > > 3 files changed, 55 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > > index 7d53380da4c0..d00024d9f857 100644 > > > --- a/include/linux/netdevice.h > > > +++ b/include/linux/netdevice.h > > > @@ -372,6 +372,7 @@ struct napi_struct { > > > int rx_count; /* length of rx_list */ > > > unsigned int napi_id; > > > int defer_hard_irqs; > > > + unsigned long gro_flush_timeout; > > > struct hrtimer timer; > > > struct task_struct *thread; > > > /* control-path-only fields follow */ > > > @@ -557,6 +558,31 @@ void napi_set_defer_hard_irqs(struct napi_struct *n, int defer); > > > */ > > > void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer); > > > > > > > Same remark : dev->gro_flush_timeout is no longer read in the fast path. > > > > Please move gro_flush_timeout out of net_device_read_txrx and update > > Documentation/networking/net_cachelines/net_device.rst > > Is there some tooling I should use to generate this file? > > I am asking because it seems like the file is missing two fields in > net_device at the end of the struct: > > struct hlist_head page_pools; > struct dim_irq_moder * irq_moder; > At first glance this is control path only, no big deal. > Both of which seem to have been added just before and long after > (respectively) commit 14006f1d8fa2 ("Documentations: Analyze heavily > used Networking related structs"). > > If this is a bug, I can submit one patch (with two fixes tags) which > adds both fields to the file? No need for a Fixes: tag for this, just submit to net-next. This file is really 'needed' for current development, for people caring about data locality.
On Thu, Aug 29, 2024 at 05:31:30PM +0200, Eric Dumazet wrote: > On Thu, Aug 29, 2024 at 5:28 PM Joe Damato <jdamato@fastly.com> wrote: > > > > On Thu, Aug 29, 2024 at 03:48:05PM +0200, Eric Dumazet wrote: > > > On Thu, Aug 29, 2024 at 3:13 PM Joe Damato <jdamato@fastly.com> wrote: > > > > > > > > Allow per-NAPI gro_flush_timeout setting. > > > > > > > > The existing sysfs parameter is respected; writes to sysfs will write to > > > > all NAPI structs for the device and the net_device gro_flush_timeout > > > > field. Reads from sysfs will read from the net_device field. > > > > > > > > The ability to set gro_flush_timeout on specific NAPI instances will be > > > > added in a later commit, via netdev-genl. > > > > > > > > Signed-off-by: Joe Damato <jdamato@fastly.com> > > > > Reviewed-by: Martin Karsten <mkarsten@uwaterloo.ca> > > > > Tested-by: Martin Karsten <mkarsten@uwaterloo.ca> > > > > --- > > > > include/linux/netdevice.h | 26 ++++++++++++++++++++++++++ > > > > net/core/dev.c | 32 ++++++++++++++++++++++++++++---- > > > > net/core/net-sysfs.c | 2 +- > > > > 3 files changed, 55 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > > > index 7d53380da4c0..d00024d9f857 100644 > > > > --- a/include/linux/netdevice.h > > > > +++ b/include/linux/netdevice.h > > > > @@ -372,6 +372,7 @@ struct napi_struct { > > > > int rx_count; /* length of rx_list */ > > > > unsigned int napi_id; > > > > int defer_hard_irqs; > > > > + unsigned long gro_flush_timeout; > > > > struct hrtimer timer; > > > > struct task_struct *thread; > > > > /* control-path-only fields follow */ > > > > @@ -557,6 +558,31 @@ void napi_set_defer_hard_irqs(struct napi_struct *n, int defer); > > > > */ > > > > void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer); > > > > > > > > > > Same remark : dev->gro_flush_timeout is no longer read in the fast path. > > > > > > Please move gro_flush_timeout out of net_device_read_txrx and update > > > Documentation/networking/net_cachelines/net_device.rst > > > > Is there some tooling I should use to generate this file? > > > > I am asking because it seems like the file is missing two fields in > > net_device at the end of the struct: > > > > struct hlist_head page_pools; > > struct dim_irq_moder * irq_moder; > > > > At first glance this is control path only, no big deal. My plan was to move both fields you pointed out in my series to the end of the struct (there is a big enough hole for both) and move them to the bottom of this doc file (with just Type and Name, of course). The two fields (page_pools, irq_moder) being missing made me unsure if I was planning on doing the right thing for the v2. > > Both of which seem to have been added just before and long after > > (respectively) commit 14006f1d8fa2 ("Documentations: Analyze heavily > > used Networking related structs"). > > > > If this is a bug, I can submit one patch (with two fixes tags) which > > adds both fields to the file? > > No need for a Fixes: tag for this, just submit to net-next. > > This file is really 'needed' for current development, for people > caring about data locality. Will do; thanks for the guidance.
Hi Joe,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/net-napi-Make-napi_defer_hard_irqs-per-NAPI/20240829-211617
base: net-next/main
patch link: https://lore.kernel.org/r/20240829131214.169977-4-jdamato%40fastly.com
patch subject: [PATCH net-next 3/5] net: napi: Make gro_flush_timeout per-NAPI
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20240831/202408310026.wxZySizP-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240831/202408310026.wxZySizP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408310026.wxZySizP-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/minmax.h:5,
from include/linux/jiffies.h:8,
from include/net/pkt_sched.h:5,
from drivers/net/ethernet/intel/idpf/idpf.h:12,
from drivers/net/ethernet/intel/idpf/idpf_dev.c:4:
include/linux/build_bug.h:78:41: error: static assertion failed: "offsetof(struct idpf_q_vector, __cacheline_group_end__read_write) - offsetofend(struct idpf_q_vector, __cacheline_group_begin__read_write) <= (424 + 2 * sizeof(struct dim))"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
include/net/libeth/cache.h:24:9: note: in expansion of macro 'static_assert'
24 | static_assert(offsetof(type, __cacheline_group_end__##grp) - \
| ^~~~~~~~~~~~~
include/net/libeth/cache.h:62:9: note: in expansion of macro 'libeth_cacheline_group_assert'
62 | libeth_cacheline_group_assert(type, read_write, rw); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/idpf/idpf_txrx.h:475:1: note: in expansion of macro 'libeth_cacheline_set_assert'
475 | libeth_cacheline_set_assert(struct idpf_q_vector, 104,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct idpf_q_vector) <= ((((((104)) + ((__typeof__((104)))((256)) - 1)) & ~((__typeof__((104)))((256)) - 1)) + ((((424 + 2 * sizeof(struct dim))) + ((__typeof__((424 + 2 * sizeof(struct dim))))((256)) - 1)) & ~((__typeof__((424 + 2 * sizeof(struct dim))))((256)) - 1)) + ((((8 + sizeof(cpumask_var_t))) + ((__typeof__((8 + sizeof(cpumask_var_t))))((256)) - 1)) & ~((__typeof__((8 + sizeof(cpumask_var_t))))((256)) - 1))))"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
include/net/libeth/cache.h:28:9: note: in expansion of macro 'static_assert'
28 | static_assert(sizeof(type) <= (sz))
| ^~~~~~~~~~~~~
include/net/libeth/cache.h:48:9: note: in expansion of macro '__libeth_cacheline_struct_assert'
48 | __libeth_cacheline_struct_assert(type, __libeth_cls(__VA_ARGS__)); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/libeth/cache.h:64:9: note: in expansion of macro 'libeth_cacheline_struct_assert'
64 | libeth_cacheline_struct_assert(type, ro, rw, c)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/idpf/idpf_txrx.h:475:1: note: in expansion of macro 'libeth_cacheline_set_assert'
475 | libeth_cacheline_set_assert(struct idpf_q_vector, 104,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +78 include/linux/build_bug.h
bc6245e5efd70c Ian Abbott 2017-07-10 60
6bab69c65013be Rasmus Villemoes 2019-03-07 61 /**
6bab69c65013be Rasmus Villemoes 2019-03-07 62 * static_assert - check integer constant expression at build time
6bab69c65013be Rasmus Villemoes 2019-03-07 63 *
6bab69c65013be Rasmus Villemoes 2019-03-07 64 * static_assert() is a wrapper for the C11 _Static_assert, with a
6bab69c65013be Rasmus Villemoes 2019-03-07 65 * little macro magic to make the message optional (defaulting to the
6bab69c65013be Rasmus Villemoes 2019-03-07 66 * stringification of the tested expression).
6bab69c65013be Rasmus Villemoes 2019-03-07 67 *
6bab69c65013be Rasmus Villemoes 2019-03-07 68 * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
6bab69c65013be Rasmus Villemoes 2019-03-07 69 * scope, but requires the expression to be an integer constant
6bab69c65013be Rasmus Villemoes 2019-03-07 70 * expression (i.e., it is not enough that __builtin_constant_p() is
6bab69c65013be Rasmus Villemoes 2019-03-07 71 * true for expr).
6bab69c65013be Rasmus Villemoes 2019-03-07 72 *
6bab69c65013be Rasmus Villemoes 2019-03-07 73 * Also note that BUILD_BUG_ON() fails the build if the condition is
6bab69c65013be Rasmus Villemoes 2019-03-07 74 * true, while static_assert() fails the build if the expression is
6bab69c65013be Rasmus Villemoes 2019-03-07 75 * false.
6bab69c65013be Rasmus Villemoes 2019-03-07 76 */
6bab69c65013be Rasmus Villemoes 2019-03-07 77 #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
6bab69c65013be Rasmus Villemoes 2019-03-07 @78 #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
6bab69c65013be Rasmus Villemoes 2019-03-07 79
07a368b3f55a79 Maxim Levitsky 2022-10-25 80
Hi Joe,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/net-napi-Make-napi_defer_hard_irqs-per-NAPI/20240829-211617
base: net-next/main
patch link: https://lore.kernel.org/r/20240829131214.169977-4-jdamato%40fastly.com
patch subject: [PATCH net-next 3/5] net: napi: Make gro_flush_timeout per-NAPI
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20240831/202408310043.fmwHg8BS-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240831/202408310043.fmwHg8BS-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408310043.fmwHg8BS-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/minmax.h:5,
from include/linux/jiffies.h:8,
from include/net/pkt_sched.h:5,
from drivers/net/ethernet/intel/idpf/idpf.h:12,
from drivers/net/ethernet/intel/idpf/idpf_dev.c:4:
include/linux/build_bug.h:78:41: error: static assertion failed: "offsetof(struct idpf_q_vector, __cacheline_group_end__read_write) - offsetofend(struct idpf_q_vector, __cacheline_group_begin__read_write) == (424 + 2 * sizeof(struct dim))"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
include/net/libeth/cache.h:17:9: note: in expansion of macro 'static_assert'
17 | static_assert(offsetof(type, __cacheline_group_end__##grp) - \
| ^~~~~~~~~~~~~
include/net/libeth/cache.h:62:9: note: in expansion of macro 'libeth_cacheline_group_assert'
62 | libeth_cacheline_group_assert(type, read_write, rw); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/idpf/idpf_txrx.h:475:1: note: in expansion of macro 'libeth_cacheline_set_assert'
475 | libeth_cacheline_set_assert(struct idpf_q_vector, 104,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct idpf_q_vector) == ((((((104)) + ((__typeof__((104)))(((1 << 6))) - 1)) & ~((__typeof__((104)))(((1 << 6))) - 1)) + ((((424 + 2 * sizeof(struct dim))) + ((__typeof__((424 + 2 * sizeof(struct dim))))(((1 << 6))) - 1)) & ~((__typeof__((424 + 2 * sizeof(struct dim))))(((1 << 6))) - 1)) + ((((8 + sizeof(cpumask_var_t))) + ((__typeof__((8 + sizeof(cpumask_var_t))))(((1 << 6))) - 1)) & ~((__typeof__((8 + sizeof(cpumask_var_t))))(((1 << 6))) - 1))))"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
include/net/libeth/cache.h:21:9: note: in expansion of macro 'static_assert'
21 | static_assert(sizeof(type) == (sz))
| ^~~~~~~~~~~~~
include/net/libeth/cache.h:48:9: note: in expansion of macro '__libeth_cacheline_struct_assert'
48 | __libeth_cacheline_struct_assert(type, __libeth_cls(__VA_ARGS__)); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/libeth/cache.h:64:9: note: in expansion of macro 'libeth_cacheline_struct_assert'
64 | libeth_cacheline_struct_assert(type, ro, rw, c)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/idpf/idpf_txrx.h:475:1: note: in expansion of macro 'libeth_cacheline_set_assert'
475 | libeth_cacheline_set_assert(struct idpf_q_vector, 104,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +78 include/linux/build_bug.h
bc6245e5efd70c Ian Abbott 2017-07-10 60
6bab69c65013be Rasmus Villemoes 2019-03-07 61 /**
6bab69c65013be Rasmus Villemoes 2019-03-07 62 * static_assert - check integer constant expression at build time
6bab69c65013be Rasmus Villemoes 2019-03-07 63 *
6bab69c65013be Rasmus Villemoes 2019-03-07 64 * static_assert() is a wrapper for the C11 _Static_assert, with a
6bab69c65013be Rasmus Villemoes 2019-03-07 65 * little macro magic to make the message optional (defaulting to the
6bab69c65013be Rasmus Villemoes 2019-03-07 66 * stringification of the tested expression).
6bab69c65013be Rasmus Villemoes 2019-03-07 67 *
6bab69c65013be Rasmus Villemoes 2019-03-07 68 * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
6bab69c65013be Rasmus Villemoes 2019-03-07 69 * scope, but requires the expression to be an integer constant
6bab69c65013be Rasmus Villemoes 2019-03-07 70 * expression (i.e., it is not enough that __builtin_constant_p() is
6bab69c65013be Rasmus Villemoes 2019-03-07 71 * true for expr).
6bab69c65013be Rasmus Villemoes 2019-03-07 72 *
6bab69c65013be Rasmus Villemoes 2019-03-07 73 * Also note that BUILD_BUG_ON() fails the build if the condition is
6bab69c65013be Rasmus Villemoes 2019-03-07 74 * true, while static_assert() fails the build if the expression is
6bab69c65013be Rasmus Villemoes 2019-03-07 75 * false.
6bab69c65013be Rasmus Villemoes 2019-03-07 76 */
6bab69c65013be Rasmus Villemoes 2019-03-07 77 #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
6bab69c65013be Rasmus Villemoes 2019-03-07 @78 #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
6bab69c65013be Rasmus Villemoes 2019-03-07 79
07a368b3f55a79 Maxim Levitsky 2022-10-25 80
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 7d53380da4c0..d00024d9f857 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -372,6 +372,7 @@ struct napi_struct { int rx_count; /* length of rx_list */ unsigned int napi_id; int defer_hard_irqs; + unsigned long gro_flush_timeout; struct hrtimer timer; struct task_struct *thread; /* control-path-only fields follow */ @@ -557,6 +558,31 @@ void napi_set_defer_hard_irqs(struct napi_struct *n, int defer); */ void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer); +/** + * napi_get_gro_flush_timeout - get the gro_flush_timeout + * @n: napi struct to get the gro_flush_timeout from + * + * Returns the per-NAPI value of the gro_flush_timeout field. + */ +unsigned long napi_get_gro_flush_timeout(const struct napi_struct *n); + +/** + * napi_set_gro_flush_timeout - set the gro_flush_timeout for a napi + * @n: napi struct to set the gro_flush_timeout + * @timeout: timeout value to set + * + * napi_set_gro_flush_timeout sets the per-NAPI gro_flush_timeout + */ +void napi_set_gro_flush_timeout(struct napi_struct *n, unsigned long timeout); + +/** + * netdev_set_gro_flush_timeout - set gro_flush_timeout for all NAPIs of a netdev + * @netdev: the net_device for which all NAPIs will have their gro_flush_timeout set + * @timeout: the timeout value to set + */ +void netdev_set_gro_flush_timeout(struct net_device *netdev, + unsigned long timeout); + /** * napi_complete_done - NAPI processing complete * @n: NAPI context diff --git a/net/core/dev.c b/net/core/dev.c index f7baff0da057..3f7cb1085efa 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6234,6 +6234,29 @@ void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer) } EXPORT_SYMBOL_GPL(netdev_set_defer_hard_irqs); +unsigned long napi_get_gro_flush_timeout(const struct napi_struct *n) +{ + return READ_ONCE(n->gro_flush_timeout); +} +EXPORT_SYMBOL_GPL(napi_get_gro_flush_timeout); + +void napi_set_gro_flush_timeout(struct napi_struct *n, unsigned long timeout) +{ + WRITE_ONCE(n->gro_flush_timeout, timeout); +} +EXPORT_SYMBOL_GPL(napi_set_gro_flush_timeout); + +void netdev_set_gro_flush_timeout(struct net_device *netdev, + unsigned long timeout) +{ + struct napi_struct *napi; + + WRITE_ONCE(netdev->gro_flush_timeout, timeout); + list_for_each_entry(napi, &netdev->napi_list, dev_list) + napi_set_gro_flush_timeout(napi, timeout); +} +EXPORT_SYMBOL_GPL(netdev_set_gro_flush_timeout); + bool napi_complete_done(struct napi_struct *n, int work_done) { unsigned long flags, val, new, timeout = 0; @@ -6251,12 +6274,12 @@ 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); + timeout = napi_get_gro_flush_timeout(n); n->defer_hard_irqs_count = napi_get_defer_hard_irqs(n); } if (n->defer_hard_irqs_count > 0) { n->defer_hard_irqs_count--; - timeout = READ_ONCE(n->dev->gro_flush_timeout); + timeout = napi_get_gro_flush_timeout(n); if (timeout) ret = false; } @@ -6391,7 +6414,7 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock, if (flags & NAPI_F_PREFER_BUSY_POLL) { napi->defer_hard_irqs_count = napi_get_defer_hard_irqs(napi); - timeout = READ_ONCE(napi->dev->gro_flush_timeout); + timeout = napi_get_gro_flush_timeout(napi); if (napi->defer_hard_irqs_count && timeout) { hrtimer_start(&napi->timer, ns_to_ktime(timeout), HRTIMER_MODE_REL_PINNED); skip_schedule = true; @@ -6673,6 +6696,7 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi, 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)); + napi_set_gro_flush_timeout(napi, READ_ONCE(dev->gro_flush_timeout)); init_gro_hash(napi); napi->skb = NULL; INIT_LIST_HEAD(&napi->rx_list); @@ -11054,7 +11078,7 @@ void netdev_sw_irq_coalesce_default_on(struct net_device *dev) WARN_ON(dev->reg_state == NETREG_REGISTERED); if (!IS_ENABLED(CONFIG_PREEMPT_RT)) { - dev->gro_flush_timeout = 20000; + netdev_set_gro_flush_timeout(dev, 20000); netdev_set_defer_hard_irqs(dev, 1); } } diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 8272f0144d81..ff545a422b1f 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -408,7 +408,7 @@ NETDEVICE_SHOW_RW(tx_queue_len, fmt_dec); static int change_gro_flush_timeout(struct net_device *dev, unsigned long val) { - WRITE_ONCE(dev->gro_flush_timeout, val); + netdev_set_gro_flush_timeout(dev, val); return 0; }