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

[Dave Chinner <david@fromorbit.com>]

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