On Tue, Sep 10, 2019 at 10:19:33AM +1000, Dave Chinner wrote:
> On Thu, Sep 05, 2019 at 08:33:47PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Refactor xfs_scrub_metadata into two functions -- one to make a single
> > call xfs_check_metadata, and the second retains the loop logic. The
> > name is a little easy to confuse with other functions, so rename it to
> > reflect what it actually does: scrub all internal metadata of a given
> > class (AG header, AG metadata, FS metadata). No functional changes.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Minor nit:
>
> > +/* Scrub non-inode metadata, saving corruption reports for later. */
> > +static int
> > +xfs_scrub_meta(
> > + struct scrub_ctx *ctx,
> > + unsigned int type,
> > + xfs_agnumber_t agno,
> > + struct xfs_action_list *alist)
> > +{
> > + struct xfs_scrub_metadata meta = {
> > + .sm_type = type,
> > + .sm_agno = agno,
> > + };
>
> This should be called xfs_scrub_meta_type() because it only
> scrubs the specific type passed into it....
Ok.
> > /* Scrub metadata, saving corruption reports for later. */
> > static bool
> > -xfs_scrub_metadata(
> > +xfs_scrub_meta_type(
> > struct scrub_ctx *ctx,
> > enum xfrog_scrub_type scrub_type,
> > xfs_agnumber_t agno,
> > struct xfs_action_list *alist)
> > {
> > - struct xfs_scrub_metadata meta = {0};
> > const struct xfrog_scrub_descr *sc;
> > - enum check_outcome fix;
> > - int type;
> > + unsigned int type;
> >
> > sc = xfrog_scrubbers;
> > for (type = 0; type < XFS_SCRUB_TYPE_NR; type++, sc++) {
> > + int ret;
> > +
>
> And this should be called xfs_scrub_all_metadata() because it
> walks across all the metadata types in the filesystem and calls
> xfs_scrub_meta_type() for each type to scrub them one by one....
Ok. I think I'll update the comments for both to make it clearer what
"type" means.
--D
> Other than that, it looks fine.
>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
