Re: [PATCH 02/19] VFS: use global wait-queue table for d_alloc_parallel()

[Al Viro <viro@zeniv.linux.org.uk>]

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 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)...

> > > > 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...