On Mon, Nov 21, 2022 at 09:05:16AM -0800, Darrick J. Wong wrote:
> On Fri, Nov 18, 2022 at 11:17:14AM +0100, Carlos Maiolino wrote:
> > Hi Darrick.
> >
> > > diff --git a/repair/rmap.c b/repair/rmap.c
> > > index a7c4b25b1f..2c809fd4f2 100644
> > > --- a/repair/rmap.c
> > > +++ b/repair/rmap.c
> > > @@ -734,6 +734,8 @@ refcount_emit(
> > > rlrec.rc_startblock = agbno;
> > > rlrec.rc_blockcount = len;
> > > rlrec.rc_refcount = REFCOUNT_CLAMP(nr_rmaps);
> > > + rlrec.rc_domain = XFS_REFC_DOMAIN_SHARED;
> > > +
> > > error = slab_add(rlslab, &rlrec);
> > > if (error)
> > > do_error(
> > > @@ -1393,7 +1395,8 @@ check_refcounts(
> > > while (rl_rec) {
> > > /* Look for a refcount record in the btree */
> > > error = -libxfs_refcount_lookup_le(bt_cur,
> > > - rl_rec->rc_startblock, &have);
> > > + XFS_REFC_DOMAIN_SHARED, rl_rec->rc_startblock,
> > > + &have);
> >
> > Out of curiosity, why did you pass XFS_REFC_DOMAIN_SHARED directly here, other
> > than just rl_rec->rc_domain?
>
> I did that to make it more directly obvious that we're comparing ondisk
> records for shared (and not cow) blocks. One could trace
> rl_rec->rc_domain from refcount_emit() all the way through to here, but
> that's more work.
Seconded, thanks for the heads up.
>
> --D
>
> >
> >
> > Cheers.
> >
> > > if (error) {
> > > do_warn(
> > > _("Could not read reference count record for (%u/%u).\n"),
> > > @@ -1424,14 +1427,21 @@ _("Missing reference count record for (%u/%u) len %u count %u\n"),
> > > }
> > >
> > > /* Compare each refcount observation against the btree's */
> > > - if (tmp.rc_startblock != rl_rec->rc_startblock ||
> > > + if (tmp.rc_domain != rl_rec->rc_domain ||
> > > + tmp.rc_startblock != rl_rec->rc_startblock ||
> > > tmp.rc_blockcount != rl_rec->rc_blockcount ||
> > > - tmp.rc_refcount != rl_rec->rc_refcount)
> > > + tmp.rc_refcount != rl_rec->rc_refcount) {
> > > + unsigned int start;
> > > +
> > > + start = xfs_refcount_encode_startblock(
> > > + tmp.rc_startblock, tmp.rc_domain);
> > > +
> > > do_warn(
> > > _("Incorrect reference count: saw (%u/%u) len %u nlinks %u; should be (%u/%u) len %u nlinks %u\n"),
> > > - agno, tmp.rc_startblock, tmp.rc_blockcount,
> > > + agno, start, tmp.rc_blockcount,
> > > tmp.rc_refcount, agno, rl_rec->rc_startblock,
> > > rl_rec->rc_blockcount, rl_rec->rc_refcount);
> > > + }
> > > next_loop:
> > > rl_rec = pop_slab_cursor(rl_cur);
> > > }
> > >
> >
> > --
> > Carlos Maiolino
--
Carlos Maiolino
