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

[John Stoffel]

>>>>> "NeilBrown" == NeilBrown  <neilb@xxxxxxxxxxx> writes:

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

Sorry!  I wasn't clear, probably because I'm not great at the VFS
stuff and was just reacting more to my parsing of the language.  It's
probably not a big deal honestly.  

> 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