Re: VFS: change ->atomic_open() calling to always have exclusive access

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Sep 15, 2025 at 01:01:06PM +1000, NeilBrown wrote:
> ->atomic_open() is called with only a shared lock on the directory when
> O_CREAT wasn't requested.  If the dentry is negative it is still called
> because the filesystem might want to revalidate-and-open in an atomic
> operations.  NFS does this to fullfil close-to-open consistency
> requirements.
> 
> NFS has complex code to drop the dentry and reallocate with
> d_alloc_parallel() in this case to ensure that only one lookup/open
> happens at a time.  It would be simpler to have NFS return zero from
> ->d_revalidate in this case so that d_alloc_parallel() will be calling
> in lookup_open() and NFS wan't need to worry about concurrency.
> 
> So this series makes that change to NFS to simplify atomic_open and remove
> the d_drop() and d_alloc_parallel(), and then changes lookup_open() so that
> atomic_open() can never be called without exclusive access to the dentry.

What will happen if you have a stale negative dentry of /mnt/foo/bar, with
/mnt/foo not writable to you (e.g. because /mnt is read-only mount) and
foo/bar having actually been created by another client since your old lookup
had happened?  What's more, assume that this file is owned by you and has 0660
for mode.

Calling open("/mnt/foo/bar", O_CREAT|O_RDWR, 0666) should succeed - the file
in question exists, so O_CREAT and the third argument of open(2) should be
ignored.

What happens is that we get ->d_revalidate() still with effects of O_CREAT
(i.e. LOOKUP_CREATE|LOOKUP_OPEN), but ->atomic_open() does *not* get O_CREAT.

Your series is broken in that case, AFAICS.