From: Linus Torvalds torvalds@linux-foundation.org

commit 9013c51 upstream.

Mateusz reports that glibc turns 'fstat()' calls into 'fstatat()', and that seems to have been going on for quite a long time due to glibc having tried to simplify its stat logic into just one point.

This turns out to cause completely unnecessary overhead, where we then go off and allocate the kernel side pathname, and actually look up the empty path. Sure, our path lookup is quite optimized, but it still causes a fair bit of allocation overhead and a couple of completely unnecessary rounds of lockref accesses etc.

This is all hopefully getting fixed in user space, and there is a patch floating around for just having glibc use the native fstat() system call. But even with the current situation we can at least improve on things by catching the situation and short-circuiting it.

Note that this is still measurably slower than just a plain 'fstat()', since just checking that the filename is actually empty is somewhat expensive due to inevitable user space access overhead from the kernel (ie verifying pointers, and SMAP on x86). But it's still quite a bit faster than actually looking up the path for real.

To quote numers from Mateusz: "Sapphire Rapids, will-it-scale, ops/s

stock fstat 5088199 patched fstat 7625244 (+49%) real fstat 8540383 (+67% / +12%)"

where that 'stock fstat' is the glibc translation of fstat into fstatat() with an empty path, the 'patched fstat' is with this short circuiting of the path lookup, and the 'real fstat' is the actual native fstat() system call with none of this overhead.

Link: https://lore.kernel.org/lkml/20230903204858.lv7i3kqvw6eamhgz@f/ Reported-by: Mateusz Guzik mjguzik@gmail.com Signed-off-by: Linus Torvalds torvalds@linux-foundation.org

Cc: stable@vger.kernel.org # 6.1.x Signed-off-by: Miao Wang shankerwangmiao@gmail.com Tested-by: Xi Ruoyao xry111@xry111.site --- fs/stat.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)

diff --git a/fs/stat.c b/fs/stat.c index ef50573c72a2..5843f73f3e7e 100644 --- a/fs/stat.c +++ b/fs/stat.c @@ -263,6 +263,23 @@ int vfs_fstatat(int dfd, const char __user *filename, int statx_flags = flags | AT_NO_AUTOMOUNT; struct filename *name;

+ /* + * Work around glibc turning fstat() into fstatat(AT_EMPTY_PATH) + * + * If AT_EMPTY_PATH is set, we expect the common case to be that + * empty path, and avoid doing all the extra pathname work. + */ + if (dfd >= 0 && flags == AT_EMPTY_PATH) { + char c; + + ret = get_user(c, filename); + if (unlikely(ret)) + return ret; + + if (likely(!c)) + return vfs_fstat(dfd, stat); + } + name = getname_flags(filename, getname_statx_lookup_flags(statx_flags), NULL); ret = vfs_statx(dfd, name, statx_flags, stat, STATX_BASIC_STATS); putname(name);

From: Christian Brauner brauner@kernel.org

commit 1bc6d44 upstream.

Make it possible to quickly check whether AT_EMPTY_PATH is valid. Note, after some discussion we decided to also allow NULL to be passed instead of requiring the empty string.

Signed-off-by: Christian Brauner brauner@kernel.org

Cc: stable@vger.kernel.org # 6.1.x Signed-off-by: Miao Wang shankerwangmiao@gmail.com Tested-by: Xi Ruoyao xry111@xry111.site --- include/linux/fs.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h index f2206c78755a..deb0abe728a7 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3692,4 +3692,21 @@ extern int vfs_fadvise(struct file *file, loff_t offset, loff_t len, extern int generic_fadvise(struct file *file, loff_t offset, loff_t len, int advice);

+static inline bool vfs_empty_path(int dfd, const char __user *path) +{ + char c; + + if (dfd < 0) + return false; + + /* We now allow NULL to be used for empty path. */ + if (!path) + return true; + + if (unlikely(get_user(c, path))) + return false; + + return !c; +} + #endif /* _LINUX_FS_H */

From: Mateusz Guzik mjguzik@gmail.com

commit 0ef625b upstream.

The newly used helper also checks for empty ("") paths.

NULL paths with any flag value other than AT_EMPTY_PATH go the usual route and end up with -EFAULT to retain compatibility (Rust is abusing calls of the sort to detect availability of statx).

This avoids path lookup code, lockref management, memory allocation and in case of NULL path userspace memory access (which can be quite expensive with SMAP on x86_64).

Benchmarked with statx(..., AT_EMPTY_PATH, ...) running on Sapphire Rapids, with the "" path for the first two cases and NULL for the last one.

Results in ops/s: stock: 4231237 pre-check: 5944063 (+40%) NULL path: 6601619 (+11%/+56%)

Signed-off-by: Mateusz Guzik mjguzik@gmail.com Link: https://lore.kernel.org/r/20240625151807.620812-1-mjguzik@gmail.com Tested-by: Xi Ruoyao xry111@xry111.site [brauner: use path_mounted() and other tweaks] Signed-off-by: Christian Brauner brauner@kernel.org

Cc: stable@vger.kernel.org # 6.1.x Signed-off-by: Miao Wang shankerwangmiao@gmail.com Tested-by: Xi Ruoyao xry111@xry111.site --- fs/internal.h | 2 ++ fs/stat.c | 92 +++++++++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 22 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h index 42df013f7fe7..004af47be35b 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -199,6 +199,8 @@ int sb_init_dio_done_wq(struct super_block *sb); int getname_statx_lookup_flags(int flags); int do_statx(int dfd, struct filename *filename, unsigned int flags, unsigned int mask, struct statx __user *buffer); +int do_statx_fd(int fd, unsigned int flags, unsigned int mask, + struct statx __user *buffer);

/* * fs/splice.c: diff --git a/fs/stat.c b/fs/stat.c index 84e07356dfa4..61b9eefb19fe 100644 --- a/fs/stat.c +++ b/fs/stat.c @@ -199,6 +199,38 @@ int getname_statx_lookup_flags(int flags) return lookup_flags; }

+static int vfs_statx_path(struct path *path, int flags, struct kstat *stat, + u32 request_mask) +{ + int error = vfs_getattr(path, stat, request_mask, flags); + + stat->mnt_id = real_mount(path->mnt)->mnt_id; + stat->result_mask |= STATX_MNT_ID; + + if (path->mnt->mnt_root == path->dentry) + stat->attributes |= STATX_ATTR_MOUNT_ROOT; + stat->attributes_mask |= STATX_ATTR_MOUNT_ROOT; + + /* Handle STATX_DIOALIGN for block devices. */ + if (request_mask & STATX_DIOALIGN) { + struct inode *inode = d_backing_inode(path->dentry); + + if (S_ISBLK(inode->i_mode)) + bdev_statx_dioalign(inode, stat); + } + + return error; +} + +static int vfs_statx_fd(int fd, int flags, struct kstat *stat, + u32 request_mask) +{ + CLASS(fd_raw, f)(fd); + if (!f.file) + return -EBADF; + return vfs_statx_path(&f.file->f_path, flags, stat, request_mask); +} + /** * vfs_statx - Get basic and extra attributes by filename * @dfd: A file descriptor representing the base dir for a relative filename @@ -228,31 +260,13 @@ static int vfs_statx(int dfd, struct filename *filename, int flags, retry: error = filename_lookup(dfd, filename, lookup_flags, &path, NULL); if (error) - goto out; - - error = vfs_getattr(&path, stat, request_mask, flags); - - stat->mnt_id = real_mount(path.mnt)->mnt_id; - stat->result_mask |= STATX_MNT_ID; - - if (path.mnt->mnt_root == path.dentry) - stat->attributes |= STATX_ATTR_MOUNT_ROOT; - stat->attributes_mask |= STATX_ATTR_MOUNT_ROOT; - - /* Handle STATX_DIOALIGN for block devices. */ - if (request_mask & STATX_DIOALIGN) { - struct inode *inode = d_backing_inode(path.dentry); - - if (S_ISBLK(inode->i_mode)) - bdev_statx_dioalign(inode, stat); - } - + return error; + error = vfs_statx_path(&path, flags, stat, request_mask); path_put(&path); if (retry_estale(error, lookup_flags)) { lookup_flags |= LOOKUP_REVAL; goto retry; } -out: return error; }

