[PATCHSET 0/4] xfs_repair: check rt bitmap and summary

[PATCHSET 0/4] xfs_repair: check rt bitmap and summary

[Darrick J. Wong]

Hi all,

I was evaluating the effectiveness of xfs_repair vs. xfs_scrub with
realtime filesystems the other day, and noticed that repair doesn't
check the free rt extent count, the contents of the rt bitmap, or the
contents of the rt summary.  It'll rebuild them with whatever
observations it makes, but it doesn't actually complain about problems.
That's a bit untidy, so let's have it do that.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=repair-check-rt-metadata
---
 repair/incore.c     |    2 
 repair/phase5.c     |   15 ++++
 repair/rt.c         |  214 ++++++++++++++++++---------------------------------
 repair/rt.h         |   18 +---
 repair/xfs_repair.c |    6 -
 5 files changed, 97 insertions(+), 158 deletions(-)

[PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

If someone creates a realtime volume exactly *one* extent in length, the
sizing calculation for the incore rt space usage bitmap will be zero
because the integer division here rounds down.  Use howmany() to round
up.  Note that there can't be that many single-extent rt volumes since
repair will corrupt them into zero-extent rt volumes, and we haven't
gotten any reports.

Found by running xfs/530 after fixing xfs_repair to check the rt bitmap.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 repair/incore.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/repair/incore.c b/repair/incore.c
index 4ffe18ab..10a8c2a8 100644
--- a/repair/incore.c
+++ b/repair/incore.c
@@ -209,7 +209,7 @@ init_rt_bmap(
 	if (mp->m_sb.sb_rextents == 0)
 		return;
 
-	rt_bmap_size = roundup(mp->m_sb.sb_rextents / (NBBY / XR_BB),
+	rt_bmap_size = roundup(howmany(mp->m_sb.sb_rextents, (NBBY / XR_BB)),
 			       sizeof(uint64_t));
 
 	rt_bmap = memalign(sizeof(uint64_t), rt_bmap_size);

Re: [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation

[Christoph Hellwig]

On Thu, May 05, 2022 at 09:04:48AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If someone creates a realtime volume exactly *one* extent in length, the
> sizing calculation for the incore rt space usage bitmap will be zero
> because the integer division here rounds down.  Use howmany() to round
> up.  Note that there can't be that many single-extent rt volumes since
> repair will corrupt them into zero-extent rt volumes, and we haven't
> gotten any reports.

Looks good:

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

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

[Darrick J. Wong]

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(-)


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);
 
 	/*

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

[Christoph Hellwig]

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?

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

[Darrick J. Wong]

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

[PATCH 3/4] xfs_repair: check the rt bitmap against observations

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Teach xfs_repair to check the ondisk realtime bitmap against its own
observations.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 repair/phase5.c |    2 +
 repair/rt.c     |  168 ++++++++++++++++++++++++++-----------------------------
 repair/rt.h     |   11 ++--
 3 files changed, 87 insertions(+), 94 deletions(-)


diff --git a/repair/phase5.c b/repair/phase5.c
index f097f0fe..ae444efb 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -616,6 +616,7 @@ phase5(xfs_mount_t *mp)
 		if (mp->m_sb.sb_rblocks) {
 			rtinit(mp);
 			generate_rtinfo(mp, btmcompute, sumcompute);
+			check_rtbitmap(mp);
 		}
 
 		return;
@@ -684,6 +685,7 @@ phase5(xfs_mount_t *mp)
 		_("        - generate realtime summary info and bitmap...\n"));
 		rtinit(mp);
 		generate_rtinfo(mp, btmcompute, sumcompute);
+		check_rtbitmap(mp);
 	}
 
 	do_log(_("        - reset superblock...\n"));
diff --git a/repair/rt.c b/repair/rt.c
index 3a065f4b..b964d168 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -119,6 +119,85 @@ generate_rtinfo(xfs_mount_t	*mp,
 	return(0);
 }
 
+static void
+check_rtfile_contents(
+	struct xfs_mount	*mp,
+	const char		*filename,
+	xfs_ino_t		ino,
+	void			*buf,
+	xfs_fileoff_t		filelen)
+{
+	struct xfs_bmbt_irec	map;
+	struct xfs_buf		*bp;
+	struct xfs_inode	*ip;
+	xfs_fileoff_t		bno = 0;
+	int			error;
+
+	error = -libxfs_iget(mp, NULL, ino, 0, &ip);
+	if (error) {
+		do_warn(_("unable to open %s file, err %d\n"), filename, error);
+		return;
+	}
+
+	if (ip->i_disk_size != XFS_FSB_TO_B(mp, filelen)) {
+		do_warn(_("expected %s file size %llu, found %llu\n"),
+				filename,
+				(unsigned long long)XFS_FSB_TO_B(mp, filelen),
+				(unsigned long long)ip->i_disk_size);
+	}
+
+	while (bno < filelen)  {
+		xfs_filblks_t	maplen;
+		int		nmap = 1;
+
+		/* Read up to 1MB at a time. */
+		maplen = min(filelen - bno, XFS_B_TO_FSBT(mp, 1048576));
+		error = -libxfs_bmapi_read(ip, bno, maplen, &map, &nmap, 0);
+		if (error) {
+			do_warn(_("unable to read %s mapping, err %d\n"),
+					filename, error);
+			break;
+		}
+
+		if (map.br_startblock == HOLESTARTBLOCK) {
+			do_warn(_("hole in %s file at dblock 0x%llx\n"),
+					filename, (unsigned long long)bno);
+			break;
+		}
+
+		error = -libxfs_buf_read_uncached(mp->m_dev,
+				XFS_FSB_TO_DADDR(mp, map.br_startblock),
+				XFS_FSB_TO_BB(mp, map.br_blockcount),
+				0, &bp, NULL);
+		if (error) {
+			do_warn(_("unable to read %s at dblock 0x%llx, err %d\n"),
+					filename, (unsigned long long)bno, error);
+			break;
+		}
+
+		if (memcmp(bp->b_addr, buf, mp->m_sb.sb_blocksize))
+			do_warn(_("discrepancy in %s at dblock 0x%llx\n"),
+					filename, (unsigned long long)bno);
+
+		buf += XFS_FSB_TO_B(mp, map.br_blockcount);
+		bno += map.br_blockcount;
+		libxfs_buf_relse(bp);
+	}
+
+	libxfs_irele(ip);
+}
+
+void
+check_rtbitmap(
+	struct xfs_mount	*mp)
+{
+	if (need_rbmino)
+		return;
+
+	check_rtfile_contents(mp, "rtbitmap", mp->m_sb.sb_rbmino, btmcompute,
+			mp->m_sb.sb_rbmblocks);
+}
+
 #if 0
 /*
  * returns 1 if bad, 0 if good
@@ -151,95 +230,6 @@ check_summary(xfs_mount_t *mp)
 	return(error);
 }
 
-/*
- * examine the real-time bitmap file and compute summary
- * info off it.  Should probably be changed to compute
- * the summary information off the incore computed bitmap
- * instead of the realtime bitmap file
- */
-void
-process_rtbitmap(
-	struct xfs_mount	*mp,
-	struct xfs_dinode	*dino,
-	blkmap_t		*blkmap)
-{
-	int			error;
-	int			bit;
-	int			bitsperblock;
-	int			bmbno;
-	int			end_bmbno;
-	xfs_fsblock_t		bno;
-	struct xfs_buf		*bp;
-	xfs_rtblock_t		extno;
-	int			i;
-	int			len;
-	int			log;
-	int			offs;
-	int			prevbit;
-	int			start_bmbno;
-	int			start_bit;
-	xfs_rtword_t		*words;
-
-	ASSERT(mp->m_rbmip == NULL);
-
-	bitsperblock = mp->m_sb.sb_blocksize * NBBY;
-	prevbit = 0;
-	extno = 0;
-	error = 0;
-
-	end_bmbno = howmany(be64_to_cpu(dino->di_size),
-						mp->m_sb.sb_blocksize);
-
-	for (bmbno = 0; bmbno < end_bmbno; bmbno++) {
-		bno = blkmap_get(blkmap, bmbno);
-
-		if (bno == NULLFSBLOCK) {
-			do_warn(_("can't find block %d for rtbitmap inode\n"),
-					bmbno);
-			error = 1;
-			continue;
-		}
-		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				XFS_FSB_TO_BB(mp, 1), 0, NULL, &bp);
-		if (error) {
-			do_warn(_("can't read block %d for rtbitmap inode\n"),
-					bmbno);
-			error = 1;
-			continue;
-		}
-		words = (xfs_rtword_t *)bp->b_un.b_addr;
-		for (bit = 0;
-		     bit < bitsperblock && extno < mp->m_sb.sb_rextents;
-		     bit++, extno++) {
-			if (xfs_isset(words, bit)) {
-				set_rtbmap(extno, XR_E_FREE);
-				sb_frextents++;
-				if (prevbit == 0) {
-					start_bmbno = bmbno;
-					start_bit = bit;
-					prevbit = 1;
-				}
-			} else if (prevbit == 1) {
-				len = (bmbno - start_bmbno) * bitsperblock +
-					(bit - start_bit);
-				log = XFS_RTBLOCKLOG(len);
-				offs = XFS_SUMOFFS(mp, log, start_bmbno);
-				sumcompute[offs]++;
-				prevbit = 0;
-			}
-		}
-		libxfs_buf_relse(bp);
-		if (extno == mp->m_sb.sb_rextents)
-			break;
-	}
-	if (prevbit == 1) {
-		len = (bmbno - start_bmbno) * bitsperblock + (bit - start_bit);
-		log = XFS_RTBLOCKLOG(len);
-		offs = XFS_SUMOFFS(mp, log, start_bmbno);
-		sumcompute[offs]++;
-	}
-}
-
 /*
  * copy the real-time summary file data into memory
  */
