[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <apadfckxgxx46eten4sftyiay5nnbuopnph5oagnch6lyrtd3r@cgpwxge6bzs3>
Date: Mon, 17 Mar 2025 18:03:04 +0100
From: Jan Kara <jack@...e.cz>
To: Mateusz Guzik <mjguzik@...il.com>
Cc: brauner@...nel.org, 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 17-03-25 00:24:21, Mateusz Guzik 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.
>
> Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> 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
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists