while digging through the alloc_netdev function I asked myself why there is a
fixed alignment for netdevices. Is there a special reason for choosing 32? If
yes, I suggest to add a comment to the definition. 

If not, I suspect cacheline alignment might be beneficial:
struct net_device contains several variables which are cache aligned 
(poll_list, queue_lock .....). As far as I can see, the compiler tries to 
increase the size of the structure to make that possible, but expects the
whole structure to be aligned to cache line size as well. With the current
value for NETDEV_ALIGN we dont align "struct net_device" to the cache line,
instead we align it to 32 bytes. That means that poll_list, queue_lock and 
friends are not always cache aligned, but 32 bytes aligned.

The only reason why everything worked so far is the slab allocator design, 
which gives us a page aligned struct net_device in most cases. I think we 
should not rely on the behaviour of the memory allocator and use a different 
value for NETDEV_ALIGN instead. Any comments or corrections?

cheers Christian



The patch below is compile and boot tested on s390 and x86.

Signed-off-by: Christian Borntraeger <[EMAIL PROTECTED]>

include/linux/netdevice.h |    2 +-
net/core/dev.c            |    2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
----------

--- a/include/linux/netdevice.h 4 Apr 2006 07:25:47 -0000
+++ b/include/linux/netdevice.h 15 May 2006 11:06:05 -0000
@@ -504,7 +504,7 @@
        struct class_device     class_dev;
 };
 
-#define        NETDEV_ALIGN            32
+#define        NETDEV_ALIGN            L1_CACHE_BYTES
 #define        NETDEV_ALIGN_CONST      (NETDEV_ALIGN - 1)
 
 static inline void *netdev_priv(struct net_device *dev)
--- a/net/core/dev.c    4 Apr 2006 07:25:50 -0000
+++ b/net/core/dev.c    15 May 2006 11:06:05 -0000
@@ -2986,7 +2986,7 @@
        struct net_device *dev;
        int alloc_size;
 
-       /* ensure 32-byte alignment of both the device and private area */
+       /* ensure cacheline alignment of both the device and private area */
        alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST) & ~NETDEV_ALIGN_CONST;
        alloc_size += sizeof_priv + NETDEV_ALIGN_CONST;
 
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to