Re: [PATCH 4/5] xfs_scrub: separate internal metadata scrub functions

Dave Chinner
2019-09-10T00:19:38Z

On Thu, Sep 05, 2019 at 08:33:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> 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@oracle.com>

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@redhat.com>

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com