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 08:38:22PM +0100, Al Viro wrote:
> On Fri, Oct 18, 2024 at 05:51:58PM +0100, Al Viro wrote:
> 
> > 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?
> 
> [snip]
> 
> BTW, could you give me a reference to the mail with those objections?
> I don't see anything in my mailbox, but...
> 
> Or was that in one of those AT_EMPTY_PATH_NOCHECK (IIRC?) threads?
> 
> Anyway, what I'm suggesting is
> 
> 1) teach filename_lookup() to handle NULL struct filename * argument, treating
> it as "".  Trivial and does not impose any overhead on the normal cases.
> 
> 2) have statx try to recognize AT_EMPTY_PATH, "" and AT_EMPTY_PATH, NULL.
> If we have that and dfd is *NOT* AT_FDCWD, we have a nice descriptor-based
> case and can deal with it.
> If the name is not empty, we have to go for dfd+filename path.  Also obvious.
> Where we get trouble is AT_FDCWD, NULL case.  But with (1) we can simply
> route that to the same dfd+filename path, just passing it NULL for filename.
> 
> That handles the currently broken case, with very little disruption to
> anything else.

See #getname.fixup; on top of #base.getname and IMO worth folding into it.
I don't believe that it's going to give any measurable slowdown compared to
mainline.  Again, if the concerns about wasted cycles had been about routing
the dfd,"" and dfd,NULL cases through the filename_lookup(), this does *NOT*
happen with that patch.  Christian, Linus?

commit 9fb26eb324d9aa9e6889f181f1683e710946258f
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Sat Oct 19 00:57:14 2024 -0400

    fix statx(2) regression on AT_FDCWD,"" and AT_FDCWD,NULL
    
    teach filename_lookup() et.al. to handle NULL struct filename reference
    as ""; make AT_FDCWD,"" (but _NOT_ the normal dfd,"") case in statx(2)
    fall back to path-based variant.
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/namei.c b/fs/namei.c
index 27eb0a81d9b8..2bfe476c3bd0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -280,7 +280,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)))
@@ -604,6 +604,7 @@ struct nameidata {
 		unsigned seq;
 	} *stack, internal[EMBEDDED_LEVELS];
 	struct filename	*name;
+	const char *pathname;
 	struct nameidata *saved;
 	unsigned	root_seq;
 	int		dfd;
@@ -622,6 +623,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;
@@ -2455,7 +2457,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)
diff --git a/fs/stat.c b/fs/stat.c
index d0d82efd44d6..b74831dc7ae6 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -328,7 +328,7 @@ int vfs_fstatat(int dfd, const char __user *filename,
 	int statx_flags = flags | AT_NO_AUTOMOUNT;
 	struct filename *name = getname_maybe_null(filename, flags);
 
-	if (!name)
+	if (!name && dfd >= 0)
 		return vfs_fstat(dfd, stat);
 
 	ret = vfs_statx(dfd, name, statx_flags, stat, STATX_BASIC_STATS);
@@ -769,7 +769,7 @@ SYSCALL_DEFINE5(statx,
 	int ret;
 	struct filename *name = getname_maybe_null(filename, flags);
 
-	if (!name)
+	if (!name && dfd >= 0)
 		return do_statx_fd(dfd, flags & ~AT_NO_AUTOMOUNT, mask, buffer);
 
 	ret = do_statx(dfd, name, flags, mask, buffer);