Re: [PATCH] xfs: Fix circular locking during xfs inode reclamation

[Dave Chinner <david@fromorbit.com>]

On Mon, Sep 30, 2024 at 11:44:06AM +0800, Yafang Shao wrote:
> I encountered the following error messages on our test servers:
> 
> [ 2553.303035] ======================================================
> [ 2553.303692] WARNING: possible circular locking dependency detected
> [ 2553.304363] 6.11.0+ #27 Not tainted
> [ 2553.304732] ------------------------------------------------------
> [ 2553.305398] python/129251 is trying to acquire lock:
> [ 2553.305940] ffff89b18582e318 (&xfs_nondir_ilock_class){++++}-{3:3}, at: xfs_ilock+0x70/0x190 [xfs]
> [ 2553.307066]
> but task is already holding lock:
> [ 2553.307682] ffffffffb4324de0 (fs_reclaim){+.+.}-{0:0}, at: __alloc_pages_slowpath.constprop.0+0x368/0xb10
> [ 2553.308670]
> which lock already depends on the new lock.

.....

> [ 2553.342664]  Possible unsafe locking scenario:
> 
> [ 2553.343621]        CPU0                    CPU1
> [ 2553.344300]        ----                    ----
> [ 2553.344957]   lock(fs_reclaim);
> [ 2553.345510]                                lock(&xfs_nondir_ilock_class);
> [ 2553.346326]                                lock(fs_reclaim);
> [ 2553.347015]   rlock(&xfs_nondir_ilock_class);
> [ 2553.347639]
>  *** DEADLOCK ***
> 
> The deadlock is as follows,
> 
>     CPU0                                  CPU1
>    ------                                ------
> 
>   alloc_anon_folio()
>     vma_alloc_folio(__GFP_FS)
>      fs_reclaim_acquire(__GFP_FS);
>        __fs_reclaim_acquire();
> 
>                                     xfs_attr_list()
>                                       xfs_ilock()
>                                       kmalloc(__GFP_FS);
>                                         __fs_reclaim_acquire();
> 
>        xfs_ilock

Yet another lockdep false positive. listxattr() is not in a
transaction context on a referenced inode, so GFP_KERNEL is correct.
The problem is lockdep has no clue that fs_reclaim context can only
lock unreferenced inodes, so we can actualy run GFP_KERNEL context
memory allocation with a locked, referenced inode safely.

We typically use __GFP_NOLOCKDEP on these sorts of allocations, but
the long term fix is to address the lockdep annotations to take
reclaim context into account. We can't do that until the realtime
inode subclasses are removed which will give use the spare lockdep
subclasses to add a reclaim context subclass. That is buried in the
middle of a much large rework:

https://lore.kernel.org/linux-xfs/172437087542.59588.13853236455832390956.stgit@frogsfrogsfrogs/

-Dave.


-- 
Dave Chinner
david@fromorbit.com