[RFC][PATCH] net drivers and cache alignment

One of the [many] nice properties of the traditional Don Becker drivers 
has been that often the driver-private structures are arranged such that 
  the structure is broken up on cacheline boundaries.  The RX thread has 
a cacheline, the TX thread has a cacheline, etc.  Jes Sorensen also 
independently, in his review of 8139cp.c, suggested that the 
driver-private struct be update with attention to cacheline boundaries.

Early next year, I would like to start cleaning up some of the net 
drivers along these lines (no pun intended).  To make it easier for 
vendors and random coders to cacheline-align struct members, I would 
like to make more explicit these cacheline boundaries, in a manner that 
is portable between 32-bit and 64-bit systems.

I attach a sample implementation, and request feedback on this approach. 
   The general idea is to make implementing this sort of concept "harder 
to screw up."

Can't the cacheline_aligned attribute be applied to individual
struct members?  I remember doing this for thread_struct on
sparc ages ago.

David S. Miller wrote:

Can't the cacheline_aligned attribute be applied to individual
struct members?  I remember doing this for thread_struct on
sparc ages ago.

I was hoping someone who knows gcc better than me knew that, and would 
speak up ;-)

David S. Miller wrote:

Can't the cacheline_aligned attribute be applied to individual
struct members?  I remember doing this for thread_struct on
sparc ages ago.

Looks like it from the 2.4 processor.h code.

Attached is cut #2.  Thanks for all the near-instant feedback so far :) 
  Andrew, does the attached still need padding on SMP?

Jeff Garzik wrote:

David S. Miller wrote:

quoted

Can't the cacheline_aligned attribute be applied to individual
struct members?  I remember doing this for thread_struct on
sparc ages ago.

Looks like it from the 2.4 processor.h code.

Attached is cut #2.  Thanks for all the near-instant feedback so far :)
  Andrew, does the attached still need padding on SMP?

It needs padding _only_ on SMP.  ____cacheline_aligned_in_smp.

#define offsetof(t, m)  ((int)(&((t *)0)->m))

struct foo {
        int a;
        int b __attribute__((__aligned__(1024)));
        int c;
} foo;

main()
{
        printf("%d\n", sizeof(struct foo));
        printf("%d\n", offsetof(struct foo, a));
        printf("%d\n", offsetof(struct foo, b));
        printf("%d\n", offsetof(struct foo, c));
}

./a.out
2048
0
1024
1028

So your patch will do what you want it to do.  You should just tag the
first member of a group with ____cacheline_aligned_in_smp, and keep an
eye on things with offsetof().

Not sure why sizeof() returned 2048 though.

   From: Andrew Morton [off-list ref]
   Date: Sat, 07 Dec 2002 15:29:16 -0800

   Jeff Garzik wrote:
   > Attached is cut #2.  Thanks for all the near-instant feedback so far :)
   >   Andrew, does the attached still need padding on SMP?
   
   It needs padding _only_ on SMP.  ____cacheline_aligned_in_smp.
   
non-smp machines lack L2 caches?  That's new to me :-)

More seriously, there are real benefits on non-SMP systems.

Andrew Morton wrote:

It needs padding _only_ on SMP.  ____cacheline_aligned_in_smp.

[...]

So your patch will do what you want it to do.  You should just tag the
first member of a group with ____cacheline_aligned_in_smp, and keep an
eye on things with offsetof().

thanks.

For this case, though, I want to align on cacheline bounaries even on 
UP, right?  That's why I picked ____cacheline_aligned.  It uses 
L1_CACHE_BYTES when !CONFIG_SMP.  Other uses of ____cacheline_aligned in 
the kernel seem to relate to irq matters, just like my groupings in tg3.h.

[obviously benchmarking can answer some of this, but I want to hammer 
out silliness first]

	Jeff

"David S. Miller" wrote:

   From: Andrew Morton [off-list ref]
   Date: Sat, 07 Dec 2002 15:29:16 -0800

   Jeff Garzik wrote:
   > Attached is cut #2.  Thanks for all the near-instant feedback so far :)
   >   Andrew, does the attached still need padding on SMP?

   It needs padding _only_ on SMP.  ____cacheline_aligned_in_smp.

non-smp machines lack L2 caches?  That's new to me :-)

More seriously, there are real benefits on non-SMP systems.

Then I am most confused.  None of these fields will be put under
busmastering or anything like that, so what advantage is there in
spreading them out?

Andrew Morton wrote:

"David S. Miller" wrote:

quoted

   From: Andrew Morton [off-list ref]
   Date: Sat, 07 Dec 2002 15:29:16 -0800

   Jeff Garzik wrote:
   > Attached is cut #2.  Thanks for all the near-instant feedback so far :)
   >   Andrew, does the attached still need padding on SMP?

   It needs padding _only_ on SMP.  ____cacheline_aligned_in_smp.

non-smp machines lack L2 caches?  That's new to me :-)

More seriously, there are real benefits on non-SMP systems.

Then I am most confused.  None of these fields will be put under
busmastering or anything like that, so what advantage is there in
spreading them out?

Oh I see what you want - to be able to pick up all the operating fields
in a single fetch.

That will increase the overall cache footprint though.  I wonder if
it's really a net win, over just keeping it small.

On Sat, Dec 07, 2002 at 03:29:16PM -0800, Andrew Morton wrote:

Jeff Garzik wrote:

quoted

David S. Miller wrote:

quoted

Can't the cacheline_aligned attribute be applied to individual
struct members?  I remember doing this for thread_struct on
sparc ages ago.

Looks like it from the 2.4 processor.h code.

Attached is cut #2.  Thanks for all the near-instant feedback so far :)
  Andrew, does the attached still need padding on SMP?

It needs padding _only_ on SMP.  ____cacheline_aligned_in_smp.

#define offsetof(t, m)  ((int)(&((t *)0)->m))

struct foo {
        int a;
        int b __attribute__((__aligned__(1024)));
        int c;
} foo;

main()
{
        printf("%d\n", sizeof(struct foo));
        printf("%d\n", offsetof(struct foo, a));
        printf("%d\n", offsetof(struct foo, b));
        printf("%d\n", offsetof(struct foo, c));
}

./a.out
2048
0
1024
1028

So your patch will do what you want it to do.  You should just tag the
first member of a group with ____cacheline_aligned_in_smp, and keep an
eye on things with offsetof().

Not sure why sizeof() returned 2048 though.

The structure contains an __aligned__(1024) item.  Think about an array
of 'struct foo' items.  They have to be 2048 bytes or you won't align
correctly.

C allows for empty space in structure padding, but not in arrays,
AFAIK.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

   From: Andrew Morton [off-list ref]
   Date: Sat, 07 Dec 2002 15:42:00 -0800

   "David S. Miller" wrote:
   > non-smp machines lack L2 caches?  That's new to me :-)
   > 
   > More seriously, there are real benefits on non-SMP systems.
   
   Then I am most confused.  None of these fields will be put under
   busmastering or anything like that, so what advantage is there in
   spreading them out?
   
When you are in the "tx path" you'll take one L2 cache miss
to bring all the necessary information into the cpu's caches.

Otherwise, when data is arbitrarily scattered over multiple L2
cache lines, you'll need to service potentially more L2 cache
misses.

This optimization has nothing to do with false data sharing amoungst
multiple processors.  It's about packing the data accesses optimally
for specific code paths.