Re: [PATCH v3 04/19] VFS: use wait_var_event for waiting in d_alloc_parallel()
From: NeilBrown
Date: Tue Apr 28 2026 - 19:35:34 EST
On Wed, 29 Apr 2026, Al Viro wrote:
> On Tue, Apr 28, 2026 at 09:18:39PM +1000, NeilBrown wrote:
> > > d_must_wait() conditional, though - ->d_flags and ->d_lock are in different
> > > cachelines and there's no need to dirty both every time we are called.
> > > IOW, have d_must_wait() do this:
> > > if (!d_in_lookup(dentry))
> > > return false;
> > > if (!(dentry->d_flags & DCACHE_LOCK_WAITER))
> > > dentry->d_flags |= DCACHE_LOCK_WAITER;
> > > return true;
> >
> > The only time that DCACHE_LOCK_WAITER will already be set is when there
> > are two (or more) waiters as well as the thread they are waiting on.
> > That means three (or more) threads all accessing the same name at the
> > same time. How often does that happen? Is the micro-optimisation worth
> > the small increase in code size?
>
> Depends upon the load, obviously - a bunch of threads hitting the same
> pathname at the same time... not impossible.
>
> More to the point, why not set DCACHE_LOCK_WAITER as soon as we grab ->d_lock
> there? Then waiting becomes simply "wait until !d_in_lookup()". Sure, we
> might end up setting DCACHE_LOCK_WAITER on a dentry that has just dropped
> DCACHE_PAR_LOOKUP - who cares?
>
> While we are at it, do we need to drop it when we clear PAR_LOOKUP? Because
> if we do not, the whole "what do we return from __d_lookup_unhash()" thing
> disappears - we simply pass the dentry to end_dir_add() and have it check
> ->d_flags & DCACHE_LOCK_WAITER to decide whether to bother with wakeup.
>
Yes, your are right.
I've been thinking of this mostly in the context of locking the dentry for
directory ops, for which lookup is just one special case.
In that context the dentry can be locked and unlocked multiple time so
we really want to clear DCACHE_LOCK_WAITERS on each unlock.
However in the current code it is only used for lookup which only
happens once on a given dentry so we never need to clear
DCACHE_LOCK_WAITERS.
On the basis that we shouldn't add complexity until we actually need it,
I'll rename DCACHE_LOCK_WAITERS to DCACHE_LOOKUP_WAITERS and never clear
it, as you suggest.
Thanks,
NeilBrown