summaryrefslogtreecommitdiffstats
path: root/fs/xfs/scrub/parent.c
diff options
context:
space:
mode:
authorDarrick J. Wong <djwong@kernel.org>2023-04-12 04:00:20 +0200
committerDarrick J. Wong <djwong@kernel.org>2023-04-12 04:00:20 +0200
commita03297a0ca9f21800c9b88028a3722715b2eb5ba (patch)
treebd47c2e91bfbe945a4df9592a0b9d69ba188ab40 /fs/xfs/scrub/parent.c
parentxfs: fix parent pointer scrub racing with subdirectory reparenting (diff)
downloadlinux-a03297a0ca9f21800c9b88028a3722715b2eb5ba.tar.xz
linux-a03297a0ca9f21800c9b88028a3722715b2eb5ba.zip
xfs: manage inode DONTCACHE status at irele time
Right now, there are statements scattered all over the online fsck codebase about how we can't use XFS_IGET_DONTCACHE because of concerns about scrub's unusual practice of releasing inodes with transactions held. However, iget is the wrong place to handle this -- the DONTCACHE state doesn't matter at all until we try to *release* the inode, and here we get things wrong in multiple ways: First, if we /do/ have a transaction, we must NOT drop the inode, because the inode could have dirty pages, dropping the inode will trigger writeback, and writeback can trigger a nested transaction. Second, if the inode already had an active reference and the DONTCACHE flag set, the icache hit when scrub grabs another ref will not clear DONTCACHE. This is sort of by design, since DONTCACHE is now used to initiate cache drops so that sysadmins can change a file's access mode between pagecache and DAX. Third, if we do actually have the last active reference to the inode, we can set DONTCACHE to avoid polluting the cache. This is the /one/ case where we actually want that flag. Create an xchk_irele helper to encode all that logic and switch the online fsck code to use it. Since this now means that nearly all scrubbers use the same xfs_iget flags, we can wrap them too. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Dave Chinner <dchinner@redhat.com>
Diffstat (limited to 'fs/xfs/scrub/parent.c')
-rw-r--r--fs/xfs/scrub/parent.c13
1 files changed, 4 insertions, 9 deletions
diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
index b6c8f6dccc8f..58d5dfb7ea21 100644
--- a/fs/xfs/scrub/parent.c
+++ b/fs/xfs/scrub/parent.c
@@ -127,20 +127,15 @@ xchk_parent_validate(
expected_nlink = VFS_I(sc->ip)->i_nlink == 0 ? 0 : 1;
/*
- * Grab this parent inode. We release the inode before we
- * cancel the scrub transaction. Since we're don't know a
- * priori that releasing the inode won't trigger eofblocks
- * cleanup (which allocates what would be a nested transaction)
- * if the parent pointer erroneously points to a file, we
- * can't use DONTCACHE here because DONTCACHE inodes can trigger
- * immediate inactive cleanup of the inode.
+ * Grab the parent directory inode. This must be released before we
+ * cancel the scrub transaction.
*
* If _iget returns -EINVAL or -ENOENT then the parent inode number is
* garbage and the directory is corrupt. If the _iget returns
* -EFSCORRUPTED or -EFSBADCRC then the parent is corrupt which is a
* cross referencing error. Any other error is an operational error.
*/
- error = xfs_iget(mp, sc->tp, parent_ino, XFS_IGET_UNTRUSTED, 0, &dp);
+ error = xchk_iget(sc, parent_ino, &dp);
if (error == -EINVAL || error == -ENOENT) {
error = -EFSCORRUPTED;
xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error);
@@ -176,7 +171,7 @@ xchk_parent_validate(
out_unlock:
xfs_iunlock(dp, lock_mode);
out_rele:
- xfs_irele(dp);
+ xchk_irele(sc, dp);
return error;
}