On Wed, Nov 08, 2023 at 06:53:20PM +0100, Christoph Hellwig wrote:
> search_rt_dup_extent takes a xfs_rtblock_t, not an RT extent number.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>
> What scares me about this is that no test seems to hit this and report
> false duplicates. I'll need to see if I can come up with an
> artifical reproducers of some kind.
I think you've misread the code -- phase 4 builds the rt_dup tree by
walks all the rtextents, and adding the duplicates:
for (rtx = 0; rtx < mp->m_sb.sb_rextents; rtx++) {
bstate = get_rtbmap(rtx);
switch (bstate) {
...
case XR_E_FS_MAP:
if (rt_start == 0)
continue;
else {
/*
* add extent and reset extent state
*/
add_rt_dup_extent(rt_start, rt_len);
rt_start = 0;
rt_len = 0;
}
break;
case XR_E_MULT:
if (rt_start == 0) {
rt_start = rtx;
rt_len = 1;
} else if (rt_len == XFS_MAX_BMBT_EXTLEN) {
/*
* large extent case
*/
add_rt_dup_extent(rt_start, rt_len);
rt_start = rtx;
rt_len = 1;
} else
rt_len++;
break;
So I think the reason why you've never seen false duplicates is that the
rt_dup tree intervals measure rt extents, not rt blocks. The units
conversion in process_rt_rec_dups is correct.
However, none of that is at all obvious because of the dual uses of
xfs_rtblock_t for rt blocks and rt extents.
I guess I ought to post the xfsprogs version of those rt units cleanups.
:)
--D
> repair/dinode.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/repair/dinode.c b/repair/dinode.c
> index c10dd1fa3..9aa367138 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -194,13 +194,11 @@ process_rt_rec_dups(
> struct xfs_bmbt_irec *irec)
> {
> xfs_fsblock_t b;
> - xfs_rtblock_t ext;
>
> for (b = rounddown(irec->br_startblock, mp->m_sb.sb_rextsize);
> b < irec->br_startblock + irec->br_blockcount;
> b += mp->m_sb.sb_rextsize) {
> - ext = (xfs_rtblock_t) b / mp->m_sb.sb_rextsize;
> - if (search_rt_dup_extent(mp, ext)) {
> + if (search_rt_dup_extent(mp, b)) {
> do_warn(
> _("data fork in rt ino %" PRIu64 " claims dup rt extent,"
> "off - %" PRIu64 ", start - %" PRIu64 ", count %" PRIu64 "\n"),
> --
> 2.39.2
>
