[PATCH] xfs: drain inodegc before quota teardown on mount failure

[PATCH] xfs: drain inodegc before quota teardown on mount failure

[Zhang Cen]

xfs_mountfs() starts background inodegc before mount completion. If a
later setup step fails, the out_agresv unwind can release the quota
inodes via xfs_qm_unmount_quotas() while m_qflags still leave dqattach
enabled, and the common failure path can destroy mp->m_quotainfo via
xfs_qm_unmount() before the first common inodegc drain.

That leaves two teardown windows for an inodegc worker that resumes
xfs_inactive(): dqget can still need qi_*quotaip after
xfs_qm_unmount_quotas(), and the quota cache lookup still dereferences
mp->m_quotainfo until xfs_qm_unmount() finishes. Normal unmount and the
quotacheck abort path already drain inodegc before those teardown
stages.

Fix this by draining inodegc twice on the mount failure path: once on
out_agresv before dropping AG reservations and calling
xfs_qm_unmount_quotas(), and again on the common teardown path before
xfs_qm_unmount(). Keep xfs_unmount_flush_inodes() in place afterwards so
that reclaim queued by quota, realtime, root, or metadir teardown still
runs after xfs_qm_unmount() releases quota inodes on partially
initialized mounts.

The buggy scenario involves two paths, with each column showing the order
within that path:

path A label: mount failure unwind      path B label: inodegc worker
1. xfs_mountfs() starts inodegc and     1. Recovery or eviction leaves an
   later jumps into out_agresv.            ordinary inode queued for
                                           background inactivation.
2. out_agresv used to reach             2. xfs_inodegc_worker() runs
   xfs_qm_unmount_quotas(), which          xfs_inactive() on that inode.
   releases qi_*quotaip while           3. xfs_inactive() calls
   m_qflags still allow dqattach.          xfs_qm_dqattach().
3. The common unwind later reached      4. dqattach follows m_qflags into
   xfs_qm_unmount(), which purged          dqget, which may need the quota
   dquots and destroyed                    inodes and mp->m_quotainfo being
   mp->m_quotainfo.                        torn down by the unwind.
4. The first common inodegc drain
   used to happen only after quota
   teardown had started.

Sanitizer validation reported:
general protection fault
Call trace:
  xfs_qm_dqget_cache_lookup() (?:?)
  trace_function() (kernel/trace/trace.h:806)
  xfs_qm_dqget_inode() (?:?)
  ring_buffer_unlock_commit() (kernel/trace/ring_buffer.c:4306)
  xfs_qm_dqattach_locked() (?:?)
  srso_alias_return_thunk() (arch/x86/include/asm/nospec-branch.h:375)
  xfs_qm_dqattach() (?:?)
  xfs_free_eofblocks() (?:?)
  xfs_inactive() (?:?)
  xfs_inodegc_worker() (?:?)
  process_one_work() (kernel/workqueue.c:3200)
  worker_thread() (?:?)
  kthread() (?:?)
  _raw_spin_unlock_irq() (kernel/locking/spinlock.c:204)
  ret_from_fork() (?:?)
  __switch_to() (?:?)
  ret_from_fork_asm() (?:?)

Fixes: ab23a7768739 ("xfs: per-cpu deferred inode inactivation queues")

Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>

---
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index b24195f570cd..e8a870482d1a 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1228,6 +1228,12 @@ xfs_mountfs(
 	return 0;
 
  out_agresv:
+	/*
+	 * Drain ordinary inodegc before we drop per-AG reservations or tear
+	 * down quota state.  Background inactivation can still attach dquots
+	 * here, and dqget can still need the quota inodes.
+	 */
+	xfs_inodegc_flush(mp);
 	xfs_fs_unreserve_ag_blocks(mp);
 	xfs_qm_unmount_quotas(mp);
 	if (xfs_has_zoned(mp))
@@ -1236,24 +1242,25 @@ xfs_mountfs(
 	xfs_rtunmount_inodes(mp);
  out_rele_rip:
 	xfs_irele(rip);
-	/* Clean out dquots that might be in memory after quotacheck. */
-	xfs_qm_unmount(mp);
  out_free_metadir:
 	if (mp->m_metadirip)
 		xfs_irele(mp->m_metadirip);
 
 	/*
-	 * Inactivate all inodes that might still be in memory after a log
-	 * intent recovery failure so that reclaim can free them.  Metadata
-	 * inodes and the root directory shouldn't need inactivation, but the
-	 * mount failed for some reason, so pull down all the state and flee.
+	 * Flush inodegc before we destroy quotainfo.  This is the first drain
+	 * for paths that bypass out_agresv, and it also catches any reclaim
+	 * queued by the quota, realtime, root, or metadir teardown above.
 	 */
 	xfs_inodegc_flush(mp);
 
+	/* Clean out dquots that might be in memory after quotacheck. */
+	xfs_qm_unmount(mp);
+
 	/*
 	 * Flush all inode reclamation work and flush the log.
 	 * We have to do this /after/ rtunmount and qm_unmount because those
-	 * two will have scheduled delayed reclaim for the rt/quota inodes.
+	 * will have scheduled delayed reclaim for the rt/quota/root/metadir
+	 * inodes.
 	 *
 	 * This is slightly different from the unmountfs call sequence
 	 * because we could be tearing down a partially set up mount.  In