On Thu, Oct 27, 2022 at 10:14:53AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > Now that we've separated the startblock and CoW/shared extent domain in > the incore refcount record structure, check the domain whenever we > retrieve a record to ensure that it's still in the domain that we want. > Depending on the circumstances, a change in domain either means we're > done processing or that we've found a corruption and need to fail out. > > The refcount check in xchk_xref_is_cow_staging is redundant since > _get_rec has done that for a long time now, so we can get rid of it. > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > --- > fs/xfs/libxfs/xfs_refcount.c | 53 ++++++++++++++++++++++++++++++++---------- > fs/xfs/scrub/refcount.c | 4 ++- > 2 files changed, 43 insertions(+), 14 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c > index 3b1cb0578770..608a122eef16 100644 > --- a/fs/xfs/libxfs/xfs_refcount.c > +++ b/fs/xfs/libxfs/xfs_refcount.c > @@ -386,6 +386,8 @@ xfs_refcount_split_extent( > goto out_error; > } > > + if (rcext.rc_domain != domain) > + return 0; > if (rcext.rc_startblock == agbno || xfs_refc_next(&rcext) <= agbno) > return 0; > > @@ -434,6 +436,9 @@ xfs_refcount_merge_center_extents( > int error; > int found_rec; > > + ASSERT(left->rc_domain == center->rc_domain); > + ASSERT(right->rc_domain == center->rc_domain); > + > trace_xfs_refcount_merge_center_extents(cur->bc_mp, > cur->bc_ag.pag->pag_agno, left, center, right); Can you move the asserts to after the trace points? That way we if need to debug the assert, we'll have a tracepoint with the record information that triggered the assert emitted immediately before it fires... > > @@ -510,6 +515,8 @@ xfs_refcount_merge_left_extent( > int error; > int found_rec; > > + ASSERT(left->rc_domain == cleft->rc_domain); > + > trace_xfs_refcount_merge_left_extent(cur->bc_mp, > cur->bc_ag.pag->pag_agno, left, cleft); > > @@ -571,6 +578,8 @@ xfs_refcount_merge_right_extent( > int error; > int found_rec; > > + ASSERT(right->rc_domain == cright->rc_domain); > + > trace_xfs_refcount_merge_right_extent(cur->bc_mp, > cur->bc_ag.pag->pag_agno, cright, right); > > @@ -654,12 +663,10 @@ xfs_refcount_find_left_extents( > goto out_error; > } > > + if (tmp.rc_domain != domain) > + return 0; > if (xfs_refc_next(&tmp) != agbno) > return 0; > - if (domain == XFS_REFC_DOMAIN_SHARED && tmp.rc_refcount < 2) > - return 0; > - if (domain == XFS_REFC_DOMAIN_COW && tmp.rc_refcount > 1) > - return 0; Ah, as these go away, you can ignore my comment about them in the previous patches... :) Otherwise, looks ok. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> -- Dave Chinner david@xxxxxxxxxxxxx
