From 253ca8678d30bcf94410b54476fc1e0f1627a137 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sun, 26 Nov 2023 12:24:38 -0800 Subject: Improve __fget_files_rcu() code generation (and thus __fget_light()) Commit 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") caused a performance regression as reported by the kernel test robot. The __fget_light() function is one of those critical ones for some loads, and the code generation was unnecessarily impacted. Let's just write that function to better. Reported-by: kernel test robot Cc: Christian Brauner Cc: Jann Horn Cc: Mateusz Guzik Closes: https://lore.kernel.org/oe-lkp/202311201406.2022ca3f-oliver.sang@intel.com Signed-off-by: Linus Torvalds Link: https://lore.kernel.org/r/CAHk-=wiCJtLbFWNURB34b9a_R_unaH3CiMRXfkR0-iihB_z68A@mail.gmail.com Signed-off-by: Christian Brauner --- include/linux/fdtable.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'include/linux/fdtable.h') diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index bc4c3287a65e..80bd7789bab1 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -83,12 +83,17 @@ struct dentry; static inline struct file *files_lookup_fd_raw(struct files_struct *files, unsigned int fd) { struct fdtable *fdt = rcu_dereference_raw(files->fdt); - - if (fd < fdt->max_fds) { - fd = array_index_nospec(fd, fdt->max_fds); - return rcu_dereference_raw(fdt->fd[fd]); - } - return NULL; + unsigned long mask = array_index_mask_nospec(fd, fdt->max_fds); + struct file *needs_masking; + + /* + * 'mask' is zero for an out-of-bounds fd, all ones for ok. + * 'fd&mask' is 'fd' for ok, or 0 for out of bounds. + * + * Accessing fdt->fd[0] is ok, but needs masking of the result. + */ + needs_masking = rcu_dereference_raw(fdt->fd[fd&mask]); + return (struct file *)(mask & (unsigned long)needs_masking); } static inline struct file *files_lookup_fd_locked(struct files_struct *files, unsigned int fd) -- cgit v1.2.3 From a88c955fcfb49727d0ed86b47410f6555a8e69e4 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 30 Nov 2023 13:49:07 +0100 Subject: file: s/close_fd_get_file()/file_close_fd()/g That really shouldn't have "get" in there as that implies we're bumping the reference count which we don't do at all. We used to but not anmore. Now we're just closing the fd and pick that file from the fdtable without bumping the reference count. Update the wrong documentation while at it. Link: https://lore.kernel.org/r/20231130-vfs-files-fixes-v1-1-e73ca6f4ea83@kernel.org Reviewed-by: Jan Kara Reviewed-by: Jens Axboe Signed-off-by: Christian Brauner --- drivers/android/binder.c | 2 +- fs/file.c | 14 +++++++++----- fs/open.c | 2 +- include/linux/fdtable.h | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) (limited to 'include/linux/fdtable.h') diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 92128aae2d06..7658103ba760 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -1921,7 +1921,7 @@ static void binder_deferred_fd_close(int fd) if (!twcb) return; init_task_work(&twcb->twork, binder_do_fd_close); - twcb->file = close_fd_get_file(fd); + twcb->file = file_close_fd(fd); if (twcb->file) { // pin it until binder_do_fd_close(); see comments there get_file(twcb->file); diff --git a/fs/file.c b/fs/file.c index 50df31e104a5..66f04442a384 100644 --- a/fs/file.c +++ b/fs/file.c @@ -796,7 +796,7 @@ int __close_range(unsigned fd, unsigned max_fd, unsigned int flags) } /* - * See close_fd_get_file() below, this variant assumes current->files->file_lock + * See file_close_fd() below, this variant assumes current->files->file_lock * is held. */ struct file *__close_fd_get_file(unsigned int fd) @@ -804,11 +804,15 @@ struct file *__close_fd_get_file(unsigned int fd) return pick_file(current->files, fd); } -/* - * variant of close_fd that gets a ref on the file for later fput. - * The caller must ensure that filp_close() called on the file. +/** + * file_close_fd - return file associated with fd + * @fd: file descriptor to retrieve file for + * + * Doesn't take a separate reference count. + * + * Returns: The file associated with @fd (NULL if @fd is not open) */ -struct file *close_fd_get_file(unsigned int fd) +struct file *file_close_fd(unsigned int fd) { struct files_struct *files = current->files; struct file *file; diff --git a/fs/open.c b/fs/open.c index 02dc608d40d8..48775329f3f1 100644 --- a/fs/open.c +++ b/fs/open.c @@ -1577,7 +1577,7 @@ SYSCALL_DEFINE1(close, unsigned int, fd) int retval; struct file *file; - file = close_fd_get_file(fd); + file = file_close_fd(fd); if (!file) return -EBADF; diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index 80bd7789bab1..78c8326d74ae 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -119,7 +119,7 @@ int iterate_fd(struct files_struct *, unsigned, extern int close_fd(unsigned int fd); extern int __close_range(unsigned int fd, unsigned int max_fd, unsigned int flags); -extern struct file *close_fd_get_file(unsigned int fd); +extern struct file *file_close_fd(unsigned int fd); extern int unshare_fd(unsigned long unshare_flags, unsigned int max_fds, struct files_struct **new_fdp); -- cgit v1.2.3