Re: [PATCH v4 2/7] VFS: use wait_var_event for waiting in d_alloc_parallel()

Re: [PATCH v4 2/7] VFS: use wait_var_event for waiting in d_alloc_parallel()

[Jeff Layton]

On Thu, 2026-04-30 at 11:56 +1000, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> d_alloc_parallel() currently requires a wait_queue_head to be passed in.
> This must have a life time which extends until the lookup is completed.
> 
> This makes it awkward to use and particularly make it hard to use in
> lookup_one_qstr_excl() which I hope to do in the future.
> 
> This patch changes d_alloc_parallel() to use wake_up_var_locked() to
> wake up waiters, and wait_var_event_spinlock() to wait.  dentry->d_lock
> is used for synchronisation as it is already held and the relevant
> times.
> 
> In most cases there will be no waiters so the wake_up_var_locked() call
> (which can walk an arbitrarily long list) would be a complete waste.  To
> optimise this a new ->d_flags flag is added: DCACHE_LOOKUP_WAITER.  This
> is set whenever any thread prepares to wait for the dentry, and if it
> isn't set when DCACHE_PAR_LOOKUP is cleared, no wakeup is sent.
> DCACHE_LOOKUP_WAITER is cleared after the wakeup is set.
> 
> __d_lookup_unhash() no longer returns a wq.  Callers check
> DCACHE_LOOKUP_WAITER to check if a wakeup is needed, and
> wake_up_var_locked() can find the wq.
> 
> __d_lookup_unhash() no longer needs to re-init ->d_lru.  That was
> previously shared (in a union) with ->d_wait but ->d_wait is now gone
> so it no longer corrupts ->d_lru.
> 
> Wakeup is move out of end_dir_add() as it is conceptually a separate action.
> 
> Co-developed-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  Documentation/filesystems/porting.rst |  6 ++
>  fs/afs/dir_silly.c                    |  4 +-
>  fs/dcache.c                           | 83 +++++++++++++++------------
>  fs/fuse/readdir.c                     |  3 +-
>  fs/namei.c                            |  6 +-
>  fs/nfs/dir.c                          |  6 +-
>  fs/nfs/unlink.c                       |  3 +-
>  fs/proc/base.c                        |  3 +-
>  fs/proc/proc_sysctl.c                 |  3 +-
>  fs/smb/client/readdir.c               |  3 +-
>  include/linux/dcache.h                | 11 ++--
>  include/linux/nfs_xdr.h               |  1 -
>  12 files changed, 67 insertions(+), 65 deletions(-)
> 
> 

[...]

>  
> @@ -2979,7 +2987,7 @@ static void __d_move(struct dentry *dentry, struct dentry *target,
>  	if (unlikely(d_in_lookup(target))) {
>  		dir = target->d_parent->d_inode;
>  		n = start_dir_add(dir);
> -		d_wait = __d_lookup_unhash(target);
> +		__d_lookup_unhash(target);
>  	}

In the old code, d_wait was associated with the target...

>  
>  	write_seqcount_begin(&dentry->d_seq);
> @@ -3018,9 +3026,10 @@ static void __d_move(struct dentry *dentry, struct dentry *target,
>  	write_seqcount_end(&target->d_seq);
>  	write_seqcount_end(&dentry->d_seq);
>  
> -	if (dir)
> -		end_dir_add(dir, n, d_wait);
> -
> +	if (dir) {
> +		end_dir_add(dir, n);
> +		__d_wake_in_lookup_waiters(dentry);
> +	}

...but here you are waking based on "dentry". Should that be:

		__d_wake_in_lookup_waiters(target)

?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v4 2/7] VFS: use wait_var_event for waiting in d_alloc_parallel()

[NeilBrown]

On Tue, 12 May 2026, Jeff Layton wrote:
> On Thu, 2026-04-30 at 11:56 +1000, NeilBrown wrote:
> > From: NeilBrown <neil@brown.name>
> > 
> > d_alloc_parallel() currently requires a wait_queue_head to be passed in.
> > This must have a life time which extends until the lookup is completed.
> > 
> > This makes it awkward to use and particularly make it hard to use in
> > lookup_one_qstr_excl() which I hope to do in the future.
> > 
> > This patch changes d_alloc_parallel() to use wake_up_var_locked() to
> > wake up waiters, and wait_var_event_spinlock() to wait.  dentry->d_lock
> > is used for synchronisation as it is already held and the relevant
> > times.
> > 
> > In most cases there will be no waiters so the wake_up_var_locked() call
> > (which can walk an arbitrarily long list) would be a complete waste.  To
> > optimise this a new ->d_flags flag is added: DCACHE_LOOKUP_WAITER.  This
> > is set whenever any thread prepares to wait for the dentry, and if it
> > isn't set when DCACHE_PAR_LOOKUP is cleared, no wakeup is sent.
> > DCACHE_LOOKUP_WAITER is cleared after the wakeup is set.
> > 
> > __d_lookup_unhash() no longer returns a wq.  Callers check
> > DCACHE_LOOKUP_WAITER to check if a wakeup is needed, and
> > wake_up_var_locked() can find the wq.
> > 
> > __d_lookup_unhash() no longer needs to re-init ->d_lru.  That was
> > previously shared (in a union) with ->d_wait but ->d_wait is now gone
> > so it no longer corrupts ->d_lru.
> > 
> > Wakeup is move out of end_dir_add() as it is conceptually a separate action.
> > 
> > Co-developed-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: NeilBrown <neil@brown.name>
> > ---
> >  Documentation/filesystems/porting.rst |  6 ++
> >  fs/afs/dir_silly.c                    |  4 +-
> >  fs/dcache.c                           | 83 +++++++++++++++------------
> >  fs/fuse/readdir.c                     |  3 +-
> >  fs/namei.c                            |  6 +-
> >  fs/nfs/dir.c                          |  6 +-
> >  fs/nfs/unlink.c                       |  3 +-
> >  fs/proc/base.c                        |  3 +-
> >  fs/proc/proc_sysctl.c                 |  3 +-
> >  fs/smb/client/readdir.c               |  3 +-
> >  include/linux/dcache.h                | 11 ++--
> >  include/linux/nfs_xdr.h               |  1 -
> >  12 files changed, 67 insertions(+), 65 deletions(-)
> > 
> > 
> 
> [...]
> 
> >  
> > @@ -2979,7 +2987,7 @@ static void __d_move(struct dentry *dentry, struct dentry *target,
> >  	if (unlikely(d_in_lookup(target))) {
> >  		dir = target->d_parent->d_inode;
> >  		n = start_dir_add(dir);
> > -		d_wait = __d_lookup_unhash(target);
> > +		__d_lookup_unhash(target);
> >  	}
> 
> In the old code, d_wait was associated with the target...
> 
> >  
> >  	write_seqcount_begin(&dentry->d_seq);
> > @@ -3018,9 +3026,10 @@ static void __d_move(struct dentry *dentry, struct dentry *target,
> >  	write_seqcount_end(&target->d_seq);
> >  	write_seqcount_end(&dentry->d_seq);
> >  
> > -	if (dir)
> > -		end_dir_add(dir, n, d_wait);
> > -
> > +	if (dir) {
> > +		end_dir_add(dir, n);
> > +		__d_wake_in_lookup_waiters(dentry);
> > +	}
> 
> ...but here you are waking based on "dentry". Should that be:
> 
> 		__d_wake_in_lookup_waiters(target)
> 
> ?

Good catch.  I needed a long test run and a hint from Al before I saw
that :-)
The version in vfs-next/work.dcache (c177d84a4476) has "target" as you
suggest.

Thanks,
NeilBrown