Re: [PATCH v2 3/9] nfs: remove d_drop()/d_alloc_parallel() from nfs_atomic_open()

[NeilBrown]

On Thu, 01 Oct 2026, John Stoffel wrote:
> >>>>> "NeilBrown" == NeilBrown  <neilb@xxxxxxxxxxx> writes:
> 
> > From: NeilBrown <neil@xxxxxxxxxx>
> > It is important that two non-create NFS "open"s of a negative dentry
> > don't race.  They both have only a shared lock on i_rwsem and so could
> > run concurrently, but they might both try to call d_splice_alias() at
> > the same time which is confusing at best.
> 
> > nfs_atomic_open() currently avoids this by discarding the negative
> > dentry and creating a new one using d_alloc_parallel().  Only one thread
> > can successfully get the d_in_lookup() dentry, the other will wait for
> > the first to finish, and can use the result of that first lookup.
> 
> This paragraph is confusing, and I'm not sure which thread can use the
> result of the first lookup from this description.  

Unfortunately it isn't clear to me what it is that isn't clear to you
:-)

When O_CREAT is not requested, calls to ->atomic_open are protected only
by a shared lock on the parent so if the dcache contains a negative
dentry for a given name then two ->atomic_open calls on that name can
happen concurrently.  That can be a problem as they both might try to
instantiate the dentry with an inode, and the second to do that will
trigger a BUG.

This is fixed by discarding (d_drop()) the dentry and using
d_alloc_parallel() to allocate a new dentry.  d_alloc_parallel()
effects a lock.  Only one thread will get an in_lookup dentry.  The
other will block until the dentry has been instantiated, and will then
be given the non-in_lookup dentry.

The current code does all this in nfs_atomic_open().  The new code does
the d_drop and the d_alloc_parallel() in VFS code were I can manage the
locking rules more easily.

I thought that is what I said above - though more briefly of course.  If
you can point me more precisely to what is not clear in the original, I
can try to improve it.

Thanks,
NeilBrown