On Wed, Nov 29, 2023 at 08:53:19PM -0800, Christoph Hellwig wrote:
> On Fri, Nov 24, 2023 at 03:53:09PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Reintroduce to xrep_reap_extents the ability to reap extents from any
> > AG. We dropped this before because it was buggy, but in the next patch
> > we will gain the ability to reap old bmap btrees, which can have blocks
> > in any AG. To do this, we require that sc->sa is uninitialized, so that
> > we can use it to hold all the per-AG context for a given extent.
>
> Can you expand a bit on why it was buggy, in what commit is was dropped
> and what we're doing better this time around?
Oh! We merged that one! Let me change the commit message:
"Back in commit a55e07308831b ("xfs: only allow reaping of per-AG
blocks in xrep_reap_extents"), we removed from the reaping code the
ability to handle bmbt blocks. At the time, the reaping code only
walked single blocks, didn't correctly detect crosslinked blocks, and
the special casing made the function hard to understand. It was easier
to remove unneeded functionality prior to fixing all the bugs.
"Now that we've fixed the problems, we want again the ability to reap
file metadata blocks. Reintroduce the per-file reaping functionality
atop the current implementation. We require that sc->sa is
uninitialized, so that we can use it to hold all the per-AG context for
a given extent."
>
> >
> > #endif /* __XFS_SCRUB_REAP_H__ */
> > diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
> > index 70a6b18e5ad3c..46bf841524f8f 100644
> > --- a/fs/xfs/scrub/repair.h
> > +++ b/fs/xfs/scrub/repair.h
> > @@ -48,6 +48,7 @@ xrep_trans_commit(
> >
> > struct xbitmap;
> > struct xagb_bitmap;
> > +struct xfsb_bitmap;
>
> Your might need the forward declaration in reap.h, but definitively
> not here :)
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Thanks!
--D
