mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jori Koolstra <jkoolstra@xs4all.nl>
To: NeilBrown <neil@brown.name>, NeilBrown <neilb@ownmail.net>
Cc: Christian Brauner <brauner@kernel.org>,
	Jeff Layton <jlayton@kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Aleksa Sarai <aleksa@amutable.com>,
	Amir Goldstein <amir73il@gmail.com>, Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 07/12] vfs: add O_CREAT|O_DIRECTORY to open*(2)
Date: Thu, 1 Oct 2026 01:25:30 +0200 (CEST)	[thread overview]
Message-ID: <1755972037.791960.1790810730535@kpc.webmail.kpnmail.nl> (raw)
In-Reply-To: <179080897361.37859.5844379463485936668@noble.neil.brown.name>


> Op 30-09-2026 18:56 EDT schreef NeilBrown <neilb@ownmail.net>:
> 
>  
> On Thu, 01 Oct 2026, Jori Koolstra wrote:
> > 
> > Err, *derp*, what a stupid suggestion of mine.
> > 
> > > Currently O_DIRECTORY|O_CREAT results in -EINVAL.  I would rather it
> > > remain a -EINVAL on any filesystem which doesn't completely support
> > > the functionality.
> > > 
> > 
> > I don't think that works for the reason I just wrote in my email to Amir:
> > it would make lookup dependent on the dentry cache. If it's in-cache, you get
> > your dir, otherwise suddenly -EINVAL.
> > 
> > > To do that we need some way to detect kernfs and tracefs.  I think
> > > the only way we can do that is to make some change to those two
> > > filesystems.
> > > Maybe a new  SB_I_ flag in sb->s_iflags would be ok in the short term.
> > > 
> > 
> > We can just implement atomic_open() for kernfs/tracefs, do a lookup there,
> > and if negative with O_CREAT return maybe -ENOENT (or really we need a new
> > error that says "the requested create could not be serviced," like -ENOCREATE,
> > or whatever). And if it is positive we do finish_no_open().
> > 
> > It's a bit of a hack because it does not really have anything to do with
> > atomicity, but it does short-circuit the mkdir call in lookup_open(). I guess
> > that would work. Maybe I am confused, but wasn't that what you proposed here
> > earlier?
> 
> Yes, it is what I proposed earlier.  But I think it would require more
> review and probably make it unrealistic to land this cycle.  But I'm not
> thinking it is unlikely to be ready this cycle any way.
> 

Not unlikely, so likely that is :)

> I'm now wondering if we should keep ->atomic_open out of the loop and
> always use ->mkdir to create a directory.
> Based on your justification you probably always want O_EXCL and I would
> be inclined to require that.
> 

I wanted it to work as regular O_CREAT, so no forced O_EXCL per se.

> So if the dentry is in-lookup we call ->atomic_open(O_DIRECTORY).  If
> that succeeds - good.  If it reports ENOENT or a negative dentry, then

Can ->atomic_open() return a negative dentry?

> we cal ->mkdir.  If that succeeds with a positive dentry, we call
> through to call ->open.
> If ->mkdir succeeds with a negative dentry - we have the problem of
> kernfs and tracefs.  I'm leaning towards fixing those to do the lookup.

Yes, I follow this. But we do need to ask their maintainers then why that
was chosen. Plus, we need to ban negative dentry returns also for future
fses, which may be limiting.

> 
> I don't think any filesystems *can* combine mkdir with open, so not
> using ->atomic_open for the mkdir doesn't actually lose anything.
> 

Yeah, I don't quite understand the specifics of this as I am not familiar
with NFS and the likes. I presume because of things like network traffic
->atomic_open() was needed as a single call into the underlying fs. But
I have no idea why or why not that would make sense for directories. If
we do it the way you suggest now, we can get rid of all the O_CREAT stripping

> NeilBrown

  reply	other threads:[~2026-09-30 23:26 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-13 18:50 [PATCH v6 00/12] " Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 01/12] fs/namei.c: use trailing_slashes() Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 02/12] vfs: prepare vfs_creat|mkdir_no_perm for reuse in lookup_open() Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 03/12] vfs: lookup_open(): move setting FMODE_CREATED down Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 04/12] vfs: move ->create check in lookup_open() to before try_break_deleg() Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 05/12] vfs: lookup_open(): use vfs_create_no_perm() Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 06/12] vfs: lookup_open(): lock the parent as I_MUTEX_PARENT Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 07/12] vfs: add O_CREAT|O_DIRECTORY to open*(2) Jori Koolstra
2026-09-18  8:17   ` Christian Brauner
2026-09-18 10:07     ` NeilBrown
2026-09-19  0:01       ` NeilBrown
2026-09-25 13:57         ` Christian Brauner
2026-09-25 21:26           ` NeilBrown
2026-09-25 22:53             ` Jori Koolstra
2026-09-29 11:54         ` Jori Koolstra
2026-09-29 22:15           ` NeilBrown
2026-09-30  9:45             ` Amir Goldstein
2026-09-30 22:16               ` Jori Koolstra
2026-10-01  9:29                 ` Amir Goldstein
2026-10-01 10:08                   ` NeilBrown
2026-10-01 11:29                     ` Amir Goldstein
2026-10-01 15:42                   ` Jori Koolstra
2026-10-01 15:59                     ` Amir Goldstein
2026-10-01 16:23                       ` Jori Koolstra
2026-10-01 17:53                         ` Amir Goldstein
2026-09-30 22:33             ` Jori Koolstra
2026-09-30 22:56               ` NeilBrown
2026-09-30 23:25                 ` Jori Koolstra [this message]
2026-10-01  1:00                   ` NeilBrown
2026-10-01 14:07                     ` Jori Koolstra
2026-09-25 23:13     ` Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 08/12] vfs: change ->create/->mkdir operations unavailable errno Jori Koolstra
2026-09-18  8:04   ` Christian Brauner
2026-09-25 22:55     ` Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 09/12] vfs: move O_IS_MKDIR check from lookup_open() into individual filesystems Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 10/12] vfs: refuse O_CREAT for directories through a dangling symlink Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 11/12] vfs: short-circuit MAY_WRITE access for O_DIRECTORY opens Jori Koolstra
2026-09-13 18:50 ` [PATCH v6 12/12] selftest: add tests for open*(O_CREAT|O_DIRECTORY) Jori Koolstra
2026-09-17 10:38 ` [PATCH v6 00/12] vfs: add O_CREAT|O_DIRECTORY to open*(2) Christian Brauner
2026-09-18  8:18   ` 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=1755972037.791960.1790810730535@kpc.webmail.kpnmail.nl \
    --to=jkoolstra@xs4all.nl \
    --cc=aleksa@amutable.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=neilb@ownmail.net \
    --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®