All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHSET 0/4] xfs_repair: check rt bitmap and summary
@ 2022-05-05 16:04 Darrick J. Wong
  2022-05-05 16:04 ` [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation Darrick J. Wong
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Darrick J. Wong @ 2022-05-05 16:04 UTC (permalink / raw)
  To: sandeen, djwong; +Cc: linux-xfs

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation
  2022-05-05 16:04 [PATCHSET 0/4] xfs_repair: check rt bitmap and summary Darrick J. Wong
@ 2022-05-05 16:04 ` Darrick J. Wong
  2022-05-10 12:51   ` Christoph Hellwig
  2022-05-05 16:04 ` [PATCH 2/4] xfs_repair: check free rt extent count Darrick J. Wong
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2022-05-05 16:04 UTC (permalink / raw)
  To: sandeen, djwong; +Cc: linux-xfs

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] xfs_repair: check free rt extent count
  2022-05-05 16:04 [PATCHSET 0/4] xfs_repair: check rt bitmap and summary Darrick J. Wong
  2022-05-05 16:04 ` [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation Darrick J. Wong
@ 2022-05-05 16:04 ` Darrick J. Wong
  2022-05-10 12:52   ` Christoph Hellwig
  2022-05-05 16:04 ` [PATCH 3/4] xfs_repair: check the rt bitmap against observations Darrick J. Wong
  2022-05-05 16:05 ` [PATCH 4/4] xfs_repair: check the rt summary " Darrick J. Wong
  3 siblings, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2022-05-05 16:04 UTC (permalink / raw)
  To: sandeen, djwong; +Cc: linux-xfs

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] xfs_repair: check the rt bitmap against observations
  2022-05-05 16:04 [PATCHSET 0/4] xfs_repair: check rt bitmap and summary Darrick J. Wong
  2022-05-05 16:04 ` [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation Darrick J. Wong
  2022-05-05 16:04 ` [PATCH 2/4] xfs_repair: check free rt extent count Darrick J. Wong
@ 2022-05-05 16:04 ` Darrick J. Wong
  2022-05-10 12:53   ` Christoph Hellwig
  2022-05-05 16:05 ` [PATCH 4/4] xfs_repair: check the rt summary " Darrick J. Wong
  3 siblings, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2022-05-05 16:04 UTC (permalink / raw)
  To: sandeen, djwong; +Cc: linux-xfs

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_ */


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] xfs_repair: check the rt summary against observations
  2022-05-05 16:04 [PATCHSET 0/4] xfs_repair: check rt bitmap and summary Darrick J. Wong
                   ` (2 preceding siblings ...)
  2022-05-05 16:04 ` [PATCH 3/4] xfs_repair: check the rt bitmap against observations Darrick J. Wong
@ 2022-05-05 16:05 ` Darrick J. Wong
  2022-05-10 12:54   ` Christoph Hellwig
  3 siblings, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2022-05-05 16:05 UTC (permalink / raw)
  To: sandeen, djwong; +Cc: linux-xfs

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_ */


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation
  2022-05-05 16:04 ` [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation Darrick J. Wong
@ 2022-05-10 12:51   ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2022-05-10 12:51 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] xfs_repair: check free rt extent count
  2022-05-05 16:04 ` [PATCH 2/4] xfs_repair: check free rt extent count Darrick J. Wong
@ 2022-05-10 12:52   ` Christoph Hellwig
  2022-05-11  0:07     ` Darrick J. Wong
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2022-05-10 12:52 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

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?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] xfs_repair: check the rt bitmap against observations
  2022-05-05 16:04 ` [PATCH 3/4] xfs_repair: check the rt bitmap against observations Darrick J. Wong
@ 2022-05-10 12:53   ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2022-05-10 12:53 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

Looks good:

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] xfs_repair: check the rt summary against observations
  2022-05-05 16:05 ` [PATCH 4/4] xfs_repair: check the rt summary " Darrick J. Wong
@ 2022-05-10 12:54   ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2022-05-10 12:54 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

Looks good:

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] xfs_repair: check free rt extent count
  2022-05-10 12:52   ` Christoph Hellwig
@ 2022-05-11  0:07     ` Darrick J. Wong
  0 siblings, 0 replies; 12+ messages in thread
From: Darrick J. Wong @ 2022-05-11  0:07 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: sandeen, linux-xfs

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] xfs_repair: check the rt bitmap against observations
  2022-05-13 20:34 ` [PATCH 3/4] xfs_repair: check the rt bitmap against observations Darrick J. Wong
@ 2022-05-16 10:50   ` Chandan Babu R
  0 siblings, 0 replies; 12+ messages in thread
From: Chandan Babu R @ 2022-05-16 10:50 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, Christoph Hellwig, linux-xfs

On Fri, May 13, 2022 at 01:34:10 PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Teach xfs_repair to check the ondisk realtime bitmap 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     |  168 ++++++++++++++++++++++++++-----------------------------
>  repair/rt.h     |   11 ++--
>  3 files changed, 86 insertions(+), 94 deletions(-)
>
>
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 273f51a8..d1ddd224 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -608,6 +608,7 @@ check_rtmetadata(
>  {
>  	rtinit(mp);
>  	generate_rtinfo(mp, btmcompute, sumcompute);
> +	check_rtbitmap(mp);
>  }
>  
>  void
> 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_ */


-- 
chandan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/4] xfs_repair: check the rt bitmap against observations
  2022-05-13 20:33 [PATCHSET v2 0/4] xfs_repair: check rt bitmap and summary Darrick J. Wong
@ 2022-05-13 20:34 ` Darrick J. Wong
  2022-05-16 10:50   ` Chandan Babu R
  0 siblings, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2022-05-13 20:34 UTC (permalink / raw)
  To: sandeen, djwong; +Cc: Christoph Hellwig, linux-xfs

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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 repair/phase5.c |    1 
 repair/rt.c     |  168 ++++++++++++++++++++++++++-----------------------------
 repair/rt.h     |   11 ++--
 3 files changed, 86 insertions(+), 94 deletions(-)


diff --git a/repair/phase5.c b/repair/phase5.c
index 273f51a8..d1ddd224 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -608,6 +608,7 @@ check_rtmetadata(
 {
 	rtinit(mp);
 	generate_rtinfo(mp, btmcompute, sumcompute);
+	check_rtbitmap(mp);
 }
 
 void
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_ */


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-05-16 11:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 16:04 [PATCHSET 0/4] xfs_repair: check rt bitmap and summary Darrick J. Wong
2022-05-05 16:04 ` [PATCH 1/4] xfs_repair: fix sizing of the incore rt space usage map calculation Darrick J. Wong
2022-05-10 12:51   ` Christoph Hellwig
2022-05-05 16:04 ` [PATCH 2/4] xfs_repair: check free rt extent count Darrick J. Wong
2022-05-10 12:52   ` Christoph Hellwig
2022-05-11  0:07     ` Darrick J. Wong
2022-05-05 16:04 ` [PATCH 3/4] xfs_repair: check the rt bitmap against observations Darrick J. Wong
2022-05-10 12:53   ` Christoph Hellwig
2022-05-05 16:05 ` [PATCH 4/4] xfs_repair: check the rt summary " Darrick J. Wong
2022-05-10 12:54   ` Christoph Hellwig
2022-05-13 20:33 [PATCHSET v2 0/4] xfs_repair: check rt bitmap and summary Darrick J. Wong
2022-05-13 20:34 ` [PATCH 3/4] xfs_repair: check the rt bitmap against observations Darrick J. Wong
2022-05-16 10:50   ` Chandan Babu R

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.