[RFC PATCH v2] fs: touch up symlink clean up in lookup

[RFC PATCH v2] fs: touch up symlink clean up in lookup

[Mateusz Guzik]

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

Re: [RFC PATCH v2] fs: touch up symlink clean up in lookup

[Al Viro]

On Wed, Dec 17, 2025 at 12:23:45PM +0100, Mateusz Guzik wrote:
> 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.

IMO the replacement name is worse; it doesn't say anything about
the purpose of those "issued delayed calls", for starters.

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

I disagree, but at the moment (6:30am here, and I'd been up since 11am yesterday ;-/)
I don't trust my ability to produce coherent detailed reply.  I'll reply in detail
once I get some sleep...

Re: [RFC PATCH v2] fs: touch up symlink clean up in lookup

[Mateusz Guzik]

On Wed, Dec 17, 2025 at 12:37 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Wed, Dec 17, 2025 at 12:23:45PM +0100, Mateusz Guzik wrote:
> > 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.
>
> IMO the replacement name is worse; it doesn't say anything about
> the purpose of those "issued delayed calls", for starters.
>

Then name it something else.

I don't care how the patch looks like for the most part, as long as
the crux is retained: dedicated routines for both rcu and ref walks
provided which completely take care of symlink clean up.

> > There are no changes in behavior, this however should be less
> > error-prone going forward.
>
> I disagree, but at the moment (6:30am here, and I'd been up since 11am yesterday ;-/)
> I don't trust my ability to produce coherent detailed reply.  I'll reply in detail
> once I get some sleep...

A good night sleep is something I could use myself. ;-)

Re: [RFC PATCH v2] fs: touch up symlink clean up in lookup

[Chris Mason]

On 12/17/25 6:23 AM, Mateusz Guzik wrote:
> 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.
> 

Looks like you and Al are still discussing the specifics, but I ran this
version through the review prompts and it didn't find any problems.

With claude or gemini subscriptions, you can run them locally:

https://github.com/masoncl/review-prompts

(Other AI providers may work too, but those are the only two I've heard
about so far)

-chris