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 10:01:39AM -0700, Linus Torvalds wrote:

> If we had a function that was "transfer spinlock without dropping RCU
> read lock", and we'd use that as an _optimization_ (maybe we could
> avoid all the preemption count games and testing for whether there are
> active RCU events etc), that would make sense to me.
>
> At that point it would *all* become "we rely on the spinlock to hold
> RCU read locking", and it wouldn't mix locking models. So I think such
> an operation might make sense on its own.

Definitely; we have the same "bridge two spin_lock scopes together into
a contiguous RCU read-side critical area" elsewhere as well.  Hell,

	rcu_read_lock();
	do {
		spin_unlock(&dentry->d_lock);
		spin_lock(&inode->i_lock);
		spin_lock(&dentry->d_lock);
		if (likely(inode == dentry->d_inode))
			break;
		spin_unlock(&inode->i_lock);
		inode = dentry->d_inode;
	} while (inode);
	rcu_read_unlock();

in lock_for_kill() might be better off as

	do {
		transfer_spinlock_with_rcu(&dentry->d_lock, &inode->i_lock);
		spin_lock(&dentry->d_lock);
		if (likely(inode == dentry->d_inode))
			break;
		spin_unlock(&inode->i_lock);
		inode = dentry->d_inode;
	} while (inode);

Explicit rcu_read_lock() uses don't explain what's really going on; in this
form it's "we can't take ->i_lock under ->d_lock, but we need an unbroken
RCU read-side critical area for memory safety".  The same goes for d_walk()
use ("can't take parent's ->d_lock under child's one"), as well as in
shrink_dcache_for_umount() ("can't take ->d_lock under ->s_roots_lock")
and I wouldn't be surprised to see other instances of the same pattern.

I would really prefer to have all explicit rcu_read_lock() in fs/dcache.c
clearly documented ;-/

FWIW, back in 2015 spin_lock() did *not* provide an RCU read-side critical
area, according to https://lwn.net/Articles/652156/; would be nice to have
the history mentioned in D/RCU/whatisRCU.rst - at least to the level of
"once upon a time one *did* need an explicit rcu_read_lock() in addition
to spin_lock() - spin_lock() or preempt_disable() alone was not enough;
that has changed in <...>".

I'm not familiar enough with that code for that kind of archaeology ;-/
Could somebody help with that?  Paul?