linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH DRAFT RFC UNTESTED 00/18] file: FD_PREPARE()
@ 2025-11-18 15:48 Christian Brauner
  2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 01/18] file: add FD_PREPARE() Christian Brauner
                   ` (17 more replies)
  0 siblings, 18 replies; 19+ messages in thread
From: Christian Brauner @ 2025-11-18 15:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Alexander Viro, Jan Kara, linux-fsdevel, Christian Brauner

Hey,

I've been playing with an idea to allow for some flexible usage of the
get_unused_fd_flags() + create file + fd_install() pattern that's used
quite extensively.

How callers allocate files is really heterogenous so it's not really
convenient to fold them into a single class. It's possibe to split them
into subclasses like for anon inodes. I think that's not necessarily
nice as well so my take is:

FD_PREPARE(fdprep, open_flag, file_open_handle(&path, open_flag));
if (fd_prepare_failed(fdprep))
	return fd_prepare_error(fdprep);

return fd_publish(fdprep);

or

FD_PREPARE(fdprep, 0, dentry_open(&path, hreq->oflags, cred));
dput(dentry);
if (fd_prepare_failed(fdprep))
        return fd_prepare_error(fdprep);

if (S_ISREG(inode->i_mode)) {
        struct file *filp = fd_prepare_file(fdprep);

        filp->f_flags |= O_NOATIME;
        filp->f_mode |= FMODE_NOCMTIME;
}

return fd_publish(fdprep);

That's somewhat akin to how we use wait_event() to allow for arbitrary
things to be used as a condition in it. Here we obviously expect the
function to return a struct file.

It's centered around struct fd_prepare { int fd; struct file *file; }.
FD_PREPARE() encapsulates all of allocation and cleanup logic and must
be followed by a call to fd_publish() which consumes the fd and file
otherwise they're deallocated.

It mandates a specific order for fd and file allocation which should be
fine I think.

We won't be able to convert everything to it but most cases can be made
to work with this pattern allowing to shed a bunch of cleanup code on
the way. To me it looks like it simplifies a bunch of code but it
obviously has assumptions, e.g., that it may freely consume the
reference by the file passed to it. I didn't see this as a big issue
though.

There's room for extending this in a way that you'd have subclasses for
some particularly often use patterns but as I said I'm not even sure
that's worth it.

I've converted a about 17 callers to see that they can be folded into
this pattern.

I'd like to gather early feedback. It's a draft, haven't compiled it or
tested it. Just played around with this today and thought I'd better get
yelled at early.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Christian Brauner (18):
      file: add FD_PREPARE()
      fs: anon_inodes
      fs: eventfd
      fs: fhandle
      fs: open_tree()
      fs: namespace
      fs: fanotify
      fs: nsfs
      fs: autofs
      fs: eventpoll
      fs: open_tree_attr()
      fs: nsfs2
      fs: open
      fs: signalfd64
      fs: timerfd
      fs: userfaultfd
      fs: xfs
      drivers: drm_lease

 drivers/gpu/drm/drm_lease.c        |  44 ++++--------
 fs/anon_inodes.c                   |  26 ++-----
 fs/autofs/dev-ioctl.c              |  32 +++------
 fs/eventfd.c                       |  32 ++++-----
 fs/eventpoll.c                     |  33 +++------
 fs/fhandle.c                       |  30 ++++----
 fs/namespace.c                     |  87 ++++++++---------------
 fs/notify/fanotify/fanotify_user.c |  19 ++---
 fs/nsfs.c                          |  48 +++++--------
 fs/open.c                          |  18 ++---
 fs/signalfd.c                      |  28 +++-----
 fs/timerfd.c                       |  28 +++-----
 fs/userfaultfd.c                   |  29 +++-----
 fs/xfs/xfs_handle.c                |  22 ++----
 include/linux/cleanup.h            |  22 ++++++
 include/linux/file.h               | 137 +++++++++++++++++++++++++++++++++++++
 16 files changed, 321 insertions(+), 314 deletions(-)
---
base-commit: a71e4f103aed69e7a11ea913312726bb194c76ee
change-id: 20251118-work-fd-prepare-f415a3bf5fda


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-11-18 15:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 15:48 [PATCH DRAFT RFC UNTESTED 00/18] file: FD_PREPARE() Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 01/18] file: add FD_PREPARE() Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 02/18] fs: anon_inodes Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 03/18] fs: eventfd Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 04/18] fs: fhandle Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 05/18] fs: open_tree() Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 06/18] fs: namespace Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 07/18] fs: fanotify Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 08/18] fs: nsfs Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 09/18] fs: autofs Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 10/18] fs: eventpoll Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 11/18] fs: open_tree_attr() Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 12/18] fs: nsfs2 Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 13/18] fs: open Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 14/18] fs: signalfd64 Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 15/18] fs: timerfd Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 16/18] fs: userfaultfd Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 17/18] fs: xfs Christian Brauner
2025-11-18 15:48 ` [PATCH DRAFT RFC UNTESTED 18/18] drivers: drm_lease Christian Brauner

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).