summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMateusz Guzik <mjguzik@gmail.com>2023-09-26 18:22:28 +0200
committerChristian Brauner <brauner@kernel.org>2023-10-19 11:02:48 +0200
commit93faf426e3cc000c95f1a5d3510b77ce99adac52 (patch)
treecad9d0928a1b11d56cb87c5c5c3b080974437cd5
parent6036c5f1317526890925576f0efcbc427a32a2ae (diff)
downloadlinux-93faf426e3cc000c95f1a5d3510b77ce99adac52.tar.gz
linux-93faf426e3cc000c95f1a5d3510b77ce99adac52.tar.bz2
linux-93faf426e3cc000c95f1a5d3510b77ce99adac52.zip
vfs: shave work on failed file open
Failed opens (mostly ENOENT) legitimately happen a lot, for example here are stats from stracing kernel build for few seconds (strace -fc make): % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ------------------ 0.76 0.076233 5 15040 3688 openat (this is tons of header files tried in different paths) In the common case of there being nothing to close (only the file object to free) there is a lot of overhead which can be avoided. This is most notably delegation of freeing to task_work, which comes with an enormous cost (see 021a160abf62 ("fs: use __fput_sync in close(2)" for an example). Benchmarked with will-it-scale with a custom testcase based on tests/open1.c, stuffed into tests/openneg.c: [snip] while (1) { int fd = open("/tmp/nonexistent", O_RDONLY); assert(fd == -1); (*iterations)++; } [/snip] Sapphire Rapids, openneg_processes -t 1 (ops/s): before: 1950013 after: 2914973 (+49%) file refcount is checked as a safety belt against buggy consumers with an atomic cmpxchg. Technically it is not necessary, but it happens to not be measurable due to several other atomics which immediately follow. Optmizing them away to make this atomic into a problem is left as an exercise for the reader. v2: - unexport fput_badopen and move to fs/internal.h - handle the refcount with cmpxchg, adjust commentary accordingly - tweak the commit message Signed-off-by: Mateusz Guzik <mjguzik@gmail.com> Link: https://lore.kernel.org/r/20230926162228.68666-1-mjguzik@gmail.com Signed-off-by: Christian Brauner <brauner@kernel.org>
-rw-r--r--fs/file_table.c12
-rw-r--r--fs/internal.h1
-rw-r--r--fs/namei.c5
3 files changed, 17 insertions, 1 deletions
diff --git a/fs/file_table.c b/fs/file_table.c
index ee21b3da9d08..e68e97d4f00a 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -82,6 +82,18 @@ static inline void file_free(struct file *f)
call_rcu(&f->f_rcuhead, file_free_rcu);
}
+void release_empty_file(struct file *f)
+{
+ WARN_ON_ONCE(f->f_mode & (FMODE_BACKING | FMODE_OPENED));
+ /* Uhm, we better find out who grabs references to an unopened file. */
+ WARN_ON_ONCE(atomic_long_cmpxchg(&f->f_count, 1, 0) != 1);
+ security_file_free(f);
+ put_cred(f->f_cred);
+ if (likely(!(f->f_mode & FMODE_NOACCOUNT)))
+ percpu_counter_dec(&nr_files);
+ kmem_cache_free(filp_cachep, f);
+}
+
/*
* Return the total number of open files in the system
*/
diff --git a/fs/internal.h b/fs/internal.h
index 8260c738980c..f08d8fe3ae5e 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -94,6 +94,7 @@ extern void chroot_fs_refs(const struct path *, const struct path *);
struct file *alloc_empty_file(int flags, const struct cred *cred);
struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred);
struct file *alloc_empty_backing_file(int flags, const struct cred *cred);
+void release_empty_file(struct file *f);
static inline void put_file_access(struct file *file)
{
diff --git a/fs/namei.c b/fs/namei.c
index c36ff6f8195a..127c868a8992 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3783,7 +3783,10 @@ static struct file *path_openat(struct nameidata *nd,
WARN_ON(1);
error = -EINVAL;
}
- fput(file);
+ if (unlikely(file->f_mode & FMODE_OPENED))
+ fput(file);
+ else
+ release_empty_file(file);
if (error == -EOPENSTALE) {
if (flags & LOOKUP_RCU)
error = -ECHILD;