linux-kernel - Re: [PATCH] fs: load the ->i_sb pointer once in inode_sb_list

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: <nb5g34ehva6wmjusa3tin3wbsr26gm6shvuxfspzkwpor6edxk@ncx2um24ipwq>
Date: Wed, 19 Mar 2025 17:11:25 +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: load the ->i_sb pointer once in
 inode_sb_list_{add,del}

On Wed 19-03-25 01:46:35, Mateusz Guzik wrote:
> While this may sound like a pedantic clean up, it does in fact impact
> code generation -- the patched add routine is slightly smaller.
> 
> Signed-off-by: Mateusz Guzik <mjguzik@...il.com>

I'm surprised it matters for the compiler but as Christian wrote, why not.
Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> ---
> 
> Below is disasm before/after. I did not want to pull this into the
> commit message because of the total length vs long term usefulness ratio.
> 
> can be moved up into the commit message no problem if someone insists on
> it:
> 
> (gdb) disassemble inode_sb_list_add
> before:
>  <+0>:     endbr64
>  <+4>:     call   0xffffffff8130e9b0 <__fentry__>
>  <+9>:     push   %rbx
>  <+10>:    mov    0x28(%rdi),%rax
>  <+14>:    mov    %rdi,%rbx
>  <+17>:    lea    0x540(%rax),%rdi
>  <+24>:    call   0xffffffff8225cf20 <_raw_spin_lock>
>  <+29>:    mov    0x28(%rbx),%rax
>  <+33>:    lea    0x110(%rbx),%rdx
>  <+40>:    mov    0x548(%rax),%rcx
>  <+47>:    mov    %rdx,0x8(%rcx)
>  <+51>:    mov    %rcx,0x110(%rbx)
>  <+58>:    lea    0x548(%rax),%rcx
>  <+65>:    mov    %rcx,0x118(%rbx)
>  <+72>:    mov    %rdx,0x548(%rax)
>  <+79>:    mov    0x28(%rbx),%rdi
>  <+83>:    pop    %rbx
>  <+84>:    add    $0x540,%rdi
>  <+91>:    jmp    0xffffffff8225d020 <_raw_spin_unlock>
> 
> after:
>  <+0>:     endbr64
>  <+4>:     call   0xffffffff8130e9b0 <__fentry__>
>  <+9>:     push   %r12
>  <+11>:    push   %rbp
>  <+12>:    push   %rbx
>  <+13>:    mov    0x28(%rdi),%rbp
>  <+17>:    mov    %rdi,%rbx
>  <+20>:    lea    0x540(%rbp),%r12
>  <+27>:    mov    %r12,%rdi
>  <+30>:    call   0xffffffff8225cf20 <_raw_spin_lock>
>  <+35>:    mov    0x548(%rbp),%rdx
>  <+42>:    lea    0x110(%rbx),%rax
>  <+49>:    mov    %r12,%rdi
>  <+52>:    mov    %rax,0x8(%rdx)
>  <+56>:    mov    %rdx,0x110(%rbx)
>  <+63>:    lea    0x548(%rbp),%rdx
>  <+70>:    mov    %rdx,0x118(%rbx)
>  <+77>:    mov    %rax,0x548(%rbp)
>  <+84>:    pop    %rbx
>  <+85>:    pop    %rbp
>  <+86>:    pop    %r12
>  <+88>:    jmp    0xffffffff8225d020 <_raw_spin_unlock>
> 
>  fs/inode.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 10121fc7b87e..e188bb1eb07a 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -623,18 +623,22 @@ static void inode_wait_for_lru_isolating(struct inode *inode)
>   */
>  void inode_sb_list_add(struct inode *inode)
>  {
> -	spin_lock(&inode->i_sb->s_inode_list_lock);
> -	list_add(&inode->i_sb_list, &inode->i_sb->s_inodes);
> -	spin_unlock(&inode->i_sb->s_inode_list_lock);
> +	struct super_block *sb = inode->i_sb;
> +
> +	spin_lock(&sb->s_inode_list_lock);
> +	list_add(&inode->i_sb_list, &sb->s_inodes);
> +	spin_unlock(&sb->s_inode_list_lock);
>  }
>  EXPORT_SYMBOL_GPL(inode_sb_list_add);
>  
>  static inline void inode_sb_list_del(struct inode *inode)
>  {
> +	struct super_block *sb = inode->i_sb;
> +
>  	if (!list_empty(&inode->i_sb_list)) {
> -		spin_lock(&inode->i_sb->s_inode_list_lock);
> +		spin_lock(&sb->s_inode_list_lock);
>  		list_del_init(&inode->i_sb_list);
> -		spin_unlock(&inode->i_sb->s_inode_list_lock);
> +		spin_unlock(&sb->s_inode_list_lock);
>  	}
>  }
>  
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR