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

[Jori Koolstra]

> Op 01-10-2026 03:00 CEST schreef NeilBrown <neilb@xxxxxxxxxxx>:
> 
>  
> 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...
> 

Let me rephrase your point: if you do an O_CREAT without O_EXCL you still
don't know whether it's your file because it might have been there already.
So the fact that you immediately get an fd still does not tell you everything.
Right?

But, at least you'll know it's a directory and you still save a syscall.
On the other hand, maybe that just breeds incorrect use, and if needed the
O_EXCL-less variant can always be added, so we might as well force its use
for now. Then again, it might be confusing to do open(O_CREAT|O_DIRECTORY)
and get an -EINVAL or -EEXISTS, given current O_CREAT semantics for regular
files.

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

I don't have LLM access right now: are there any ->atomic_open() implementations
currently that do return negative dentries? Seems to be a bit strange, just like
with ->mkdir(). What would be the purpose?

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

Fair enough. I'll spin this idea up then, to add the lookup to kern/tracefs
->mkdir() and add that warning to vfs_mkdir(). And I'll CC those maintainers
on that version.

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

OK, if it isn't likely that any ->atomic_open() wants O_CREAT|O_DIRECTORY
support we can get rid of some of the complexity. Summarize:

1. use ->atomic_open(O_DIRECTORY) for lookup, no O_CREAT bit passed in.
2. if positive then done, otherwise (on -ENOENT) we call ->mkdir()
3. open that dentry in the regular way, WARN_ON negative dentry return

Best,
Jori.

PS. Maybe I asked already before (in case apologies), but will you be
at LPC?