diff --git a/repair/rt.h b/repair/rt.h
index f5d8f80c..2023153f 100644
--- a/repair/rt.h
+++ b/repair/rt.h
@@ -3,6 +3,8 @@
  * Copyright (c) 2000-2001,2005 Silicon Graphics, Inc.
  * All Rights Reserved.
  */
+#ifndef _XFS_REPAIR_RT_H_
+#define _XFS_REPAIR_RT_H_
 
 struct blkmap;
 
@@ -14,17 +16,16 @@ generate_rtinfo(xfs_mount_t	*mp,
 		xfs_rtword_t	*words,
 		xfs_suminfo_t	*sumcompute);
 
+void check_rtbitmap(struct xfs_mount *mp);
+
 #if 0
 
 int
 check_summary(xfs_mount_t	*mp);
 
-void
-process_rtbitmap(xfs_mount_t		*mp,
-		struct xfs_dinode	*dino,
-		struct blkmap		*blkmap);
-
 void
 process_rtsummary(xfs_mount_t	*mp,
 		struct blkmap	*blkmap);
 #endif
+
+#endif /* _XFS_REPAIR_RT_H_ */

Re: [PATCH 3/4] xfs_repair: check the rt bitmap against observations

[Christoph Hellwig]

Looks good:

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

[PATCH 4/4] xfs_repair: check the rt summary against observations

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Teach xfs_repair to check the ondisk realtime summary file against its
own observations.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 repair/phase5.c |    2 ++
 repair/rt.c     |   71 +++++--------------------------------------------------
 repair/rt.h     |   11 +--------
 3 files changed, 9 insertions(+), 75 deletions(-)


