ticket spinlocks and sizeof(struct dentry)

ticket spinlocks and sizeof(struct dentry)

[Nick Piggin]

Hi guys,

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.

If I'm nost mistaken, this would probably be worth correcting.

Thanks,
Nick

RE: ticket spinlocks and sizeof(struct dentry)

[Luck, Tony]

> 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, so all your careful work to
put stuff that is used together into the same cache line is randomly
broken on some dentry instances.

> If I'm nost mistaken, this would probably be worth correcting.

I'm working on it.

-Tony

Re: ticket spinlocks and sizeof(struct dentry)

[Nick Piggin]

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