diff mbox series

[6/9] xfs_repair: check inode btree block counters in AGI

Message ID 160375522427.880355.15446960142376313542.stgit@magnolia
State Superseded
Headers show
Series xfsprogs: add a inode btree blocks counts to the AGI header | expand

Commit Message

Darrick J. Wong Oct. 26, 2020, 11:33 p.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

Make sure that both inode btree block counters in the AGI are correct.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/scan.c |   38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

Comments

Brian Foster Oct. 28, 2020, 5:29 p.m. UTC | #1
On Mon, Oct 26, 2020 at 04:33:44PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Make sure that both inode btree block counters in the AGI are correct.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  repair/scan.c |   38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/repair/scan.c b/repair/scan.c
> index 2a38ae5197c6..c826af7dee86 100644
> --- a/repair/scan.c
> +++ b/repair/scan.c
...
> @@ -2022,6 +2029,17 @@ scan_inobt(
>  			return;
>  	}
>  
> +	switch (magic) {
> +	case XFS_FIBT_MAGIC:
> +	case XFS_FIBT_CRC_MAGIC:
> +		ipriv->fino_blocks++;
> +		break;
> +	case XFS_IBT_MAGIC:
> +	case XFS_IBT_CRC_MAGIC:
> +		ipriv->ino_blocks++;
> +		break;
> +	}
> +

Is this intentionally not folded into the earlier magic switch
statement?

