Re: [RFC][PATCH] getname_maybe_null() - the third variant of pathname copy-in

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Oct 18, 2024 at 01:06:12PM +0200, Christian Brauner wrote:
> > Look: all it takes is the following trick
> > 	* add const char *pathname to struct nameidata
> > 	* in __set_nameidata() add
> >         p->pathname = likely(name) ? name->name : "";
> > 	* in path_init() replace
> > 	const char *s = nd->name->name;
> > with
> > 	const char *s = nd->pathname;
> > and we are done.  Oh, and teach putname() to treat NULL as no-op.
> 
> I know, that's what I suggested to Linus initially but he NAKed it
> because he didn't want the extra cycles.

Extra cycles where?  If anything, I'd expect a too-small-to-measure
speedup due to dereference shifted from path_init() to __set_nameidata().
Below is literally all it takes to make filename_lookup() treat NULL
as empty-string name.

NOTE: I'm not talking about forcing the pure by-descriptor case through
the dfd+pathname codepath; not without serious profiling.  But treating
AT_FDCWD + NULL by the delta below and passing NULL struct filename to
filename_lookup()?  Where do you expect to have the lost cycles on that?

diff --git a/fs/namei.c b/fs/namei.c
index 4a4a22a08ac2..fc2053877e5c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -264,7 +264,7 @@ EXPORT_SYMBOL(getname_kernel);
 
 void putname(struct filename *name)
 {
-	if (IS_ERR(name))
+	if (IS_ERR_OR_NULL(name))
 		return;
 
 	if (WARN_ON_ONCE(!atomic_read(&name->refcnt)))
@@ -588,6 +588,7 @@ struct nameidata {
 		unsigned seq;
 	} *stack, internal[EMBEDDED_LEVELS];
 	struct filename	*name;
+	const char *pathname;
 	struct nameidata *saved;
 	unsigned	root_seq;
 	int		dfd;
@@ -606,6 +607,7 @@ static void __set_nameidata(struct nameidata *p, int dfd, struct filename *name)
 	p->depth = 0;
 	p->dfd = dfd;
 	p->name = name;
+	p->pathname = likely(name) ? name->name : "";
 	p->path.mnt = NULL;
 	p->path.dentry = NULL;
 	p->total_link_count = old ? old->total_link_count : 0;
@@ -2439,7 +2441,7 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 static const char *path_init(struct nameidata *nd, unsigned flags)
 {
 	int error;
-	const char *s = nd->name->name;
+	const char *s = nd->pathname;
 
 	/* LOOKUP_CACHED requires RCU, ask caller to retry */
 	if ((flags & (LOOKUP_RCU | LOOKUP_CACHED)) == LOOKUP_CACHED)