diff mbox series

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

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

Commit Message

Darrick J. Wong May 13, 2022, 8:34 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/protos.h     |    1 +
 repair/rt.c         |    5 +++++
 repair/xfs_repair.c |    7 +++++--
 4 files changed, 20 insertions(+), 4 deletions(-)

Comments

Chandan Babu R May 16, 2022, 9:43 a.m. UTC | #1 
On Fri, May 13, 2022 at 01:34:04 PM -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 changes look good to me from the perspective of logical correctness.

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

> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  repair/phase5.c     |   11 +++++++++--
>  repair/protos.h     |    1 +
>  repair/rt.c         |    5 +++++
>  repair/xfs_repair.c |    7 +++++--
>  4 files changed, 20 insertions(+), 4 deletions(-)
>
>
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 74b1dcb9..273f51a8 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -602,6 +602,14 @@ inject_lost_extent(
>  	return -libxfs_trans_commit(tp);
>  }
>  
> +void
> +check_rtmetadata(
> +	struct xfs_mount	*mp)
> +{
> +	rtinit(mp);
> +	generate_rtinfo(mp, btmcompute, sumcompute);
> +}
> +
>  void
>  phase5(xfs_mount_t *mp)
>  {
> @@ -671,8 +679,7 @@ phase5(xfs_mount_t *mp)
>  	if (mp->m_sb.sb_rblocks)  {
>  		do_log(
>  		_("        - generate realtime summary info and bitmap...\n"));
> -		rtinit(mp);
> -		generate_rtinfo(mp, btmcompute, sumcompute);
> +		check_rtmetadata(mp);
>  	}
>  
>  	do_log(_("        - reset superblock...\n"));
> diff --git a/repair/protos.h b/repair/protos.h
> index 83734e85..03ebae14 100644
> --- a/repair/protos.h
> +++ b/repair/protos.h
> @@ -36,6 +36,7 @@ void	phase1(struct xfs_mount *);
>  void	phase2(struct xfs_mount *, int);
>  void	phase3(struct xfs_mount *, int);
>  void	phase4(struct xfs_mount *);
> +void	check_rtmetadata(struct xfs_mount *mp);
>  void	phase5(struct xfs_mount *);
>  void	phase6(struct xfs_mount *);
>  void	phase7(struct xfs_mount *, int);
> 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..d08b0cec 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -1174,9 +1174,12 @@ main(int argc, char **argv)
>  	phase4(mp);
>  	phase_end(4);
>  
> -	if (no_modify)
> +	if (no_modify) {
>  		printf(_("No modify flag set, skipping phase 5\n"));
> -	else {
> +
> +		if (mp->m_sb.sb_rblocks > 0)
> +			check_rtmetadata(mp);
> +	} else {
>  		phase5(mp);
>  	}
>  	phase_end(5);
Christoph Hellwig May 16, 2022, 1:44 p.m. UTC | #2 
Looks good:

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

Patch

diff --git a/repair/phase5.c b/repair/phase5.c
index 74b1dcb9..273f51a8 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -602,6 +602,14 @@  inject_lost_extent(
 	return -libxfs_trans_commit(tp);
 }
 
+void
+check_rtmetadata(
+	struct xfs_mount	*mp)
+{
+	rtinit(mp);
+	generate_rtinfo(mp, btmcompute, sumcompute);
+}
+
 void
 phase5(xfs_mount_t *mp)
 {
@@ -671,8 +679,7 @@  phase5(xfs_mount_t *mp)
 	if (mp->m_sb.sb_rblocks)  {
 		do_log(
 		_("        - generate realtime summary info and bitmap...\n"));
-		rtinit(mp);
-		generate_rtinfo(mp, btmcompute, sumcompute);
+		check_rtmetadata(mp);
 	}
 
 	do_log(_("        - reset superblock...\n"));
diff --git a/repair/protos.h b/repair/protos.h
index 83734e85..03ebae14 100644
--- a/repair/protos.h
+++ b/repair/protos.h
@@ -36,6 +36,7 @@  void	phase1(struct xfs_mount *);
 void	phase2(struct xfs_mount *, int);
 void	phase3(struct xfs_mount *, int);
 void	phase4(struct xfs_mount *);
+void	check_rtmetadata(struct xfs_mount *mp);
 void	phase5(struct xfs_mount *);
 void	phase6(struct xfs_mount *);
 void	phase7(struct xfs_mount *, int);
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..d08b0cec 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -1174,9 +1174,12 @@  main(int argc, char **argv)
 	phase4(mp);
 	phase_end(4);
 
-	if (no_modify)
+	if (no_modify) {
 		printf(_("No modify flag set, skipping phase 5\n"));
-	else {
+
+		if (mp->m_sb.sb_rblocks > 0)
+			check_rtmetadata(mp);
+	} else {
 		phase5(mp);
 	}
 	phase_end(5);