Re: ticket spinlocks and sizeof(struct dentry)

[Nick Piggin <npiggin@suse.de>]

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