linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christian Brauner <brauner@kernel.org>,
	Amir Goldstein <amir73il@gmail.com>,
	linux-fsdevel@vger.kernel.org
Subject: [PATCH] backing file: free directly
Date: Thu,  5 Oct 2023 19:08:35 +0200	[thread overview]
Message-ID: <20231005-sakralbau-wappnen-f5c31755ed70@brauner> (raw)

Backing files as used by overlayfs are never installed into file
descriptor tables and are explicitly documented as such. They aren't
subject to rcu access conditions like regular files are.

Their lifetime is bound to the lifetime of the overlayfs file, i.e.,
they're stashed in ovl_file->private_data and go away otherwise. If
they're set as vma->vm_file - which is their main purpose - then they're
subject to regular refcount rules and vma->vm_file can't be installed
into an fdtable after having been set. All in all I don't see any need
for rcu delay here. So free it directly.

This all hinges on such hybrid beasts to never actually be installed
into fdtables which - as mentioned before - is not allowed. So add an
explicit WARN_ON_ONCE() so we catch any case where someone is suddenly
trying to install one of those things into a file descriptor table so we
can have nice long chat with them.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Hey Linus,

I've put the following change on top of the rcu change. We really don't
need any rcu delayed freeing for backing files unless I'm missing
something. They should never ever appear in fdtables. So fd_install()
should yell if anyone tries to do that. I'm still off this week but
this bothered me.

Christian
---
 fs/file.c       | 3 +++
 fs/file_table.c | 9 +--------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 666514381159..2f6965848907 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -604,6 +604,9 @@ void fd_install(unsigned int fd, struct file *file)
 	struct files_struct *files = current->files;
 	struct fdtable *fdt;
 
+	if (WARN_ON_ONCE(unlikely(file->f_mode & FMODE_BACKING)))
+		return;
+
 	rcu_read_lock_sched();
 
 	if (unlikely(files->resize_in_progress)) {
diff --git a/fs/file_table.c b/fs/file_table.c
index a79a80031343..08fd1dd6d863 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -61,13 +61,6 @@ struct path *backing_file_real_path(struct file *f)
 }
 EXPORT_SYMBOL_GPL(backing_file_real_path);
 
-static void file_free_rcu(struct rcu_head *head)
-{
-	struct file *f = container_of(head, struct file, f_rcuhead);
-
-	kfree(backing_file(f));
-}
-
 static inline void file_free(struct file *f)
 {
 	security_file_free(f);
@@ -76,7 +69,7 @@ static inline void file_free(struct file *f)
 	put_cred(f->f_cred);
 	if (unlikely(f->f_mode & FMODE_BACKING)) {
 		path_put(backing_file_real_path(f));
-		call_rcu(&f->f_rcuhead, file_free_rcu);
+		kfree(backing_file(f));
 	} else {
 		kmem_cache_free(filp_cachep, f);
 	}
-- 
2.34.1


             reply	other threads:[~2023-10-05 17:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-05 17:08 Christian Brauner [this message]
2023-10-05 18:09 ` [PATCH] backing file: free directly Linus Torvalds

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=20231005-sakralbau-wappnen-f5c31755ed70@brauner \
    --to=brauner@kernel.org \
    --cc=amir73il@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --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).