diff mbox series

[1/6] xfs_repair: checking rt free space metadata must happen during phase 4

Message ID 172983774454.3041643.14059149217223950457.stgit@frogsfrogsfrogs (mailing list archive)
State New
Headers show
Series [1/6] xfs_repair: checking rt free space metadata must happen during phase 4 | expand

Commit Message

Darrick J. Wong Oct. 25, 2024, 6:36 a.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

Back in the really old days, xfs_repair would generate the new free
space information for the realtime section during phase 5, and write the
contents to the rtbitmap and summary files during phase 6.  This was ok
because the incore information isn't used until phase 6.

Then I changed the behavior to check the generated information against
what was on disk and complain about the discrepancies.  Unfortunately,
there was a subtle flaw here -- for a non -n run, we'll have regenerated
the AG metadata before we actually check the rt free space information.
If the AG btree regeneration should clobber one of the old rtbitmap or
summary blocks, this will be reported as a corruption even though
nothing's wrong.

Move check_rtmetadata to the end of phase 4 so that this doesn't happen.

Cc: <linux-xfs@vger.kernel.org> # v5.19.0
Fixes: f2e388616d7491 ("xfs_repair: check free rt extent count")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 repair/phase4.c     |    7 +++++++
 repair/phase5.c     |    6 ------
 repair/xfs_repair.c |    3 ---
 3 files changed, 7 insertions(+), 9 deletions(-)

Comments

Christoph Hellwig Oct. 28, 2024, 8:43 a.m. UTC | #1
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/repair/phase4.c b/repair/phase4.c
index 5e5d8c3c7d9b96..071f20ed736e4b 100644
--- a/repair/phase4.c
+++ b/repair/phase4.c
@@ -401,4 +401,11 @@  phase4(xfs_mount_t *mp)
 	 */
 	quotino_check(mp);
 	quota_sb_check(mp);
+
+	/* Check the rt metadata before we rebuild */
+	if (mp->m_sb.sb_rblocks)  {
+		do_log(
+		_("        - generate realtime summary info and bitmap...\n"));
+		check_rtmetadata(mp);
+	}
 }
diff --git a/repair/phase5.c b/repair/phase5.c
index d18ec095b0524b..9207da7172c05b 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -694,12 +694,6 @@  phase5(xfs_mount_t *mp)
 	free(sb_ifree_ag);
 	free(sb_fdblocks_ag);
 
-	if (mp->m_sb.sb_rblocks)  {
-		do_log(
-		_("        - generate realtime summary info and bitmap...\n"));
-		check_rtmetadata(mp);
-	}
-
 	do_log(_("        - reset superblock...\n"));
 
 	/*
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index e325d61f10367e..3ade85bbcbb7fd 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -1318,9 +1318,6 @@  main(int argc, char **argv)
 
 	if (no_modify) {
 		printf(_("No modify flag set, skipping phase 5\n"));
-
-		if (mp->m_sb.sb_rblocks > 0)
-			check_rtmetadata(mp);
 	} else {
 		phase5(mp);
 	}