@@ -656,16 +670,35 @@ int do_statx(int dfd, struct filename *filename, unsigned int flags, return cp_statx(&stat, buffer); }

+int do_statx_fd(int fd, unsigned int flags, unsigned int mask, + struct statx __user *buffer) +{ + struct kstat stat; + int error; + + if (mask & STATX__RESERVED) + return -EINVAL; + if ((flags & AT_STATX_SYNC_TYPE) == AT_STATX_SYNC_TYPE) + return -EINVAL; + + error = vfs_statx_fd(fd, flags, &stat, mask); + if (error) + return error; + + return cp_statx(&stat, buffer); +} + /** * sys_statx - System call to get enhanced stats * @dfd: Base directory to pathwalk from *or* fd to stat. - * @filename: File to stat or "" with AT_EMPTY_PATH + * @filename: File to stat or either NULL or "" with AT_EMPTY_PATH * @flags: AT_* flags to control pathwalk. * @mask: Parts of statx struct actually required. * @buffer: Result buffer. * * Note that fstat() can be emulated by setting dfd to the fd of interest, - * supplying "" as the filename and setting AT_EMPTY_PATH in the flags. + * supplying "" (or preferably NULL) as the filename and setting AT_EMPTY_PATH + * in the flags. */ SYSCALL_DEFINE5(statx, int, dfd, const char __user *, filename, unsigned, flags, @@ -673,8 +706,23 @@ SYSCALL_DEFINE5(statx, struct statx __user *, buffer) { int ret; + unsigned lflags; struct filename *name;

+ /* + * Short-circuit handling of NULL and "" paths. + * + * For a NULL path we require and accept only the AT_EMPTY_PATH flag + * (possibly |'d with AT_STATX flags). + * + * However, glibc on 32-bit architectures implements fstatat as statx + * with the "" pathname and AT_NO_AUTOMOUNT | AT_EMPTY_PATH flags. + * Supporting this results in the uglification below. + */ + lflags = flags & ~(AT_NO_AUTOMOUNT | AT_STATX_SYNC_TYPE); + if (lflags == AT_EMPTY_PATH && vfs_empty_path(dfd, filename)) + return do_statx_fd(dfd, flags & ~AT_NO_AUTOMOUNT, mask, buffer); + name = getname_flags(filename, getname_statx_lookup_flags(flags), NULL); ret = do_statx(dfd, name, flags, mask, buffer); putname(name);