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.... > /* 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.... Other than that, it looks fine. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx