Messages in this thread | | | Date | Sat, 18 Mar 2000 00:34:41 +1100 | From | Martijn van Oosterhout <> | Subject | Re: New information on the CIPE problem (compiler conflicts) |
| |
Olaf Titz wrote: > > Good spot. I've always suspected that the hh_cache was involved in > some way. This struct is (in 2.2.15pre7 <linux/netdevice.h>) as > follows, and it's even worse than seen from your first look: > > struct hh_cache > { > struct hh_cache *hh_next; /* Next entry */ > atomic_t hh_refcnt; /* number of users */ > unsigned short hh_type; /* protocol identifier, f.e ETH_P_IP */ > /*** Possible alignment problem here ***/ > int (*hh_output)(struct sk_buff *skb); > rwlock_t hh_lock; > /*** And another possible alignment problem here ***/ > /* cached hardware header; allow for machine alignment needs. */ > unsigned long hh_data[16/sizeof(unsigned long)]; > }; > > As I've already mentioned I've got a _four_ byte shift. The reason is > probably the rwlock_t definition in <asm-i386/spinlock.h> for the UP case: > > #if (__GNUC__ > 2) || (__GNUC__ == 2 && __GNUC_MINOR__ >= 8) > typedef struct { } rwlock_t; > #define RW_LOCK_UNLOCKED (rwlock_t) { } > #else > typedef struct { int gcc_is_buggy; } rwlock_t; > #define RW_LOCK_UNLOCKED (rwlock_t) { 0 } > #endif > > This is not only an alignment problem but an explicit compiler > dependency as well :-(
Yikes! This means that anything compiled with 2.95 will get a different structure size from a 2.7.2.3 compile if it included rwlock.
> Alan (or whoever is responsible :-), is there any chance to get this > fixed in some way?
That explicit compiler version dependancy has to go. I guess the reason for it is to make sure they take up no space on a UP compile.
> > Suggestion: use attribute((packed)), make it an int > > and move that member elsewhere. > > Packing doesn't look right to me, as misaligning the array and the > lock (for SMP) is performance-unfriendly. Reordering could help, like > this:
If its the the lock causing the problem then the below won't fix it because the ip_send also writes the hh_type field. I really think that the hh_type should be made unsigned int and the rwlock moved to the end.
> struct hh_cache > { > struct hh_cache *hh_next; /* Next entry */ > atomic_t hh_refcnt; /* number of users */ > int (*hh_output)(struct sk_buff *skb); > /* cached hardware header; allow for machine alignment needs. */ > unsigned long hh_data[16/sizeof(unsigned long)]; > rwlock_t hh_lock; > unsigned short hh_type; /* protocol identifier, f.e ETH_P_IP */ > };
Have there been any other responses to this? -- Martijn van Oosterhout <kleptog@cupid.suninternet.com>
Trust the computer industry to shorten "Year 2000" to Y2K. It was this kind of thinking that caused the problem in the first place.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |