Re: [PATCH 2/2] xfs: hoist xfs_scrub_agfl_walk to libxfs as xfs_agfl_walk

[Dave Chinner <david@fromorbit.com>]

On Mon, Apr 02, 2018 at 12:55:42PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This function is basically a generic AGFL block iterator, so promote it
> to libxfs ahead of online repair wanting to use it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_alloc.c |   37 +++++++++++++++++++++
>  fs/xfs/libxfs/xfs_alloc.h |    5 +++
>  fs/xfs/scrub/agheader.c   |   78 ++++++++-------------------------------------
>  fs/xfs/scrub/common.h     |    4 --
>  4 files changed, 55 insertions(+), 69 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 4c03ec2..d860116 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3128,3 +3128,40 @@ xfs_alloc_has_record(
>  
>  	return xfs_btree_has_record(cur, &low, &high, exists);
>  }
> +
> +/*
> + * Walk all the blocks in the AGFL.  The fn function can return any negative
> + * error code or XFS_BTREE_QUERY_RANGE_ABORT.
> + */
> +int
> +xfs_agfl_walk(
> +	struct xfs_mount	*mp,
> +	struct xfs_agf		*agf,
> +	struct xfs_buf		*agflbp,
> +	xfs_agfl_walk_fn	fn,

	xfs_agfl_walk_fn	walk_fn,

I don't really like "fn" as a variable - it's hard to pick out what
it does and where it's called in the code.

Also, the comment would make more sense as "The @walk_fn can return
any negative....

Otherwise looks OK.

-Dave.

-- 
Dave Chinner
david@fromorbit.com