xfs: allow queued AG intents to drain before scrubbingscrub-drain-intents_2021-09-17

Currently, online scrub isn't sufficiently careful about quiescing
allocation groups before checking them.  While scrub does take the AG
header locks, it doesn't serialize against chains of AG update intents
that are being processed concurrently.  If there's a collision,
cross-referencing between data structures (e.g. rmapbt and refcountbt)
can yield false corruption events; if repair is running, this results in
incorrect repairs.

Fix this by adding to the perag structure the count of active intents
and make scrub wait until there aren't any to continue.  This is a
little stupid since transactions can queue intents without taking buffer
locks, but we'll also wait for those transactions.

XXX: should have instead a per-ag rwsem that gets taken as soon as the
AG[IF] are locked and stays held until the transaction commits or moves
on to the next AG?  would we rather have a six lock so that intents can
take an ix lock, and not have to upgrade to x until we actually want to
make changes to that ag?  is that how those even work??

Signed-off-by: Darrick J. Wong <djwong@kernel.org>

1 files changed, 48 insertions, 1 deletions

diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 2b2d09a95ed9..4eead6f75509 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -351,14 +351,30 @@ xfs_bmap_update_finish_item(
 	struct xfs_btree_cur		**state)
 {
 	struct xfs_bmap_intent		*bi;
+	struct xfs_mount		*mp = tp->t_mountp;
+	xfs_fsblock_t			orig_startblock;
 	int				error;
 
 	bi = container_of(item, struct xfs_bmap_intent, bi_list);
+	orig_startblock = bi->bi_bmap.br_startblock;
 	error = xfs_trans_log_finish_bmap_update(tp, BUD_ITEM(done), bi);
 	if (!error && bi->bi_bmap.br_blockcount > 0) {
 		ASSERT(bi->bi_type == XFS_BMAP_UNMAP);
 		return -EAGAIN;
 	}
+
+	/*
+	 * Drop our intent counter reference now that we've either queued a
+	 * deferred rmap intent or failed.  Be careful to use the original
+	 * startblock since the finishing functions can update the intent
+	 * state.
+	 */
+	if (xfs_has_rmapbt(mp)) {
+		bool rt = xfs_ifork_is_realtime(bi->bi_owner, bi->bi_whichfork);
+
+		xfs_fs_drop_intents(mp, rt, orig_startblock);
+	}
+
 	kmem_free(bi);
 	return error;
 }
@@ -371,17 +387,47 @@ xfs_bmap_update_abort_intent(
 	xfs_bui_release(BUI_ITEM(intent));
 }
 
-/* Cancel a deferred rmap update. */
+/* Cancel a deferred bmap update. */
 STATIC void
 xfs_bmap_update_cancel_item(
+	struct xfs_mount		*mp,
 	struct list_head		*item)
 {
 	struct xfs_bmap_intent		*bi;
 
 	bi = container_of(item, struct xfs_bmap_intent, bi_list);
+
+	/* Drop our intent counter reference since we're going away. */
+	if (xfs_has_rmapbt(mp)) {
+		bool rt = xfs_ifork_is_realtime(bi->bi_owner, bi->bi_whichfork);
+
+		xfs_fs_drop_intents(mp, rt, bi->bi_bmap.br_startblock);
+	}
+
 	kmem_free(bi);
 }
 
+/* Add a deferred bmap update. */
+STATIC void
+xfs_bmap_update_add_item(
+	struct xfs_mount		*mp,
+	const struct list_head		*item)
+{
+	const struct xfs_bmap_intent	*bi;
+
+	bi = container_of(item, struct xfs_bmap_intent, bi_list);
+
+	/*
+	 * Grab an intent counter reference on behalf of the deferred rmap
+	 * intent item that we will queue when we finish this bmap work.
+	 */
+	if (xfs_has_rmapbt(mp)) {
+		bool rt = xfs_ifork_is_realtime(bi->bi_owner, bi->bi_whichfork);
+
+		xfs_fs_bump_intents(mp, rt, bi->bi_bmap.br_startblock);
+	}
+}
+
 const struct xfs_defer_op_type xfs_bmap_update_defer_type = {
 	.max_items	= XFS_BUI_MAX_FAST_EXTENTS,
 	.create_intent	= xfs_bmap_update_create_intent,
@@ -389,6 +435,7 @@ const struct xfs_defer_op_type xfs_bmap_update_defer_type = {
 	.create_done	= xfs_bmap_update_create_done,
 	.finish_item	= xfs_bmap_update_finish_item,
 	.cancel_item	= xfs_bmap_update_cancel_item,
+	.add_item	= xfs_bmap_update_add_item,
 };
 
 /* Is this recovered BUI ok? */