Skip to content

Commit f477af0

Browse files
Darrick J. Wongcmaiolino
authored andcommitted
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: [email protected] # v6.10 Fixes: 77ede5f ("xfs: walk directory parent pointers to determine backref count") Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Signed-off-by: Carlos Maiolino <[email protected]>
1 parent 3e7ec34 commit f477af0

File tree

1 file changed

+31
-3
lines changed

1 file changed

+31
-3
lines changed

fs/xfs/scrub/nlinks.c

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,36 @@ xchk_nlinks_collect_pptr(
376376
return error;
377377
}
378378

379+
static uint
380+
xchk_nlinks_ilock_dir(
381+
struct xfs_inode *ip)
382+
{
383+
uint lock_mode = XFS_ILOCK_SHARED;
384+
385+
/*
386+
* We're going to scan the directory entries, so we must be ready to
387+
* pull the data fork mappings into memory if they aren't already.
388+
*/
389+
if (xfs_need_iread_extents(&ip->i_df))
390+
lock_mode = XFS_ILOCK_EXCL;
391+
392+
/*
393+
* We're going to scan the parent pointers, so we must be ready to
394+
* pull the attr fork mappings into memory if they aren't already.
395+
*/
396+
if (xfs_has_parent(ip->i_mount) && xfs_inode_has_attr_fork(ip) &&
397+
xfs_need_iread_extents(&ip->i_af))
398+
lock_mode = XFS_ILOCK_EXCL;
399+
400+
/*
401+
* Take the IOLOCK so that other threads cannot start a directory
402+
* update while we're scanning.
403+
*/
404+
lock_mode |= XFS_IOLOCK_SHARED;
405+
xfs_ilock(ip, lock_mode);
406+
return lock_mode;
407+
}
408+
379409
/* Walk a directory to bump the observed link counts of the children. */
380410
STATIC int
381411
xchk_nlinks_collect_dir(
@@ -394,8 +424,7 @@ xchk_nlinks_collect_dir(
394424
return 0;
395425

396426
/* Prevent anyone from changing this directory while we walk it. */
397-
xfs_ilock(dp, XFS_IOLOCK_SHARED);
398-
lock_mode = xfs_ilock_data_map_shared(dp);
427+
lock_mode = xchk_nlinks_ilock_dir(dp);
399428

400429
/*
401430
* The dotdot entry of an unlinked directory still points to the last
@@ -452,7 +481,6 @@ xchk_nlinks_collect_dir(
452481
xchk_iscan_abort(&xnc->collect_iscan);
453482
out_unlock:
454483
xfs_iunlock(dp, lock_mode);
455-
xfs_iunlock(dp, XFS_IOLOCK_SHARED);
456484
return error;
457485
}
458486

0 commit comments

Comments
 (0)