diff --git a/repair/phase5.c b/repair/phase5.c
index ae444efb..42b0f117 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -617,6 +617,7 @@ phase5(xfs_mount_t *mp)
 			rtinit(mp);
 			generate_rtinfo(mp, btmcompute, sumcompute);
 			check_rtbitmap(mp);
+			check_rtsummary(mp);
 		}
 
 		return;
@@ -686,6 +687,7 @@ phase5(xfs_mount_t *mp)
 		rtinit(mp);
 		generate_rtinfo(mp, btmcompute, sumcompute);
 		check_rtbitmap(mp);
+		check_rtsummary(mp);
 	}
 
 	do_log(_("        - reset superblock...\n"));
diff --git a/repair/rt.c b/repair/rt.c
index b964d168..a4cca7aa 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -198,72 +198,13 @@ check_rtbitmap(
 			mp->m_sb.sb_rbmblocks);
 }
 
-#if 0
-/*
- * returns 1 if bad, 0 if good
- */
-int
-check_summary(xfs_mount_t *mp)
-{
-	xfs_rfsblock_t	bno;
-	xfs_suminfo_t	*csp;
-	xfs_suminfo_t	*fsp;
-	int		log;
-	int		error = 0;
-
-	error = 0;
-	csp = sumcompute;
-	fsp = sumfile;
-	for (log = 0; log < mp->m_rsumlevels; log++) {
-		for (bno = 0;
-		     bno < mp->m_sb.sb_rbmblocks;
-		     bno++, csp++, fsp++) {
-			if (*csp != *fsp) {
-				do_warn(
-	_("rt summary mismatch, size %d block %llu, file: %d, computed: %d\n"),
-						log, bno, *fsp, *csp);
-				error = 1;
-			}
-		}
-	}
-
-	return(error);
-}
-
-/*
- * copy the real-time summary file data into memory
- */
 void
