Re: [PATCH] fs: use wq_has_sleeper() in end_dir_add()

[Mateusz Guzik]

On Mon, Mar 17, 2025 at 12:24 AM Mateusz Guzik <mjguzik@xxxxxxxxx> wrote:
>
> The routine is used a lot, while the wakeup almost never has anyone to
> deal with.
>
> wake_up_all() takes an irq-protected spinlock, wq_has_sleeper() "only"
> contains a full fence -- not free by any means, but still cheaper.
>
> Sample result tracing waiters using a custom probe during -j 20 kernel
> build (0 - no waiters, 1 - waiters):
>
> @[
>     wakeprobe+5
>     __wake_up_common+63
>     __wake_up+54
>     __d_add+234
>     d_splice_alias+146
>     ext4_lookup+439
>     path_openat+1746
>     do_filp_open+195
>     do_sys_openat2+153
>     __x64_sys_openat+86
>     do_syscall_64+82
>     entry_SYSCALL_64_after_hwframe+118
> ]:
> [0, 1)             13999 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
> [1, ...)               1 |                                                    |
>
> So that 14000 calls in total from this backtrace, where only one time
> had a waiter.

I noticed unusually weird engrish after sending.

how about this sentence instead:
> Only 1 call out of 14000 with this backtrace had waiters.


>
> Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
> ---
>  fs/dcache.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index df8833fe9986..bd5aa136153a 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2497,7 +2497,8 @@ static inline void end_dir_add(struct inode *dir, unsigned int n,
>  {
>         smp_store_release(&dir->i_dir_seq, n + 2);
>         preempt_enable_nested();
> -       wake_up_all(d_wait);
> +       if (wq_has_sleeper(d_wait))
> +               wake_up_all(d_wait);
>  }
>
>  static void d_wait_lookup(struct dentry *dentry)
> --
> 2.43.0
>


-- 
Mateusz Guzik <mjguzik gmail.com>