On Tue, Jul 9, 2024 at 5:54 AM Breno Leitao <[email protected]> wrote: > > From: Alexander Lobakin <[email protected]> > > In fact, this structure contains a flexible array at the end, but > historically its size, alignment etc., is calculated manually. > There are several instances of the structure embedded into other > structures, but also there's ongoing effort to remove them and we > could in the meantime declare &net_device properly. > Declare the array explicitly, use struct_size() and store the array > size inside the structure, so that __counted_by() can be applied. > Don't use PTR_ALIGN(), as SLUB itself tries its best to ensure the > allocated buffer is aligned to what the user expects. > Also, change its alignment from %NETDEV_ALIGN to the cacheline size > as per several suggestions on the netdev ML. > > bloat-o-meter for vmlinux: > > free_netdev 445 440 -5 > netdev_freemem 24 - -24 > alloc_netdev_mqs 1481 1450 -31 > > On x86_64 with several NICs of different vendors, I was never able to > get a &net_device pointer not aligned to the cacheline size after the > change. > > Signed-off-by: Alexander Lobakin <[email protected]> > Signed-off-by: Breno Leitao <[email protected]> > Reviewed-by: Przemek Kitszel <[email protected]> > --- > Changelog: > > v2: > * Rebased Alexander's patch on top of f750dfe825b90 ("ethtool: provide > customized dim profile management"). > * Removed the ALIGN() of SMP_CACHE_BYTES for sizeof_priv. > > v1: > * > https://lore.kernel.org/netdev/[email protected] > > include/linux/netdevice.h | 12 +++++++----- > net/core/dev.c | 30 ++++++------------------------ > net/core/net-sysfs.c | 2 +- > 3 files changed, 14 insertions(+), 30 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 93558645c6d0..f0dd499244d4 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -2199,10 +2199,10 @@ struct net_device { > unsigned short neigh_priv_len; > unsigned short dev_id; > unsigned short dev_port; > - unsigned short padded; > + int irq; > + u32 priv_len; > > spinlock_t addr_list_lock; > - int irq; > > struct netdev_hw_addr_list uc; > struct netdev_hw_addr_list mc; > @@ -2406,7 +2406,10 @@ struct net_device { > > /** @irq_moder: dim parameters used if IS_ENABLED(CONFIG_DIMLIB). */ > struct dim_irq_moder *irq_moder; > -}; > + > + u8 priv[] ____cacheline_aligned > + __counted_by(priv_len); > +} ____cacheline_aligned; > #define to_net_dev(d) container_of(d, struct net_device, dev) > > /* > @@ -2596,7 +2599,7 @@ void dev_net_set(struct net_device *dev, struct net > *net) > */ > static inline void *netdev_priv(const struct net_device *dev) > { > - return (char *)dev + ALIGN(sizeof(struct net_device), NETDEV_ALIGN); > + return (void *)dev->priv;
Minor remark : the cast is not needed, but this is fine. Reviewed-by: Eric Dumazet <[email protected]> It would be great to get rid of NETDEV_ALIGN eventually. Thanks.
