On Fri, Oct 28, 2022 at 08:15:31AM +1100, Dave Chinner wrote: > 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... Done. > > > > @@ -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> Cool, thanks! --D > -- > Dave Chinner > david@xxxxxxxxxxxxx
