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>

diff --git a/fs/file_table.c b/fs/file_table.c
index ee21b3da9d08121f4712f100cde62c5ea771faaf..e68e97d4f00a30c2ab93bb412a0e84534867a027 100644 (file)
--- 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 8260c738980cfe9b2f3ba4b3f061609bc77b7b35..f08d8fe3ae5e81d4252087d5553ae309a4136761 100644 (file)
--- 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 c36ff6f8195adfdb1911f8b12a7b1a59446364c4..127c868a8992c66a206b8949def7ed8c445fd7f8 100644 (file)
--- 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;