Re: [RFC] changing value of NETDEV_ALIGN to cacheline size

[Stephen Hemminger <shemminger@osdl.org>]

On Mon, 15 May 2006 14:39:23 -0700
Rick Jones <rick.jones2@hp.com> wrote:

> David S. Miller wrote:
> > From: "Randy.Dunlap" <rdunlap@xenotime.net>
> > Date: Mon, 15 May 2006 08:02:58 -0700
> > 
> > 
> >>>-#define	NETDEV_ALIGN		32
> >>>+#define	NETDEV_ALIGN		L1_CACHE_BYTES
> >>> #define	NETDEV_ALIGN_CONST	(NETDEV_ALIGN - 1)
> >>
> >>I don't know about the fixed value of 32, but if this patch is
> >>accepted, I'd prefer NETDEV_ALIGN_MASK instead of NETDEV_ALIGN_CONST.
> > 
> > 
> > The reason it's 32 is that old drivers depended on the
> > struct being at least 32-byte aligned because they would
> > embed structures DMA'd to/from the card in their private
> > area and just assumed that would be aligned enough for
> > the card's restrictions.
> > 
> > So setting it to L1_CACHE_BYTES would be wrong, because if
> > that happens to be less than 32 it would violate said
> > assumption which we are catering to.
> 
> How about:
> 
> #define NETDEV_ALIGN_MIN 32
> #if L1_CACHE_BYTES > NETDEV_ALIGN_MIN
> # define NETDEV_ALIGN L1_CACHE_BYTES
> #else
> # define NETDEV_ALIGN NETDEV_ALIGN_MIN
> #endif
> 

I can't see how adding more padding could help performance here.
Most drivers reference both parts (netdevice and netdev_priv) in
most code paths. So having the stuff on separate cache lines couldn't
help that much. 

Alternatively, there might be a performance gain if the netdevice stats
structure was rearranged so the rx and tx stats were on different cache lines.