-process_rtsummary(
-	xfs_mount_t		*mp,
-	struct xfs_dinode	*dino,
-	blkmap_t		*blkmap)
+check_rtsummary(
+	struct xfs_mount	*mp)
 {
-	xfs_fsblock_t		bno;
-	struct xfs_buf		*bp;
-	char			*bytes;
-	int			sumbno;
+	if (need_rsumino)
+		return;
 
-	for (sumbno = 0; sumbno < blkmap->count; sumbno++) {
-		bno = blkmap_get(blkmap, sumbno);
-		if (bno == NULLFSBLOCK) {
-			do_warn(_("block %d for rtsummary inode is missing\n"),
-					sumbno);
-			error++;
-			continue;
-		}
-		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				XFS_FSB_TO_BB(mp, 1), 0, NULL, &bp);
-		if (error) {
-			do_warn(_("can't read block %d for rtsummary inode\n"),
-					sumbno);
-			error++;
-			continue;
-		}
-		bytes = bp->b_un.b_addr;
-		memmove((char *)sumfile + sumbno * mp->m_sb.sb_blocksize, bytes,
-			mp->m_sb.sb_blocksize);
-		libxfs_buf_relse(bp);
-	}
+	check_rtfile_contents(mp, "rtsummary", mp->m_sb.sb_rsumino, sumcompute,
+			XFS_B_TO_FSB(mp, mp->m_rsumsize));
 }
-#endif
diff --git a/repair/rt.h b/repair/rt.h
index 2023153f..be24e91c 100644
--- a/repair/rt.h
+++ b/repair/rt.h
@@ -17,15 +17,6 @@ generate_rtinfo(xfs_mount_t	*mp,
 		xfs_suminfo_t	*sumcompute);
 
 void check_rtbitmap(struct xfs_mount *mp);
-
-#if 0
-
-int
-check_summary(xfs_mount_t	*mp);
-
-void
-process_rtsummary(xfs_mount_t	*mp,
-		struct blkmap	*blkmap);
-#endif
+void check_rtsummary(struct xfs_mount *mp);
 
 #endif /* _XFS_REPAIR_RT_H_ */

Re: [PATCH 4/4] xfs_repair: check the rt summary against observations

[Christoph Hellwig]

Looks good:

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

[PATCHSET v2 0/4] xfs_repair: check rt bitmap and summary

[Darrick J. Wong]

Hi all,

I was evaluating the effectiveness of xfs_repair vs. xfs_scrub with
realtime filesystems the other day, and noticed that repair doesn't
check the free rt extent count, the contents of the rt bitmap, or the
contents of the rt summary.  It'll rebuild them with whatever
observations it makes, but it doesn't actually complain about problems.
That's a bit untidy, so let's have it do that.

v2: only check the rt bitmap when the primary super claims there's a
    realtime section

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=repair-check-rt-metadata
---
 repair/incore.c     |    2 
 repair/phase5.c     |   13 +++
 repair/protos.h     |    1 
 repair/rt.c         |  214 ++++++++++++++++++---------------------------------
 repair/rt.h         |   18 +---
 repair/xfs_repair.c |    7 +-
 6 files changed, 98 insertions(+), 157 deletions(-)

