diff mbox series

[2/4] xfs_repair: check free rt extent count

Message ID 165176669425.247207.10108411720464005906.stgit@magnolia (mailing list archive)
State Superseded
Headers show
Series xfs_repair: check rt bitmap and summary | expand

Commit Message

Darrick J. Wong May 5, 2022, 4:04 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Check the superblock's free rt extent count against what we observed.
This increases the runtime and memory usage, but we can now report
undercounting frextents as a result of a logging bug in the kernel.
Note that repair has always fixed the undercount, but it no longer does
that silently.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 repair/phase5.c     |   11 +++++++++++
 repair/rt.c         |    5 +++++
 repair/xfs_repair.c |    6 +-----
 3 files changed, 17 insertions(+), 5 deletions(-)

Comments

Christoph Hellwig May 10, 2022, 12:52 p.m. UTC | #1 
On Thu, May 05, 2022 at 09:04:54AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Check the superblock's free rt extent count against what we observed.
> This increases the runtime and memory usage, but we can now report
> undercounting frextents as a result of a logging bug in the kernel.
> Note that repair has always fixed the undercount, but it no longer does
> that silently.

This looks sensible, but can't we still skip running phase5 for
file systems without an RT subvolume?
Darrick J. Wong May 11, 2022, 12:07 a.m. UTC | #2 
On Tue, May 10, 2022 at 05:52:08AM -0700, Christoph Hellwig wrote:
> On Thu, May 05, 2022 at 09:04:54AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Check the superblock's free rt extent count against what we observed.
> > This increases the runtime and memory usage, but we can now report
> > undercounting frextents as a result of a logging bug in the kernel.
> > Note that repair has always fixed the undercount, but it no longer does
> > that silently.
> 
> This looks sensible, but can't we still skip running phase5 for
> file systems without an RT subvolume?

Yeah, I was of half a mind to make a separate function phase5_nomodify
instead of burying that in phase5_func()...

...but OTOH, I suppose the dinode inspection functions will flag errors
if the superblock says sb_rextents == 0 but DIFLAG_REALTIME is set on a
regular file.  So, I guess that could be a separate check_rtmetadata()
function that lives inside phase5.c and gets triggered in the
(nomodify && sb_rblocks > 0) case.

--D
diff mbox series

Patch

diff --git a/repair/phase5.c b/repair/phase5.c
index 74b1dcb9..f097f0fe 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -610,6 +610,17 @@  phase5(xfs_mount_t *mp)
 	xfs_agnumber_t		agno;
 	int			error;
 
+	if (no_modify) {
+		printf(_("No modify flag set, skipping phase 5\n"));
+
+		if (mp->m_sb.sb_rblocks) {
+			rtinit(mp);
+			generate_rtinfo(mp, btmcompute, sumcompute);
+		}
+
+		return;
+	}
+
 	do_log(_("Phase 5 - rebuild AG headers and trees...\n"));
 	set_progress_msg(PROG_FMT_REBUILD_AG, (uint64_t)glob_agcount);
 
diff --git a/repair/rt.c b/repair/rt.c
index d663a01d..3a065f4b 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -111,6 +111,11 @@  generate_rtinfo(xfs_mount_t	*mp,
 		sumcompute[offs]++;
 	}
 
+	if (mp->m_sb.sb_frextents != sb_frextents) {
+		do_warn(_("sb_frextents %" PRIu64 ", counted %" PRIu64 "\n"),
+				mp->m_sb.sb_frextents, sb_frextents);
+	}
+
 	return(0);
 }
 
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index de8617ba..ef2a6ff1 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -1174,11 +1174,7 @@  main(int argc, char **argv)
 	phase4(mp);
 	phase_end(4);
 
-	if (no_modify)
-		printf(_("No modify flag set, skipping phase 5\n"));
-	else {
-		phase5(mp);
-	}
+	phase5(mp);
 	phase_end(5);
 
 	/*