Commit f477af0c authored by Darrick J. Wong's avatar Darrick J. Wong Committed by Carlos Maiolino
Browse files

xfs: fix locking in xchk_nlinks_collect_dir 



On a filesystem with parent pointers, xchk_nlinks_collect_dir walks both
the directory entries (data fork) and the parent pointers (attr fork) to
determine the correct link count.  Unfortunately I forgot to update the
lock mode logic to handle the case of a directory whose attr fork is in
btree format and has not yet been loaded *and* whose data fork doesn't
need loading.

This leads to a bunch of assertions from xfs/286 in xfs_iread_extents
because we only took ILOCK_SHARED, not ILOCK_EXCL.  You'd need the rare
happenstance of a directory with a large number of non-pptr extended
attributes set and enough memory pressure to cause the directory to be
evicted and partially reloaded from disk.

I /think/ this only started in 6.18-rc1 because I've started seeing OOM
errors with the maple tree slab using 70% of memory, and this didn't
happen in 6.17.  Yay dynamic systems!

Cc: stable@vger.kernel.org # v6.10
Fixes: 77ede5f4 ("xfs: walk directory parent pointers to determine backref count")
Signed-off-by: default avatarDarrick J. Wong <djwong@kernel.org>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarCarlos Maiolino <cem@kernel.org>
parent 3e7ec343

fs/xfs/scrub/nlinks.c

+31 −3
Original line number Diff line number Diff line
@@ -376,6 +376,36 @@ xchk_nlinks_collect_pptr( 
	return error;

}



static uint

xchk_nlinks_ilock_dir(

	struct xfs_inode	*ip)

{

	uint			lock_mode = XFS_ILOCK_SHARED;



	/*

	 * We're going to scan the directory entries, so we must be ready to

	 * pull the data fork mappings into memory if they aren't already.

	 */

	if (xfs_need_iread_extents(&ip->i_df))

		lock_mode = XFS_ILOCK_EXCL;



	/*

	 * We're going to scan the parent pointers, so we must be ready to

	 * pull the attr fork mappings into memory if they aren't already.

	 */

	if (xfs_has_parent(ip->i_mount) && xfs_inode_has_attr_fork(ip) &&

	    xfs_need_iread_extents(&ip->i_af))

		lock_mode = XFS_ILOCK_EXCL;



	/*

	 * Take the IOLOCK so that other threads cannot start a directory

	 * update while we're scanning.

	 */

	lock_mode |= XFS_IOLOCK_SHARED;

	xfs_ilock(ip, lock_mode);

	return lock_mode;

}



/* Walk a directory to bump the observed link counts of the children. */

STATIC int

xchk_nlinks_collect_dir(
@@ -394,8 +424,7 @@ xchk_nlinks_collect_dir( 
		return 0;



	/* Prevent anyone from changing this directory while we walk it. */

	xfs_ilock(dp, XFS_IOLOCK_SHARED);

	lock_mode = xfs_ilock_data_map_shared(dp);

	lock_mode = xchk_nlinks_ilock_dir(dp);



	/*

	 * The dotdot entry of an unlinked directory still points to the last
@@ -452,7 +481,6 @@ xchk_nlinks_collect_dir( 
	xchk_iscan_abort(&xnc->collect_iscan);

out_unlock:

	xfs_iunlock(dp, lock_mode);

	xfs_iunlock(dp, XFS_IOLOCK_SHARED);

	return error;

}