Re: [RFC PATCH 21/25] shrinking rcu_read_lock() scope in d_alloc_parallel()

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

On Thu, May 07, 2026 at 11:52:21PM +0200, Jori Koolstra wrote:
> > 	If no match is found, we proceed to lock the hash chain of
> > in-lookup hash and scan that for a match.  If we find a match, we want
> > to grab it and wait for lookup in progress to finish.  Since the bitlock
> > we use for these hash chains has to nest inside ->d_lock, we need to
> > unlock the chain first and use lockref_get_not_dead() on the match.
> 
> Just curious, reading this code as someone fairly new to the kernel,
> how can I know what is the agreed-upon lock order between these hash
> chains and the d_locks of the dentries they contain?

Either wait for documentation to get there, or look at the places where
we deal with those locks; in particular, the removal from the in-lookup
hash chain comes in __d_lookup_unhash():
        hlist_bl_lock(b);
	dentry->d_flags &= ~DCACHE_PAR_LOOKUP;
	__hlist_bl_del(&dentry->d_in_lookup_hash);
	hlist_bl_unlock(b);
with callers of that thing all done under ->d_lock.  And yes, we *do*
need to document that shite - no arguments here.

> > 	That makes the entire thing easier to follow and the purpose
> > of those rcu_read_lock() calls easier to describe - the first scope is
> > for __d_lookup_rcu() + lockref_get_not_dead(), the second one bridges
> > over from the bitlock scope to the ->d_lock scope on the match found in
> > in-lookup hash.
> 
> Because the dentry could be freed after the hash chain unlock and before
> grabbing the d_lock?

Yes.  It's a bit more complicated - the missing bit is "and dentry we might
find in the in-lookup hash is not going to be NORCU, so freeing is guaranteed
to be RCU-delayed".

> I do think the hlist_bl_for_each_entry loop would read clearer if we
> pull the code after having found the initial match out of it, since we
> always exit the loop afterwards.

Umm...  Not sure - we'd need to distinguish between "no match found" and "got
a match, stopped looking" cases.  Would have to be something like
	if (!pos)
		insert into hash
	ele
		dentry is a match, deal with it
which is more reliant upon the details of hlist_bl_for_each_entry than I'd like...