Re: [PATCH 3/3] xfs: don't let background reclaim forget sick inodes
From: Dave Chinner <david@fromorbit.com>
Date: 2021-06-03 04:43:02
On Wed, Jun 02, 2021 at 08:12:52PM -0700, Darrick J. Wong wrote:
quoted hunk ↗ jump to hunk
From: Darrick J. Wong <djwong@kernel.org> It's important that the filesystem retain its memory of sick inodes for a little while after problems are found so that reports can be collected about what was wrong. Don't let background inode reclamation free sick inodes unless we're under memory pressure. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/xfs_icache.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index 0e2b6c05e604..54285d1ad574 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c@@ -911,7 +911,8 @@ xfs_dqrele_all_inodes( */ static bool xfs_reclaim_igrab( - struct xfs_inode *ip) + struct xfs_inode *ip, + struct xfs_eofblocks *eofb) { ASSERT(rcu_read_lock_held());@@ -922,6 +923,17 @@ xfs_reclaim_igrab( spin_unlock(&ip->i_flags_lock); return false; } + + /* + * Don't reclaim a sick inode unless we're under memory pressure or the + * filesystem is unmounting. + */ + if (ip->i_sick && eofb == NULL && + !(ip->i_mount->m_flags & XFS_MOUNT_UNMOUNTING)) { + spin_unlock(&ip->i_flags_lock); + return false; + }
Using the "eofb == NULL" as a proxy for being under memory pressure is ... a bit obtuse. If we've got a handful of sick inodes, then there is no problem with just leaving the in memory regardless of memory pressure. If we've got lots of sick inodes, we're likely to end up in a shutdown state or be unmounted for checking real soon. I'd just leave sick inodes around until unmount or shutdown occurs; lots of sick inodes means repair is necessary right now, so shutdown+unmount is the right solution here, not memory reclaim.... Cheers, Dave. -- Dave Chinner david@fromorbit.com