linux-kernel - Re: [PATCH] fs: use wq_has_sleeper() in end_dir

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGudoHFOPkPGQzcuymrW17uf9NksfL2PLA_BDzfDezNoAyfnLQ@mail.gmail.com>
Date: Mon, 17 Mar 2025 00:27:09 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: brauner@...nel.org
Cc: viro@...iv.linux.org.uk, jack@...e.cz, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] fs: use wq_has_sleeper() in end_dir_add()

On Mon, Mar 17, 2025 at 12:24 AM Mateusz Guzik <mjguzik@...il.com> 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@...il.com>
> ---
>  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>