Re: [PATCH v6 07/12] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[NeilBrown]

On Thu, 01 Oct 2026, Jori Koolstra wrote:
> > Op 30-09-2026 18:56 EDT schreef NeilBrown <neilb@xxxxxxxxxxx>:
> > 
> >  
> > 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.

Why?  The whole point is atomicity, and without O_EXCL there is no
atomicity.
I guess it doesn't hurt to not require O_EXCL, but it is just another
combination to test...

> 
> > 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?

No reason why not.  It is mapped to  -ENOENT

			if (unlikely(d_is_negative(dentry)))
				error = -ENOENT;

I'm in favour of ->atomic_open() calling finish_no_open() is all
no-error cases where it didn't do something about opening the file.
But I don't object to an explicit -ENOENT return.

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

Odds a good that there wasn' a specific reason why leaving the dentry
negative was chosen.  I think the better question is "do you have a
problem with this patch" where the patch does a lookup.

Putting a WARN_ON(d_really_is_negative()) somewhere in vfs_mkdir would be
easy enough. I cannot imagine it really being a burden for any fs.  If
that does happen were can solve the problem then.  Being prepared of all
possible eventuality is a no-win proposition.  I've tried - I gave up.

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

->atomic_open was created specifically for NFSv4 which has a combined
lookup/create/open request.  You can do everything with a combination of
->lookup and (exclusive) ->create (providing create returns a
non-negative dentry) and ->truncate and open.  Using ->atomic_open means
fewer round-trips to the server so less latency.

NFS doesn't have a concept of "open" for directories.  CIFS probably
does.  FUSE probably doesn't but could possibly add one.
But directories are not opened nearly as often as files, so combining
things isn't so important.  And O_TRUNC is not meaningful for
directories so that is one fewer thing that could be combined.

NeilBrown