[PATCH 4/4] xfs_repair: check the rt summary against observations

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Teach xfs_repair to check the ondisk realtime summary file against its
own observations.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 repair/phase5.c |    1 +
 repair/rt.c     |   71 +++++--------------------------------------------------
 repair/rt.h     |   11 +--------
 3 files changed, 8 insertions(+), 75 deletions(-)


diff --git a/repair/phase5.c b/repair/phase5.c
index d1ddd224..b04912d8 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -609,6 +609,7 @@ check_rtmetadata(
 	rtinit(mp);
 	generate_rtinfo(mp, btmcompute, sumcompute);
 	check_rtbitmap(mp);
+	check_rtsummary(mp);
 }
 
 void
diff --git a/repair/rt.c b/repair/rt.c
index b964d168..a4cca7aa 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -198,72 +198,13 @@ check_rtbitmap(
 			mp->m_sb.sb_rbmblocks);
 }
 
-#if 0
-/*
- * returns 1 if bad, 0 if good
- */
-int
-check_summary(xfs_mount_t *mp)
-{
-	xfs_rfsblock_t	bno;
-	xfs_suminfo_t	*csp;
-	xfs_suminfo_t	*fsp;
-	int		log;
-	int		error = 0;
-
-	error = 0;
-	csp = sumcompute;
-	fsp = sumfile;
-	for (log = 0; log < mp->m_rsumlevels; log++) {
-		for (bno = 0;
-		     bno < mp->m_sb.sb_rbmblocks;
-		     bno++, csp++, fsp++) {
-			if (*csp != *fsp) {
-				do_warn(
-	_("rt summary mismatch, size %d block %llu, file: %d, computed: %d\n"),
-						log, bno, *fsp, *csp);
-				error = 1;
-			}
-		}
-	}
-
-	return(error);
-}
-
-/*
- * copy the real-time summary file data into memory
- */
 void
-process_rtsummary(
-	xfs_mount_t		*mp,
-	struct xfs_dinode	*dino,
-	blkmap_t		*blkmap)
+check_rtsummary(
+	struct xfs_mount	*mp)
 {
-	xfs_fsblock_t		bno;
-	struct xfs_buf		*bp;
-	char			*bytes;
-	int			sumbno;
+	if (need_rsumino)
+		return;
 
-	for (sumbno = 0; sumbno < blkmap->count; sumbno++) {
-		bno = blkmap_get(blkmap, sumbno);
-		if (bno == NULLFSBLOCK) {
-			do_warn(_("block %d for rtsummary inode is missing\n"),
-					sumbno);
-			error++;
-			continue;
-		}
-		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				XFS_FSB_TO_BB(mp, 1), 0, NULL, &bp);
-		if (error) {
-			do_warn(_("can't read block %d for rtsummary inode\n"),
-					sumbno);
-			error++;
-			continue;
-		}
-		bytes = bp->b_un.b_addr;
-		memmove((char *)sumfile + sumbno * mp->m_sb.sb_blocksize, bytes,
-			mp->m_sb.sb_blocksize);
-		libxfs_buf_relse(bp);
-	}
+	check_rtfile_contents(mp, "rtsummary", mp->m_sb.sb_rsumino, sumcompute,
+			XFS_B_TO_FSB(mp, mp->m_rsumsize));
 }
-#endif
diff --git a/repair/rt.h b/repair/rt.h
index 2023153f..be24e91c 100644
--- a/repair/rt.h
+++ b/repair/rt.h
@@ -17,15 +17,6 @@ generate_rtinfo(xfs_mount_t	*mp,
 		xfs_suminfo_t	*sumcompute);
 
 void check_rtbitmap(struct xfs_mount *mp);
-
-#if 0
-
-int
-check_summary(xfs_mount_t	*mp);
-
-void
-process_rtsummary(xfs_mount_t	*mp,
-		struct blkmap	*blkmap);
-#endif
+void check_rtsummary(struct xfs_mount *mp);
 
 #endif /* _XFS_REPAIR_RT_H_ */

Re: [PATCH 4/4] xfs_repair: check the rt summary against observations

[Chandan Babu R]

On Fri, May 13, 2022 at 01:34:16 PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Teach xfs_repair to check the ondisk realtime summary file against its
> own observations.
>

Looks good.

Reviewed-by: Chandan Babu R <chandan.babu@oracle.com>

> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  repair/phase5.c |    1 +
>  repair/rt.c     |   71 +++++--------------------------------------------------
>  repair/rt.h     |   11 +--------
>  3 files changed, 8 insertions(+), 75 deletions(-)
>
>
> diff --git a/repair/phase5.c b/repair/phase5.c
> index d1ddd224..b04912d8 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -609,6 +609,7 @@ check_rtmetadata(
>  	rtinit(mp);
>  	generate_rtinfo(mp, btmcompute, sumcompute);
>  	check_rtbitmap(mp);
> +	check_rtsummary(mp);
>  }
>  
>  void
> diff --git a/repair/rt.c b/repair/rt.c
> index b964d168..a4cca7aa 100644
> --- a/repair/rt.c
> +++ b/repair/rt.c
> @@ -198,72 +198,13 @@ check_rtbitmap(
>  			mp->m_sb.sb_rbmblocks);
>  }
>  
> -#if 0
> -/*
> - * returns 1 if bad, 0 if good
> - */
> -int
> -check_summary(xfs_mount_t *mp)
> -{
> -	xfs_rfsblock_t	bno;
> -	xfs_suminfo_t	*csp;
> -	xfs_suminfo_t	*fsp;
> -	int		log;
> -	int		error = 0;
> -
> -	error = 0;
> -	csp = sumcompute;
> -	fsp = sumfile;
> -	for (log = 0; log < mp->m_rsumlevels; log++) {
> -		for (bno = 0;
> -		     bno < mp->m_sb.sb_rbmblocks;
> -		     bno++, csp++, fsp++) {
> -			if (*csp != *fsp) {
> -				do_warn(
> -	_("rt summary mismatch, size %d block %llu, file: %d, computed: %d\n"),
> -						log, bno, *fsp, *csp);
> -				error = 1;
> -			}
> -		}
> -	}
> -
> -	return(error);
> -}
> -
> -/*
> - * copy the real-time summary file data into memory
> - */
>  void
> -process_rtsummary(
> -	xfs_mount_t		*mp,
> -	struct xfs_dinode	*dino,
> -	blkmap_t		*blkmap)
> +check_rtsummary(
> +	struct xfs_mount	*mp)
>  {
> -	xfs_fsblock_t		bno;
> -	struct xfs_buf		*bp;
> -	char			*bytes;
> -	int			sumbno;
> +	if (need_rsumino)
> +		return;
>  
> -	for (sumbno = 0; sumbno < blkmap->count; sumbno++) {
> -		bno = blkmap_get(blkmap, sumbno);
> -		if (bno == NULLFSBLOCK) {
> -			do_warn(_("block %d for rtsummary inode is missing\n"),
> -					sumbno);
> -			error++;
> -			continue;
> -		}
> -		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
> -				XFS_FSB_TO_BB(mp, 1), 0, NULL, &bp);
> -		if (error) {
> -			do_warn(_("can't read block %d for rtsummary inode\n"),
> -					sumbno);
> -			error++;
> -			continue;
> -		}
> -		bytes = bp->b_un.b_addr;
> -		memmove((char *)sumfile + sumbno * mp->m_sb.sb_blocksize, bytes,
> -			mp->m_sb.sb_blocksize);
> -		libxfs_buf_relse(bp);
> -	}
> +	check_rtfile_contents(mp, "rtsummary", mp->m_sb.sb_rsumino, sumcompute,
> +			XFS_B_TO_FSB(mp, mp->m_rsumsize));
>  }
> -#endif
> diff --git a/repair/rt.h b/repair/rt.h
> index 2023153f..be24e91c 100644
> --- a/repair/rt.h
> +++ b/repair/rt.h
> @@ -17,15 +17,6 @@ generate_rtinfo(xfs_mount_t	*mp,
>  		xfs_suminfo_t	*sumcompute);
>  
>  void check_rtbitmap(struct xfs_mount *mp);
> -
> -#if 0
> -
> -int
> -check_summary(xfs_mount_t	*mp);
> -
> -void
> -process_rtsummary(xfs_mount_t	*mp,
> -		struct blkmap	*blkmap);
> -#endif
> +void check_rtsummary(struct xfs_mount *mp);
>  
>  #endif /* _XFS_REPAIR_RT_H_ */


-- 
chandan