| Message ID | 20250316232421.1642758-1-mjguzik@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fs: use wq_has_sleeper() in end_dir_add() | expand |
On Mon, Mar 17, 2025 at 12:24 AM Mateusz Guzik <mjguzik@gmail.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@gmail.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 >
On Mon, 17 Mar 2025 00:24:21 +0100, 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): > > [...] Applied to the vfs-6.15.misc branch of the vfs/vfs.git tree. Patches in the vfs-6.15.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs-6.15.misc [1/1] fs: use wq_has_sleeper() in end_dir_add() https://git.kernel.org/vfs/vfs/c/f81ad90aa97e
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@gmail.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.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 >
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)
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@gmail.com> --- fs/dcache.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)