| Message ID | 20231109160233.703566-1-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | repair: fix the call to search_rt_dup_extent in scan_bmapbt | expand |
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 >
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.
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)++;
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(-)