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

[Amir Goldstein]

On Thu, Oct 1, 2026 at 12:16 AM Jori Koolstra <jkoolstra@xxxxxxxxx> wrote:
>
>
> > Op 30-09-2026 05:45 EDT schreef Amir Goldstein <amir73il@xxxxxxxxx>:
> >
> >
> > On Wed, Sep 30, 2026 at 12:15 AM NeilBrown <neilb@xxxxxxxxxxx> wrote:
> > >
...
> > > The problem with this approach is that open(.., O_CREAT|O_DIRECTORY)
> > > might create the directory, then return -EOPNOTSUPP.  This is weird and
> > > I'd rather it not be visible.
> > >
> > > 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.
> > >
> >
> > Joining late to this party so apologies in advance if my questions
> > have already been addressed.
> >
> > I agree with Neil's statement above, but IMO, the atomic_open() fs
> > match the description of "doesn't completely support the functionality."
> > Therefore, I think that rather than success if directory exists, they
> > should also return -EINVAL/-EOPNOTSUPP consistently (see below).
> >
>
> The issue with this is that if you want per fs atomic_open() opt-in (in
> contrast to either implementing all instances in one release or disabling
> all), you have the issue that your lookup now depends on the caching status
> of the directory dentry.
>
> If that dentry is in cache, and positive, d_lookup() earlier in lookup_open()
> makes it return early:
>
>         if (dentry->d_inode) {
>                 /* Cached positive dentry: will open in do_open(). */
>                 goto out;
>         }
>
> So you get your lookup. But if the same dentry is not in cache, now you
> suddenly get -EINVAL. I thought that behavior was more unwanted then
> what I eventually settled on, namely to strip the O_CREAT bit.
>

Maybe I am missing something, but I think you misunderstand me.
What I mean is - if directory inode has a ->atomic_open() op,
bail early with -EINVAL/-EOPNOTSUPP, because this is a network
filesystem that does not support atomic O_CREATE|O_DIRECTORY
and in most likelihood never will support it.

This gating criteria is not dependent on cache state,
which is what we wanted.

The justification of using ->d_revalidate() as another opt-out
is that existence of ->d_revalidate() means that the state known
to dcache is only semi-reliable, so making atomic create/open
promises is problematic (O_EXCL for example).

The problem is that some fs (overlayfs/ext4/f2fs) register
a mostly-noop  ->d_revalidate() so I proposed how to deal with those.

Thanks,
Amir.