diff mbox series

file: mostly eliminate spurious relocking in __range_close

Message ID 20230727113809.800067-1-mjguzik@gmail.com
State New
Headers
Series file: mostly eliminate spurious relocking in __range_close |

Commit Message

Mateusz Guzik July 27, 2023, 11:38 a.m. UTC 
  Stock code takes a lock trip for every fd in range, but this can be
trivially avoided and real-world consumers do have plenty of already
closed cases.

Just booting Debian 12 with a debug printk shows:
(sh) min 3 max 17 closed 15 empty 0
(sh) min 19 max 63 closed 31 empty 14
(sh) min 4 max 63 closed 0 empty 60
(spawn) min 3 max 63 closed 13 empty 48
(spawn) min 3 max 63 closed 13 empty 48
(mount) min 3 max 17 closed 15 empty 0
(mount) min 19 max 63 closed 32 empty 13

and so on.

While here use more idiomatic naming.

An avoidable relock is left in place to avoid uglifying the code.
The code was not switched to bitmap traversal for the same reason.

Tested with ltp kernel/syscalls/close_range

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/file.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

Comments

Christian Brauner Aug. 4, 2023, 4:49 p.m. UTC | #1 
On Thu, 27 Jul 2023 13:38:09 +0200, Mateusz Guzik wrote:
> Stock code takes a lock trip for every fd in range, but this can be
> trivially avoided and real-world consumers do have plenty of already
> closed cases.
> 
> Just booting Debian 12 with a debug printk shows:
> (sh) min 3 max 17 closed 15 empty 0
> (sh) min 19 max 63 closed 31 empty 14
> (sh) min 4 max 63 closed 0 empty 60
> (spawn) min 3 max 63 closed 13 empty 48
> (spawn) min 3 max 63 closed 13 empty 48
> (mount) min 3 max 17 closed 15 empty 0
> (mount) min 19 max 63 closed 32 empty 13
> 
> [...]

massaged it a bit

---

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[1/1] file: mostly eliminate spurious relocking in __range_close
      https://git.kernel.org/vfs/vfs/c/215baa741614
diff mbox series

Patch

diff --git a/fs/file.c b/fs/file.c
index 35c62b54c9d6..5483c052ce93 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -693,29 +693,25 @@  static inline void __range_cloexec(struct files_struct *cur_fds,
 	spin_unlock(&cur_fds->file_lock);
 }
 
-static inline void __range_close(struct files_struct *cur_fds, unsigned int fd,
+static inline void __range_close(struct files_struct *files, unsigned int fd,
 				 unsigned int max_fd)
 {
+	struct file *file;
 	unsigned n;
 
-	rcu_read_lock();
-	n = last_fd(files_fdtable(cur_fds));
-	rcu_read_unlock();
+	spin_lock(&files->file_lock);
+	n = last_fd(files_fdtable(files));
 	max_fd = min(max_fd, n);
-
-	while (fd <= max_fd) {
-		struct file *file;
-
-		spin_lock(&cur_fds->file_lock);
-		file = pick_file(cur_fds, fd++);
-		spin_unlock(&cur_fds->file_lock);
-
-		if (file) {
-			/* found a valid file to close */
-			filp_close(file, cur_fds);
+	for (; fd <= max_fd; fd++) {
+		file = pick_file(files, fd);
+		if (file != NULL) {
+			spin_unlock(&files->file_lock);
+			filp_close(file, files);
 			cond_resched();
+			spin_lock(&files->file_lock);
 		}
 	}
+	spin_unlock(&files->file_lock);
 }
 
 /**