Messages in this thread | | | From | Eric Dumazet <> | Date | Wed, 30 Jun 2021 13:02:53 +0200 | Subject | Re: [PATCH] net: core: Modify alloc_size in alloc_netdev_mqs() |
| |
On Wed, Jun 30, 2021 at 7:11 AM Yajun Deng <yajun.deng@linux.dev> wrote: > > Use ALIGN for 'struct net_device', and remove the unneeded > 'NETDEV_ALIGN - 1'. This can save a few bytes. and modify > the pr_err content when txqs < 1.
I think that in old times (maybe still today), SLAB debugging could lead to not unaligned allocated zones. The forced alignment for netdev structures came in commit f346af6a27c0cea99522213cb813fd30489136e2 ("net_device and netdev private struct allocation improvements.") in linux-2.6.3 (back in 2004)
This supposedly was a win in itself, otherwise Al Viro would not have spent time on this.
> > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > --- > net/core/dev.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index c253c2aafe97..c42a682a624d 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -10789,7 +10789,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, > BUG_ON(strlen(name) >= sizeof(dev->name)); > > if (txqs < 1) { > - pr_err("alloc_netdev: Unable to allocate device with zero queues\n"); > + pr_err("alloc_netdev: Unable to allocate device with zero TX queues\n"); > return NULL; > } > > @@ -10798,14 +10798,12 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, > return NULL; > } > > - alloc_size = sizeof(struct net_device); > + /* ensure 32-byte alignment of struct net_device*/ > + alloc_size = ALIGN(sizeof(struct net_device), NETDEV_ALIGN);
This is not really needed, because struct net_device is cache line aligned already on SMP builds.
> if (sizeof_priv) { > /* ensure 32-byte alignment of private area */ > - alloc_size = ALIGN(alloc_size, NETDEV_ALIGN); > - alloc_size += sizeof_priv; > + alloc_size += ALIGN(sizeof_priv, NETDEV_ALIGN);
No longer needed, the private area starts at the end of struct net_device, whose size is a multiple of cache line.
Really I doubt this makes sense anymore these days, we have hundreds of structures in the kernel that would need a similar handling if SLAB/SLUB was doing silly things.
I would simply do :
alloc_size += sizeof_priv;
> } > - /* ensure 32-byte alignment of whole construct */ > - alloc_size += NETDEV_ALIGN - 1; > > p = kvzalloc(alloc_size, GFP_KERNEL | __GFP_RETRY_MAYFAIL); > if (!p) > -- > 2.32.0 >
| |