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
|