[lvc-project] [PATCH v5 1/2] xfs: skip inode inactivation on a shut down mount

[Mikhail Lobanov]

XFS already declines to inactivate inodes on a shut down mount, but only
at queue time: xfs_inode_mark_reclaimable() calls
xfs_inode_needs_inactive(), which returns false when the mount is shut
down ("If the log isn't running, push inodes straight to reclaim"), and
then drops the dquots and marks the inode reclaimable directly.

An inode that was queued for background inactivation while the mount was
still live is not covered by that check: the inodegc worker still calls
xfs_inactive() on it even after the mount has been shut down in the
meantime.  Inactivation modifies persistent metadata and runs
transactions that cannot complete on a shut down mount, and it relies on
subsystems (e.g. quota) that a torn down, or never fully set up, mount
may not have available.

Honour the same invariant in xfs_inactive() itself: if the mount is shut
down, return early before doing any inactivation work.  The dquots
attached to the inode are released by the existing xfs_qm_dqdetach() at
the out: label, so references are not leaked, and the caller then makes
the inode reclaimable exactly as before.

On its own this is a consistency fix with the existing queue-time
behaviour; it is also a prerequisite for shutting the mount down in the
xfs_mountfs() failure path in the following patch.

Fixes: ab23a7768739 ("xfs: per-cpu deferred inode inactivation queues")
Signed-off-by: Mikhail Lobanov <m.lobanov at rosa.ru>
---
v5: per Christoph's review of v4, move the shutdown check into
    xfs_inactive() itself - an early "goto out" after initialising mp at
    declaration - instead of guarding the call in
    xfs_inodegc_inactivate().  The attached dquots are then released by
    the existing xfs_qm_dqdetach() at the out: label, which also removes
    the empty else branch that v4 had to brace to silence -Wempty-body
    without CONFIG_XFS_QUOTA.  Christoph's other comment - a potential
    NULL deref in xfs_qm_dqdetach() when the quota subsystem is gone - is
    discussed separately in-thread; I could not find a path that reaches
    it in the current tree, so it is not folded into this series.

v4: brace both branches of the if/else.  Without CONFIG_XFS_QUOTA
    xfs_qm_dqdetach() expands to nothing, leaving the else with an empty
    body, which trips -Wempty-body on a W=1 build (i386-allnoconfig,
    reported by the kernel test robot).

v3: split out of the v2 single patch as a prep patch, and additionally
    drop the attached dquots in the shutdown branch to avoid leaking
    references when an inode queued while the mount was live is processed
    after a (normal) shutdown - spotted in review of v2.

v2: https://lore.kernel.org/linux-xfs/aiKA7vVQ_RxT_YOr@infradead.org/T/#t
v1: https://lore.kernel.org/linux-xfs/ah6BIsvEitNW5Edb@infradead.org/

 fs/xfs/xfs_inode.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index beaa26ec62da..658da1e5fd42 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1386,7 +1386,7 @@ int
 xfs_inactive(
 	xfs_inode_t	*ip)
 {
-	struct xfs_mount	*mp;
+	struct xfs_mount	*mp = ip->i_mount;
 	int			error = 0;
 	int			truncate = 0;
 
@@ -1399,7 +1399,20 @@ xfs_inactive(
 		goto out;
 	}
 
-	mp = ip->i_mount;
+	/*
+	 * If the filesystem has been shut down - for example a mount that
+	 * failed after background inactivation was enabled - do not
+	 * inactivate the inode.  Inactivation modifies persistent metadata,
+	 * its transactions cannot complete on a shut down mount, and the
+	 * subsystems it relies on (e.g. quota, mp->m_quotainfo) may not be
+	 * set up.  The attached dquots are dropped at the out: label and the
+	 * inode then goes straight to reclaim, the same way
+	 * xfs_inode_needs_inactive() already declines to inactivate on a shut
+	 * down mount at queue time.
+	 */
+	if (xfs_is_shutdown(mp))
+		goto out;
+
 	ASSERT(!xfs_iflags_test(ip, XFS_IRECOVERY));
 
 	xfs_inactive_health(ip);
--
2.43.0