[PATCH 4/4] kernfs: Remove kernfs_rwsem from dentry revalidation

[Shakeel Butt <shakeel.butt@linux.dev>]

kernfs_dop_revalidate() takes kernfs_rwsem for read once per path component
of every walk into a kernfs mount. Linux rwsems do not permit reader lock
stealing once a writer is queued, so a single writer parks the whole
incoming reader stream in uninterruptible sleep, stalling cgroup-polling
daemons for minutes.

Nothing the callback reads requires the semaphore. kn->active is an
atomic_t that kernfs_find_and_get_node_by_id() already tests through
__kernfs_active(); kn->__parent and kn->name are RCU pointers whose old
values are freed only after a grace period; kn->ns is now compared rather
than dereferenced; parent->dir.rev was annotated earlier in this series.

What the semaphore does provide is a coherent snapshot, and that is not
needed. ->d_revalidate() answers a question about a single instant, and the
answer is already stale when it returns: a rename landing just after
up_read() gives the same outcome as one observed mid-read. A lockless
reader can only return "valid" for the (parent, name, namespace) triple
identifying the dentry it was handed, and that triple was true when the
dentry was instantiated, so it reports a genuine past state exactly as the
locked version did. Removal is backstopped by kernfs_get_active() failing
in the subsequent open().

Take an RCU read lock instead. kernfs_parent() and kernfs_rcu_name() work
unchanged: the condition in their rcu_dereference_check() is an alternative
to holding the RCU read lock, not an extra requirement. The negative dentry
path needs nothing, as @dir pins the parent. The namespace check can use
@parent directly once the preceding check establishes it equals
kernfs_parent(kn), so the kn_parent local and its NULL test go away.

kernfs_ns_enabled() reads @parent->flags, which KERNFS_ACTIVATED and
KERNFS_REMOVING update as a plain read-modify-write under kernfs_rwsem.
Those bits are not read here and KERNFS_NS cannot change once the directory
has children, so mark the read data_race() rather than READ_ONCE(), which
would not silence KCSAN against the unmarked writers anyway.

kernfs_iop_permission() still forces every walk out of RCU-walk before
children are revalidated, so lifting the LOOKUP_RCU bail here would have no
observable effect; it is left to the series fixing that path.

Readers walking cgroupfs and sysfs against concurrent cgroup and netdev
churn: kernfs_rwsem read acquisitions drop from 48,593,360 to 1,280,280,
and kernfs_dop_revalidate() no longer appears among its contention sites.

Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 fs/kernfs/dir.c | 49 ++++++++++++++++++++-----------------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 27949b0e027c..cd7a8ff8b6b2 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1171,9 +1171,8 @@ struct kernfs_node *kernfs_create_empty_dir(struct kernfs_node *parent,
 static int kernfs_dop_revalidate(struct inode *dir, const struct qstr *name,
 				 struct dentry *dentry, unsigned int flags)
 {
-	struct kernfs_node *kn, *kn_parent;
 	struct kernfs_node *parent = dir->i_private;
-	struct kernfs_root *root;
+	struct kernfs_node *kn;
 	const char *kn_name;
 
 	if (flags & LOOKUP_RCU)
@@ -1191,49 +1190,41 @@ static int kernfs_dop_revalidate(struct inode *dir, const struct qstr *name,
 		 * changes and the lookup re-done so that a new positive
 		 * dentry can be properly created.
 		 */
-		root = kernfs_root(parent);
-		down_read(&root->kernfs_rwsem);
-		if (kernfs_dir_changed(parent, dentry)) {
-			up_read(&root->kernfs_rwsem);
-			return 0;
-		}
-		up_read(&root->kernfs_rwsem);
-
-		/* The kernfs parent node hasn't changed, leave the
-		 * dentry negative and return success.
-		 */
-		return 1;
+		return !kernfs_dir_changed(parent, dentry);
 	}
 
 	kn = kernfs_dentry_node(dentry);
-	root = kernfs_root(kn);
-	down_read(&root->kernfs_rwsem);
+
+	guard(rcu)();
 
 	/* The kernfs node has been deactivated */
-	if (!kernfs_active(kn))
-		goto out_bad;
+	if (!__kernfs_active(kn))
+		return 0;
 
-	kn_parent = kernfs_parent(kn);
 	/* The kernfs node has been moved? */
-	if (parent != kn_parent)
-		goto out_bad;
+	if (kernfs_parent(kn) != parent)
+		return 0;
 
 	/* The kernfs node has been renamed */
 	kn_name = kernfs_rcu_name(kn);
 	if (name->len != strlen(kn_name) ||
 	    memcmp(name->name, kn_name, name->len))
-		goto out_bad;
+		return 0;
 
-	/* The kernfs node has been moved to a different namespace */
-	if (kn_parent && kernfs_ns_enabled(kn_parent) &&
+	/*
+	 * The kernfs node has been moved to a different namespace.
+	 *
+	 * KERNFS_NS is set by kernfs_enable_ns() while @parent still has no
+	 * children, so it cannot change while a child of @parent is being
+	 * revalidated. The other bits in that word, KERNFS_ACTIVATED and
+	 * KERNFS_REMOVING, are updated under kernfs_rwsem and are not read
+	 * here, so racing with them is intentional and harmless.
+	 */
+	if (data_race(kernfs_ns_enabled(parent)) &&
 	    kernfs_info(dir->i_sb)->ns != READ_ONCE(kn->ns))
-		goto out_bad;
+		return 0;
 
-	up_read(&root->kernfs_rwsem);
 	return 1;
-out_bad:
-	up_read(&root->kernfs_rwsem);
-	return 0;
 }
 
 const struct dentry_operations kernfs_dops = {
-- 
2.53.0-Meta