[PATCH] repair: fix the call to search_rt_dup_extent in scan_bmapbt

[PATCH] repair: fix the call to search_rt_dup_extent in scan_bmapbt

[Christoph Hellwig]

search_rt_dup_extent expects an RT extent number and not a fsbno.
Convert the units before the call.  Without this we are unlikely
to ever found a legit duplicate extent on the RT subvolume because
the search will always be off the end.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 repair/scan.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/repair/scan.c b/repair/scan.c
index 27a33286a..7a0587615 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -402,8 +402,10 @@ _("bad state %d, inode %" PRIu64 " bmap block 0x%" PRIx64 "\n"),
 					XFS_FSB_TO_AGBNO(mp, bno) + 1))
 				return(1);
 		} else  {
-			if (search_rt_dup_extent(mp, bno))
-				return(1);
+			xfs_rtblock_t	ext = bno / mp->m_sb.sb_rextsize;
+
+			if (search_rt_dup_extent(mp, ext))
+				return 1;
 		}
 	}
 	(*tot)++;
-- 
2.39.2

Re: [PATCH] repair: fix the call to search_rt_dup_extent in scan_bmapbt

[Darrick J. Wong]

On Thu, Nov 09, 2023 at 05:02:33PM +0100, Christoph Hellwig wrote:
> search_rt_dup_extent expects an RT extent number and not a fsbno.
> Convert the units before the call.  Without this we are unlikely
> to ever found a legit duplicate extent on the RT subvolume because
> the search will always be off the end.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks familiar! :)

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

In the longer run: whenever the libxfs 6.7 sync hits the list, I'll be
ready to go with a pair of broader patches to fix all the confusing /
incorrect units and variable names in xfs_repair.  This ought to get
merged to xfsprogs 6.6.

--D

> ---
>  repair/scan.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/repair/scan.c b/repair/scan.c
> index 27a33286a..7a0587615 100644
> --- a/repair/scan.c
> +++ b/repair/scan.c
> @@ -402,8 +402,10 @@ _("bad state %d, inode %" PRIu64 " bmap block 0x%" PRIx64 "\n"),
>  					XFS_FSB_TO_AGBNO(mp, bno) + 1))
>  				return(1);
>  		} else  {
> -			if (search_rt_dup_extent(mp, bno))
> -				return(1);
> +			xfs_rtblock_t	ext = bno / mp->m_sb.sb_rextsize;
> +
> +			if (search_rt_dup_extent(mp, ext))
> +				return 1;
>  		}
>  	}
>  	(*tot)++;
> -- 
> 2.39.2
> 

Re: [PATCH] repair: fix the call to search_rt_dup_extent in scan_bmapbt

[Christoph Hellwig]

On Thu, Nov 09, 2023 at 08:13:19AM -0800, Darrick J. Wong wrote:
> In the longer run: whenever the libxfs 6.7 sync hits the list, I'll be
> ready to go with a pair of broader patches to fix all the confusing /
> incorrect units and variable names in xfs_repair.  This ought to get
> merged to xfsprogs 6.6.

Yes, please!

FYI, I've done a prototype of annotation the rxtnumber_t with __nocast
in the kernel, and except for the ugliness in casting all the 0 values
it actually looks pretty nice.  I hope with the extra work Luc promised
we can actually annotate our non-byte offset/bno/len fields with it and
get real type checking for them.