Re: [PATCH 2/4] xfs: don't track the AGFL buffer in the scrub AG context

[Dave Chinner <david@fromorbit.com>]

On Sun, Oct 02, 2022 at 11:19:48AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> While scrubbing an allocation group, we don't need to hold the AGFL
> buffer as part of the scrub context.  All that is necessary to lock an
> AG is to hold the AGI and AGF buffers, so fix all the existing users of
> the AGFL buffer to grab them only when necessary.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/scrub/agheader.c        |   47 +++++++++++++++++++++++++---------------
>  fs/xfs/scrub/agheader_repair.c |    1 -
>  fs/xfs/scrub/common.c          |    8 -------
>  fs/xfs/scrub/repair.c          |   11 +++++----
>  fs/xfs/scrub/scrub.h           |    1 -
>  5 files changed, 35 insertions(+), 33 deletions(-)

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

> @@ -717,24 +731,21 @@ xchk_agfl(
>  
>  	/* Allocate buffer to ensure uniqueness of AGFL entries. */
>  	agf = sc->sa.agf_bp->b_addr;
> -	agflcount = be32_to_cpu(agf->agf_flcount);
> -	if (agflcount > xfs_agfl_size(sc->mp)) {
> +	sai.agflcount = be32_to_cpu(agf->agf_flcount);
> +	if (sai.agflcount > xfs_agfl_size(sc->mp)) {
>  		xchk_block_set_corrupt(sc, sc->sa.agf_bp);
>  		goto out;
>  	}
> -	memset(&sai, 0, sizeof(sai));
> -	sai.sc = sc;
> -	sai.sz_entries = agflcount;
> -	sai.entries = kmem_zalloc(sizeof(xfs_agblock_t) * agflcount,
> -			KM_MAYFAIL);
> +	sai.entries = kvcalloc(sai.agflcount, sizeof(xfs_agblock_t),
> +			GFP_KERNEL | __GFP_RETRY_MAYFAIL);

The code is fine, but I'm curious why kvcalloc()? Are there really
devices out there with sector sizes large than 4kB that we support?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com