linux-kernel - Re: [PATCH] vfs: use RCU in ilookup

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] [day] [month] [year] [list]

Message-ID: <20240716135555.fywhj75tkw5ogujl@quack3>
Date: Tue, 16 Jul 2024 15:55:55 +0200
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,
	bharata@....com
Subject: Re: [PATCH] vfs: use RCU in ilookup

On Mon 15-07-24 09:13:24, Mateusz Guzik wrote:
> A soft lockup in ilookup was reported when stress-testing a 512-way
> system [1] (see [2] for full context) and it was verified that not
> taking the lock shifts issues back to mm.
> 
> [1] https://lore.kernel.org/linux-mm/56865e57-c250-44da-9713-cf1404595bcc@amd.com/
> [2] https://lore.kernel.org/linux-mm/d2841226-e27b-4d3d-a578-63587a3aa4f3@amd.com/
> 
> Signed-off-by: Mateusz Guzik <mjguzik@...il.com>

Looks good to me. Feel free to add:

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

								Honza

> ---
> 
> fwiw the originally sent patch to the reporter performs a lockless
> lookup first and falls back to the locked variant, but that was me
> playing overfly safe.
> 
> I would add tested-by but patches are not the same in the end.
> 
> This is the only spot which can get this fixup, everything else taking
> the lock is also using custom callbacks, so filesystems invoking such
> code will need to get patched up on case-by-case basis (but
> realistically they probably already can do RCU-only operation).
> 
>  fs/inode.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index f356fe2ec2b6..52ca063c552c 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1525,9 +1525,7 @@ struct inode *ilookup(struct super_block *sb, unsigned long ino)
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  	struct inode *inode;
>  again:
> -	spin_lock(&inode_hash_lock);
> -	inode = find_inode_fast(sb, head, ino, true);
> -	spin_unlock(&inode_hash_lock);
> +	inode = find_inode_fast(sb, head, ino, false);
>  
>  	if (inode) {
>  		if (IS_ERR(inode))
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR