On Thu, Jun 12, 2025 at 5:11 PM NeilBrown [off-list ref] wrote:
[...]

quoted

+
+false_out:
+     path_put(path);
+     memset(path, 0, sizeof(*path));
+     return false;
+}

I think the public function should return 0 on success and -error on
failure.  That is a well established pattern.

Yeah, I think we can use this pattern.

I also think you
shouldn't assume that all callers will want the same flags.

__path_walk_parent() only handles two LOOKUP_ flags, so
it is a bit weird to allow all the flags. But if folks think this is a
good idea, I don't have strong objections to taking various flags.

And it isn't clear to me why you want to path_put() on failure.

In earlier versions, we would keep "path" unchanged when the
walk stopped. However, this is not the case in this version
(choose_mountpoint() => in_root => return -EXDEV). So I
decided to just release it, so that we will not leak a path that
the walk should not get to.

I wonder if there might be other potential users in the kernel.
If so we should consider how well the interface meets their needs.

autofs, devpts, nfsd, landlock all call follow_up...
maybe they should be using the new interface...
nfsd is the most likely to benefit - particularly nfsd_lookup_parent().

AFAICT, autofs and devpts can just use follow_up().
For nfsd, nfsd_lookup_parent() and nfsd4_encode_pathname4() can
use path_walk_parent. And 2/5 covers landlock.

I think we can update nfsd in a follow up patch, just to keep this set
simpler.

Thanks,
Song

Just a thought..

[...]