mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mateusz Guzik <mjguzik@gmail.com>
To: brauner@kernel.org, viro@zeniv.linux.org.uk
Cc: jack@suse.cz, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, clm@meta.com,
	Mateusz Guzik <mjguzik@gmail.com>
Subject: [RFC PATCH v2] fs: touch up symlink clean up in lookup
Date: Wed, 17 Dec 2025 12:23:45 +0100	[thread overview]
Message-ID: <20251217112345.2340007-1-mjguzik@gmail.com> (raw)

Provide links_cleanup_rcu() and links_cleanup_ref() for rcu- and ref-
walks respectively.

The somewhat misleading drop_links() gets renamed to links_issue_delayed_calls(),
which spells out what it is actually doing.

There are no changes in behavior, this however should be less
error-prone going forward.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/namei.c | 45 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

v2:
- remove the mindless copy-paste in links_cleanup_ref, instead only move
  path_puts on the symlinks

this performs path_put on nd->path later, which should not be a material
difference.

this assumes the LOOKUP_CACHED fixup is applied:
https://lore.kernel.org/linux-fsdevel/20251217104854.GU1712166@ZenIV/T/#mff14d1dd88729f40fa94ada8beaa64e0c41097ff

technically the 2 patches could be combined, but I did not want to do it
given the bug

Chris, can you feed this thing into the magic llm? Better yet, is it
something I can use myself? I passed the known buggy patch to gcc
-fanalyze and clang --analyze and neither managed to point out any
issues.

diff --git a/fs/namei.c b/fs/namei.c
index 69d0aa9ad2a8..7d1722cebe99 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -766,7 +766,7 @@ static bool path_connected(struct vfsmount *mnt, struct dentry *dentry)
 	return is_subdir(dentry, mnt->mnt_root);
 }
 
-static void drop_links(struct nameidata *nd)
+static void links_issue_delayed_calls(struct nameidata *nd)
 {
 	int i = nd->depth;
 	while (i--) {
@@ -774,6 +774,29 @@ static void drop_links(struct nameidata *nd)
 		do_delayed_call(&last->done);
 		clear_delayed_call(&last->done);
 	}
+}
+
+static void links_cleanup_rcu(struct nameidata *nd)
+{
+	VFS_BUG_ON(!(nd->flags & LOOKUP_RCU));
+
+	if (likely(!nd->depth))
+		return;
+
+	links_issue_delayed_calls(nd);
+	nd->depth = 0;
+}
+
+static void links_cleanup_ref(struct nameidata *nd)
+{
+	VFS_BUG_ON(nd->flags & LOOKUP_RCU);
+
+	if (likely(!nd->depth))
+		return;
+
+	links_issue_delayed_calls(nd);
+	for (int i = 0; i < nd->depth; i++)
+		path_put(&nd->stack[i].link);
 	nd->depth = 0;
 }
 
@@ -786,20 +809,16 @@ static void leave_rcu(struct nameidata *nd)
 
 static void terminate_walk(struct nameidata *nd)
 {
-	int depth = nd->depth;
-	if (unlikely(depth))
-		drop_links(nd);
-	if (!(nd->flags & LOOKUP_RCU)) {
-		int i;
+	if (nd->flags & LOOKUP_RCU) {
+		links_cleanup_rcu(nd);
+		leave_rcu(nd);
+	} else {
+		links_cleanup_ref(nd);
 		path_put(&nd->path);
-		for (i = 0; i < depth; i++)
-			path_put(&nd->stack[i].link);
 		if (nd->state & ND_ROOT_GRABBED) {
 			path_put(&nd->root);
 			nd->state &= ~ND_ROOT_GRABBED;
 		}
-	} else {
-		leave_rcu(nd);
 	}
 	VFS_BUG_ON(nd->depth);
 	nd->path.mnt = NULL;
@@ -838,7 +857,7 @@ static bool legitimize_links(struct nameidata *nd)
 	for (i = 0; i < nd->depth; i++) {
 		struct saved *last = nd->stack + i;
 		if (unlikely(!legitimize_path(nd, &last->link, last->seq))) {
-			drop_links(nd);
+			links_issue_delayed_calls(nd);
 			nd->depth = i + 1;
 			return false;
 		}
@@ -884,7 +903,7 @@ static bool try_to_unlazy(struct nameidata *nd)
 	BUG_ON(!(nd->flags & LOOKUP_RCU));
 
 	if (unlikely(nd->flags & LOOKUP_CACHED)) {
-		drop_links(nd);
+		links_cleanup_rcu(nd);
 		goto out1;
 	}
 	if (unlikely(nd->depth && !legitimize_links(nd)))
@@ -923,7 +942,7 @@ static bool try_to_unlazy_next(struct nameidata *nd, struct dentry *dentry)
 	BUG_ON(!(nd->flags & LOOKUP_RCU));
 
 	if (unlikely(nd->flags & LOOKUP_CACHED)) {
-		drop_links(nd);
+		links_cleanup_rcu(nd);
 		goto out2;
 	}
 	if (unlikely(nd->depth && !legitimize_links(nd)))
-- 
2.48.1


             reply	other threads:[~2025-12-17 11:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-17 11:23 Mateusz Guzik [this message]
2025-12-17 11:38 ` Al Viro
2025-12-17 11:45   ` Mateusz Guzik
2025-12-18 17:11 ` Chris Mason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251217112345.2340007-1-mjguzik@gmail.com \
    --to=mjguzik@gmail.com \
    --cc=brauner@kernel.org \
    --cc=clm@meta.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®