[MEH PATCH] fs: move fd_install() slowpath into a dedicated routine and provide commentary

[MEH PATCH] fs: move fd_install() slowpath into a dedicated routine and provide commentary

[Mateusz Guzik]

On stock kernel gcc 14 emits avoidable register spillage:
	endbr64
	call   ffffffff81374630 <__fentry__>
	push   %r13
	push   %r12
	push   %rbx
	sub    $0x8,%rsp
	[snip]

Total fast path is 99 bytes.

Moving the slowpath out avoids it and shortens the fast path to 74
bytes.

Take this opportunity to elaborate on the resize_in_progress machinery.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---

I don't feel particularly strongly about the patch, so if there is
resistance and I'm not going to argue for it.

Spotted on the profile while looking at open()

 fs/file.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 28743b742e3c..d73730203bb5 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -641,6 +641,35 @@ void put_unused_fd(unsigned int fd)
 
 EXPORT_SYMBOL(put_unused_fd);
 
+/*
+ * Install a file pointer in the fd array while it is being resized.
+ *
+ * We need to make sure our update to the array does not get lost as the resizing
+ * thread can be copying the content as we modify it.
+ *
+ * We have two ways to do it:
+ * - go off CPU waiting for resize_in_progress to clear
+ * - take the spin lock
+ *
+ * The latter is trivial to implement and saves us from having to might_sleep()
+ * for debugging purposes.
+ *
+ * This is moved out of line from fd_install() to convince gcc to optimize that
+ * routine better.
+ */
+static void noinline fd_install_slowpath(unsigned int fd, struct file *file)
+{
+	struct files_struct *files = current->files;
+	struct fdtable *fdt;
+
+	rcu_read_unlock_sched();
+	spin_lock(&files->file_lock);
+	fdt = files_fdtable(files);
+	VFS_BUG_ON(rcu_access_pointer(fdt->fd[fd]) != NULL);
+	rcu_assign_pointer(fdt->fd[fd], file);
+	spin_unlock(&files->file_lock);
+}
+
 /**
  * fd_install - install a file pointer in the fd array
  * @fd: file descriptor to install the file in
@@ -658,14 +687,8 @@ void fd_install(unsigned int fd, struct file *file)
 		return;
 
 	rcu_read_lock_sched();
-
 	if (unlikely(files->resize_in_progress)) {
-		rcu_read_unlock_sched();
-		spin_lock(&files->file_lock);
-		fdt = files_fdtable(files);
-		VFS_BUG_ON(rcu_access_pointer(fdt->fd[fd]) != NULL);
-		rcu_assign_pointer(fdt->fd[fd], file);
-		spin_unlock(&files->file_lock);
+		fd_install_slowpath(fd, file);
 		return;
 	}
 	/* coupled with smp_wmb() in expand_fdtable() */
-- 
2.48.1

Re: [MEH PATCH] fs: move fd_install() slowpath into a dedicated routine and provide commentary

[Al Viro]

On Sun, Nov 09, 2025 at 01:02:59PM +0100, Mateusz Guzik wrote:

> +/*
> + * Install a file pointer in the fd array while it is being resized.
> + *
> + * We need to make sure our update to the array does not get lost as the resizing
> + * thread can be copying the content as we modify it.
> + *
> + * We have two ways to do it:
> + * - go off CPU waiting for resize_in_progress to clear
> + * - take the spin lock
> + *
> + * The latter is trivial to implement and saves us from having to might_sleep()
> + * for debugging purposes.
> + *
> + * This is moved out of line from fd_install() to convince gcc to optimize that
> + * routine better.
> + */

Does it become seriously worse if you move rcu_read_unlock_sched() into the caller?