Unverified Commit 93faf426 authored by Mateusz Guzik's avatar Mateusz Guzik Committed by Christian Brauner
Browse files

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 021a160a ("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: default avatarMateusz Guzik <mjguzik@gmail.com>
Link: https://lore.kernel.org/r/20230926162228.68666-1-mjguzik@gmail.com


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 6036c5f1

fs/file_table.c

+12 −0
Original line number Diff line number Diff line
@@ -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

 */

fs/internal.h

+1 −0
Original line number Diff line number Diff line
@@ -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)

{

fs/namei.c

+4 −1
Original line number Diff line number Diff line
@@ -3783,7 +3783,10 @@ static struct file *path_openat(struct nameidata *nd, 
		WARN_ON(1);

		error = -EINVAL;

	}

	if (unlikely(file->f_mode & FMODE_OPENED))

		fput(file);

	else

		release_empty_file(file);

	if (error == -EOPENSTALE) {

		if (flags & LOOKUP_RCU)

			error = -ECHILD;