[PATCH] vfs: use RCU in ilookup

[PATCH] vfs: use RCU in ilookup

[Mateusz Guzik]

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@gmail.com>
---

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

Re: [PATCH] vfs: use RCU in ilookup

[Bharata B Rao]

On 15-Jul-24 12:43 PM, 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/

Mateusz,

Just want to mention explicitly that in addition to the lockless lookup 
changes that you suggested in [1], the test run also included your other 
commit 
https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.inode.rcu&id=7180f8d91fcbf252de572d9ffacc945effed0060 
as you had suggested.

Regards,
Bharata.

Re: [PATCH] vfs: use RCU in ilookup

[Mateusz Guzik]

On Mon, Jul 15, 2024 at 10:02 AM Bharata B Rao <bharata@amd.com> wrote:
>
> On 15-Jul-24 12:43 PM, 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/
>
> Mateusz,
>
> Just want to mention explicitly that in addition to the lockless lookup
> changes that you suggested in [1], the test run also included your other
> commit
> https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.inode.rcu&id=7180f8d91fcbf252de572d9ffacc945effed0060
> as you had suggested.
>

That commit is needed to have this compile to begin with, so it was
kind of implied. :)

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [PATCH] vfs: use RCU in ilookup

[Christian Brauner]

On Mon, 15 Jul 2024 09:13:24 +0200, 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/
> 
> [...]

Applied to the vfs.inode.rcu branch of the vfs/vfs.git tree.
Patches in the vfs.inode.rcu 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.inode.rcu

[1/1] vfs: use RCU in ilookup
      https://git.kernel.org/vfs/vfs/c/2eae762dece6

Re: [PATCH] vfs: use RCU in ilookup

[Jan Kara]

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@gmail.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.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@suse.com>
SUSE Labs, CR