On Fri, Oct 09, 2009 at 09:02:31AM -0700, Luck, Tony wrote: > > Just to let you know you may have turned sizeof(struct dentry) into > > something nasty with 64 bit spinlocks (like 200 bytes) which would > > mean a significant number (~half?) of dentries will cover 3 cachelines > > instead of 2. > > Thanks for the heads-up ... Linus also pointed out this downside of > the current implementation. IA64 uses 128 byte cache lines ... so your > calculation of how many we use for a dentry is off (we move from > exactly 1.5 cache lines to 1.5625 cache lines). But this sucks too > because now the cache-line boundaries are scattered in different > places in different dentry instances, Well this is why a given random dentry will now be nearly 50% likely to cover 3 cachelines. So a statistically, your cacheline footprint for dentry structures (assuming no sharing between adjacent dentries) has increased from 2 lines per dentry to nearly 2.5. > so all your careful work to > put stuff that is used together into the same cache line is randomly > broken on some dentry instances. Well yes but I would say the way we pad the dentry is not very flexible and prone to breakage like this. If we did something like: struct dentry { union { struct { /* real dentry fields here */ unsigned char d_iname[]; }; char ___pad[192]; }; }; #define D_INLINE_NAME_LEN (192 - offsetof(d_iname)) Then it would be more flexible. OTOH maybe it is a bad thing if it is resized without anybody noticing. We could instead leave it as it is and have a cpp compile time warning if the size if not 192. > > If I'm nost mistaken, this would probably be worth correcting. > > I'm working on it. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |