Re: [RFC PATCH 20/25] d_walk(): shrink rcu_read_lock() scope

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

On Tue, May 05, 2026 at 11:40:44PM +0200, Frederic Weisbecker wrote:

> > I would really prefer to have all explicit rcu_read_lock() in fs/dcache.c
> > clearly documented ;-/
> 
> And the calls to rcu_read_lock() / rcu_read_unlock() are _usually_ cheap enough
> that their explicit calls should go unnoticed over an implicit spinlock RCU
> critical section. Ok rcu_read_unlock() has its slowpath but it would otherwise
> trigger randomly when implicit read side are used (context switch, interrupts,
> etc...)
> 
> Also rcu_read_lock() has the advantage to tell clearly that we are doing an RCU read
> side (and its uses indeed claim to be documented) here while relying on spinlocks to do
> that is fine but implicit and more error-prone when there are tricky things
> like spinlock transfers: better don't ask for trouble.
> 
> So I would say that we should probably avoid introducing such an API if possible.

The problem is, for dentries such transfer may be _less_ error-prone than
a large rcu_read_lock() scope.  There's such thing as NORCU dentries, and
for those freeing is not RCU-delayed.  So for d_walk() the safety of
                dentry = this_parent;
		this_parent = dentry->d_parent;
		 
		spin_unlock(&dentry->d_lock);
		spin_lock(&this_parent->d_lock);
requires more than just "everything is in RCU read-side critical area"; it's
also "and NORCU dentries never occur as anyone's parents".

We do rely upon ->d_lock for all other transitions in that graph walk -
for descent into a subtree and for traversals into the next sibling,
including those during ascent from subtree.

Any place where we rely solely upon rcu_read_lock() needs an analysis of
NORCU case and it's not always "NORCU can't occur here" - e.g. fast_dput()
is hit every time we close a pipe or socket, and these are the original
reason for having NORCU in the first place.

FWIW, the rules (and I'm still not finished writing down a coherent
variant of those) are very heavily ->d_lock-based; basically, we have
3 safe cases - counting reference, RCU reference and locked reference.
We don't have that enforced by the type system, unfortunately, and I'm
not sure we *can* express that with C type system in a way that would
be entirely compiler-enforced.  I do think we can reduce most of that
down to a handful of "prove the safety manually" areas; we'll see.

Long story short, rcu_read_lock() is really too blunt a tool in that area.
Look at the list of scopes in
https://lore.kernel.org/all/20260507073510.GL3518998@ZenIV/
and tell me if you like having to rely upon that kind of analysis in
proof of correctness...