On Wed, 12 Feb 2025, Al Viro wrote:
> On Wed, Feb 12, 2025 at 10:35:41AM +1100, NeilBrown wrote:
>
> > Without lockdep making the dentry extra large, struct dentry is 192
> > bytes, exactly 3 cache lines. There are 16 entries per 4K slab.
> > Now exactly 1/4 of possible indices are used.
> > For every group of 16 possible indices, only 0, 4, 8, 12 are used.
> > slabinfo says the object size is 256 which explains some of the spread.
>
> Interesting...
>
> root@cannonball:~# grep -w dentry /proc/slabinfo
> dentry 1370665 1410864 192 21 1 : tunables 0 0 0 : slabdata 67184 67184 0
>
> Where does that 256 come from? The above is on amd64, with 6.1-based debian
> kernel and I see the same object size on other boxen (with local configs).
I found SLUB_DEBUG and redzoning does that. Disabling the debug brings
done to 192 bytes and 21 per slab which you see. That is still only 33%
hit rate.
>
> > I don't think there is a good case here for selecting bits from the
> > middle of the dentry address.
> >
> > If I use hash_ptr(dentry, 8) I get a more uniform distribution. 64000
> > entries would hope for 250 per bucket. Median is 248. Range is 186 to
> > 324 so +/- 25%.
> >
> > Maybe that is the better choice.
>
> That's really interesting, considering the implications for m_hash() and mp_hash()
> (see fs/namespace.c)...
Those functions add in the next set of bits as well - effectively mixing
in more bits from the page address. If I do that the spread is better
but there are still buckets with close to twice the median, though most
are +/- 30%.
>
> > > > > 3) the dance with conditional __wake_up() is worth a helper, IMO.
> > >
> > > I mean an inlined helper function.
> >
> > Yes.. Of course...
> >
> > Maybe we should put
> >
> > static inline void wake_up_key(struct wait_queue_head *wq, void *key)
> > {
> > __wake_up(wq, TASK_NORMAL, 0, key);
> > }
> >
> > in include/linux/wait.h to avoid the __wake_up() "internal" name, and
> > then use
> > wake_up_key(d_wait, dentry);
> > in the two places in dcache.c, or did you want something
> > dcache-specific?
>
> More like
> if (wq)
> __wake_up(wq, TASK_NORMAL, 0, key);
> probably...
>
Thanks,
NeilBrown