>  	/*
>  	 * check for btree blocks multiply claimed, any unknown/free state
>  	 * is ok in the bitmap block.
...
> @@ -2393,6 +2414,17 @@ validate_agi(
>  		}
>  	}
>  
> +	if (xfs_sb_version_hasinobtcounts(&mp->m_sb)) {
> +		if (be32_to_cpu(agi->agi_iblocks) != priv.ino_blocks)
> +			do_warn(_("bad inobt block count %u, saw %u\n"),
> +					priv.ino_blocks,
> +					be32_to_cpu(agi->agi_iblocks));

These two params are backwards (here and below), no?

Brian

> +		if (be32_to_cpu(agi->agi_fblocks) != priv.fino_blocks)
> +			do_warn(_("bad finobt block count %u, saw %u\n"),
> +					priv.fino_blocks,
> +					be32_to_cpu(agi->agi_fblocks));
> +	}
> +
>  	if (be32_to_cpu(agi->agi_count) != agcnts->agicount) {
>  		do_warn(_("agi_count %u, counted %u in ag %u\n"),
>  			 be32_to_cpu(agi->agi_count), agcnts->agicount, agno);
>
Darrick J. Wong Oct. 29, 2020, 1:01 a.m. UTC | #2
On Wed, Oct 28, 2020 at 01:29:59PM -0400, Brian Foster wrote:
> On Mon, Oct 26, 2020 at 04:33:44PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Make sure that both inode btree block counters in the AGI are correct.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  repair/scan.c |   38 +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 35 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/repair/scan.c b/repair/scan.c
> > index 2a38ae5197c6..c826af7dee86 100644
> > --- a/repair/scan.c
> > +++ b/repair/scan.c
> ...
> > @@ -2022,6 +2029,17 @@ scan_inobt(
> >  			return;
> >  	}
> >  
> > +	switch (magic) {
> > +	case XFS_FIBT_MAGIC:
> > +	case XFS_FIBT_CRC_MAGIC:
> > +		ipriv->fino_blocks++;
> > +		break;
> > +	case XFS_IBT_MAGIC:
> > +	case XFS_IBT_CRC_MAGIC:
> > +		ipriv->ino_blocks++;
> > +		break;
> > +	}
> > +
> 
> Is this intentionally not folded into the earlier magic switch
> statement?

I'd originally thought that we wouldn't want to count the block if it's
obviously bad, but the tree and the agi counter are supposed to be
consistent with each other, so yes, we should always bump the counter
when we are pointed towards a block.

> >  	/*
> >  	 * check for btree blocks multiply claimed, any unknown/free state
> >  	 * is ok in the bitmap block.
> ...
> > @@ -2393,6 +2414,17 @@ validate_agi(
> >  		}
> >  	}
> >  
> > +	if (xfs_sb_version_hasinobtcounts(&mp->m_sb)) {
> > +		if (be32_to_cpu(agi->agi_iblocks) != priv.ino_blocks)
> > +			do_warn(_("bad inobt block count %u, saw %u\n"),
> > +					priv.ino_blocks,
> > +					be32_to_cpu(agi->agi_iblocks));
> 
> These two params are backwards (here and below), no?

Oops.  Good catch!

--D

> Brian
> 
> > +		if (be32_to_cpu(agi->agi_fblocks) != priv.fino_blocks)
> > +			do_warn(_("bad finobt block count %u, saw %u\n"),
> > +					priv.fino_blocks,
> > +					be32_to_cpu(agi->agi_fblocks));
> > +	}
> > +
> >  	if (be32_to_cpu(agi->agi_count) != agcnts->agicount) {
> >  		do_warn(_("agi_count %u, counted %u in ag %u\n"),
> >  			 be32_to_cpu(agi->agi_count), agcnts->agicount, agno);
> > 
>
diff mbox series

Patch

diff --git a/repair/scan.c b/repair/scan.c
index 2a38ae5197c6..c826af7dee86 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -1948,6 +1948,12 @@  _("invalid inode count, inode chunk %d/%u, count %d ninodes %d\n"),
 	return suspect;
 }
 
+struct ino_priv {
+	struct aghdr_cnts	*agcnts;
+	uint32_t		ino_blocks;
+	uint32_t		fino_blocks;
+};
+
 /*
  * this one walks the inode btrees sucking the info there into
  * the incore avl tree.  We try and rescue corrupted btree records
@@ -1976,7 +1982,8 @@  scan_inobt(
 	void			*priv,
 	const struct xfs_buf_ops *ops)
 {
-	struct aghdr_cnts	*agcnts = priv;
+	struct ino_priv		*ipriv = priv;
+	struct aghdr_cnts	*agcnts = ipriv->agcnts;
 	char			*name;
 	xfs_agino_t		lastino = 0;
 	int			i;
@@ -2022,6 +2029,17 @@  scan_inobt(
 			return;
 	}
 
+	switch (magic) {
+	case XFS_FIBT_MAGIC:
+	case XFS_FIBT_CRC_MAGIC:
+		ipriv->fino_blocks++;
+		break;
+	case XFS_IBT_MAGIC:
+	case XFS_IBT_CRC_MAGIC:
+		ipriv->ino_blocks++;
+		break;
+	}
+
 	/*
 	 * check for btree blocks multiply claimed, any unknown/free state
 	 * is ok in the bitmap block.
@@ -2363,6 +2381,9 @@  validate_agi(
 	xfs_agnumber_t		agno,
 	struct aghdr_cnts	*agcnts)
 {
+	struct ino_priv		priv = {
+		.agcnts = agcnts,
+	};
 	xfs_agblock_t		bno;
 	int			i;
 	uint32_t		magic;
@@ -2372,7 +2393,7 @@  validate_agi(
 		magic = xfs_sb_version_hascrc(&mp->m_sb) ? XFS_IBT_CRC_MAGIC
 							 : XFS_IBT_MAGIC;
 		scan_sbtree(bno, be32_to_cpu(agi->agi_level),
-			    agno, 0, scan_inobt, 1, magic, agcnts,
+			    agno, 0, scan_inobt, 1, magic, &priv,
 			    &xfs_inobt_buf_ops);
 	} else {
 		do_warn(_("bad agbno %u for inobt root, agno %d\n"),
@@ -2385,7 +2406,7 @@  validate_agi(
 			magic = xfs_sb_version_hascrc(&mp->m_sb) ?
 					XFS_FIBT_CRC_MAGIC : XFS_FIBT_MAGIC;
 			scan_sbtree(bno, be32_to_cpu(agi->agi_free_level),
-				    agno, 0, scan_inobt, 1, magic, agcnts,
+				    agno, 0, scan_inobt, 1, magic, &priv,
 				    &xfs_finobt_buf_ops);
 		} else {
 			do_warn(_("bad agbno %u for finobt root, agno %d\n"),
@@ -2393,6 +2414,17 @@  validate_agi(
 		}
 	}
 
+	if (xfs_sb_version_hasinobtcounts(&mp->m_sb)) {
+		if (be32_to_cpu(agi->agi_iblocks) != priv.ino_blocks)
+			do_warn(_("bad inobt block count %u, saw %u\n"),
+					priv.ino_blocks,
+					be32_to_cpu(agi->agi_iblocks));
+		if (be32_to_cpu(agi->agi_fblocks) != priv.fino_blocks)
+			do_warn(_("bad finobt block count %u, saw %u\n"),
+					priv.fino_blocks,
+					be32_to_cpu(agi->agi_fblocks));
+	}
+
 	if (be32_to_cpu(agi->agi_count) != agcnts->agicount) {
 		do_warn(_("agi_count %u, counted %u in ag %u\n"),
 			 be32_to_cpu(agi->agi_count), agcnts->agicount, agno);