linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: linux-fsdevel@vger.kernel.org
Cc: Mateusz Guzik <mjguzik@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Christian Brauner <brauner@kernel.org>
Subject: [RFC][PATCH] simpler way to get benefits of "vfs: shave work on failed file open"
Date: Sun, 26 Nov 2023 02:08:34 +0000	[thread overview]
Message-ID: <20231126020834.GC38156@ZenIV> (raw)

IMO 93faf426e3cc "vfs: shave work on failed file open" had gone overboard -
avoiding an RCU delay in that particular case is fine, but it's done on
the wrong level.  A file that has never gotten FMODE_OPENED will never
have RCU-accessed references, its final fput() is equivalent to file_free()
and if it doesn't have FMODE_BACKING either, it can be done from any context
and won't need task_work treatment.

However, all of that can be achieved easier - all it takes is fput()
recognizing that case and calling file_free() directly.
No need to introduce a special primitive for that - and things like
failing dentry_open() could benefit from that as well.

Objections?

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/file_table.c b/fs/file_table.c
index de4a2915bfd4..7bcfa169dd45 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -75,18 +75,6 @@ static inline void file_free(struct file *f)
 	}
 }
 
-void release_empty_file(struct file *f)
-{
-	WARN_ON_ONCE(f->f_mode & (FMODE_BACKING | FMODE_OPENED));
-	if (atomic_long_dec_and_test(&f->f_count)) {
-		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
  */
@@ -445,6 +433,10 @@ void fput(struct file *file)
 	if (atomic_long_dec_and_test(&file->f_count)) {
 		struct task_struct *task = current;
 
+		if (unlikely(!(f->f_mode & (FMODE_BACKING | FMODE_OPENED)))) {
+			file_free(f);
+			return;
+		}
 		if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) {
 			init_task_work(&file->f_rcuhead, ____fput);
 			if (!task_work_add(task, &file->f_rcuhead, TWA_RESUME))
diff --git a/fs/internal.h b/fs/internal.h
index 58e43341aebf..273e6fd40d1b 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -94,7 +94,6 @@ 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 file_put_write_access(struct file *file)
 {
diff --git a/fs/namei.c b/fs/namei.c
index 22915c40e2bd..e7f641d3115f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3787,10 +3787,7 @@ 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);
+	fput(file);
 	if (error == -EOPENSTALE) {
 		if (flags & LOOKUP_RCU)
 			error = -ECHILD;

             reply	other threads:[~2023-11-26  2:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-26  2:08 Al Viro [this message]
2023-11-26  4:59 ` [RFC][PATCH] simpler way to get benefits of "vfs: shave work on failed file open" Linus Torvalds
2023-11-26  5:08   ` Al Viro
2023-11-26  5:17     ` Linus Torvalds
2023-11-26  9:21       ` Christian Brauner
2023-11-26 10:58         ` Mateusz Guzik
2023-11-27 15:03           ` Christian Brauner
2023-11-26  9:31 ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231126020834.GC38156@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).