mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jann Horn <jannh@google.com>, Mateusz Guzik <mjguzik@gmail.com>,
	viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2] vfs: shave work on failed file open
Date: Fri, 29 Sep 2023 11:20:48 +0200	[thread overview]
Message-ID: <20230929-kerzen-fachjargon-ca17177e9eeb@brauner> (raw)
In-Reply-To: <CAHk-=wj-5ahmODDWDBVL81wSG-12qPYEw=o-iEo8uzY0HBGGRQ@mail.gmail.com>

> But yes, that protection would be broken by SLAB_TYPESAFE_BY_RCU,
> since then the "f_count is zero" is no longer a final thing.

I've tried coming up with a patch that is simple enough so the pattern
is easy to follow and then converting all places to rely on a pattern
that combine lookup_fd_rcu() or similar with get_file_rcu(). The obvious
thing is that we'll force a few places to now always acquire a reference
when they don't really need one right now and that already may cause
performance issues.

We also can't fully get rid of plain get_file_rcu() uses itself because
of users such as mm->exe_file. They don't go from one of the rcu fdtable
lookup helpers to the struct file obviously. They rcu replace the file
pointer in their struct ofc so we could change get_file_rcu() to take a
struct file __rcu **f and then comparing that the passed in pointer
hasn't changed before we managed to do atomic_long_inc_not_zero(). Which
afaict should work for such cases.

But overall we would introduce a fairly big and at the same time subtle
semantic change. The idea is pretty neat and it was fun to do but I'm
just not convinced we should do it given how ubiquitous struct file is
used and now to make the semanics even more special by allowing
refcounts.

I've kept your original release_empty_file() proposal in vfs.misc which
I think is a really nice change.

Let me know if you all passionately disagree. ;)

  reply	other threads:[~2023-09-29  9:21 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-26 16:22 Mateusz Guzik
2023-09-26 19:00 ` Linus Torvalds
2023-09-26 19:28   ` Mateusz Guzik
2023-09-27 14:09     ` Christian Brauner
2023-09-27 14:34       ` Mateusz Guzik
2023-09-27 17:48       ` Linus Torvalds
2023-09-27 17:56         ` Mateusz Guzik
2023-09-27 18:05           ` Linus Torvalds
2023-09-27 18:32             ` Mateusz Guzik
2023-09-27 20:27               ` Linus Torvalds
2023-09-27 21:06                 ` Mateusz Guzik
2023-09-27 21:18                   ` Linus Torvalds
2023-09-27 21:30                     ` Mateusz Guzik
2023-09-28 13:25                 ` Christian Brauner
2023-09-28 14:05                   ` Christian Brauner
2023-09-28 14:43                     ` Jann Horn
2023-09-28 17:21                       ` Linus Torvalds
2023-09-29  9:20                         ` Christian Brauner [this message]
2023-09-29 13:31                           ` Jann Horn
2023-09-29 19:57                             ` Christian Brauner
2023-09-29 21:23                               ` Mateusz Guzik
2023-09-29 21:39                                 ` Mateusz Guzik
2023-09-29 23:57                                   ` Linus Torvalds
2023-09-30  9:04                                     ` Christian Brauner
2023-10-03 16:45                                       ` Nathan Chancellor
2023-10-10  3:06                                       ` Al Viro
2023-10-10  8:29                                         ` Christian Brauner
2023-09-29 22:24                                 ` Matthew Wilcox
2023-09-29 23:02                                   ` Jann Horn

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=20230929-kerzen-fachjargon-ca17177e9eeb@brauner \
    --to=brauner@kernel.org \
    --cc=jannh@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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

all inboxes | Powered by JetHome®