diff options
author | Mateusz Guzik <mjguzik@gmail.com> | 2023-09-26 18:22:28 +0200 |
---|---|---|
committer | Christian Brauner <brauner@kernel.org> | 2023-10-19 11:02:48 +0200 |
commit | 93faf426e3cc000c95f1a5d3510b77ce99adac52 (patch) | |
tree | cad9d0928a1b11d56cb87c5c5c3b080974437cd5 /fs | |
parent | fs: simplify misleading code to remove ambiguity regarding ihold()/iput() (diff) | |
download | linux-93faf426e3cc000c95f1a5d3510b77ce99adac52.tar.xz 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>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/file_table.c | 12 | ||||
-rw-r--r-- | fs/internal.h | 1 | ||||
-rw-r--r-- | fs/namei.c | 5 |
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; |