Re: [PATCH v3 4/4] xfs: refactor successful AG allocation accounting code

Darrick J. Wong
2019-08-17T00:28:34Z

On Thu, Aug 15, 2019 at 08:55:38AM -0400, Brian Foster wrote:
> The higher level allocation code is unnecessarily split across
> xfs_alloc_ag_vextent() and xfs_alloc_ag_vextent_type(). In
> preparation for condensing this code, factor out the AG accounting
> bits and move the caller down after the generic allocation structure
> and function definitions to pick them up without the need for
> declarations. No functional changes.
>
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> fs/xfs/libxfs/xfs_alloc.c | 75 +++++++++++++++++++++++----------------
> 1 file changed, 45 insertions(+), 30 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index d550aa5597bf..4ae4cfa0ed7f 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1364,6 +1364,48 @@ xfs_alloc_ag_vextent_near(
> return error;
> }
>
> +/*
> + * Various AG accounting updates for a successful allocation. This includes
> + * updating the rmapbt, AG free block accounting and AG reservation accounting.
> + */
> +STATIC int
> +xfs_alloc_ag_vextent_accounting(
> + struct xfs_alloc_arg *args)
> +{
> + int error = 0;
> +
> + ASSERT(args->agbno != NULLAGBLOCK);
> + ASSERT(args->len >= args->minlen);
> + ASSERT(args->len <= args->maxlen);
> + ASSERT(!args->wasfromfl || args->resv != XFS_AG_RESV_AGFL);
> + ASSERT(args->agbno % args->alignment == 0);
> +
> + /* if not file data, insert new block into the reverse map btree */
> + if (!xfs_rmap_should_skip_owner_update(&args->oinfo)) {
> + error = xfs_rmap_alloc(args->tp, args->agbp, args->agno,
> + args->agbno, args->len, &args->oinfo);
> + if (error)
> + return error;
> + }
> +
> + if (!args->wasfromfl) {
> + error = xfs_alloc_update_counters(args->tp, args->pag,
> + args->agbp,
> + -((long)(args->len)));
> + if (error)
> + return error;
> +
> + ASSERT(!xfs_extent_busy_search(args->mp, args->agno,
> + args->agbno, args->len));
> + }
> +
> + xfs_ag_resv_alloc_extent(args->pag, args->resv, args);
> +
> + XFS_STATS_INC(args->mp, xs_allocx);
> + XFS_STATS_ADD(args->mp, xs_allocb, args->len);
> + return error;
> +}
> +
> /*
> * Allocate a variable extent in the allocation group agno.
> * Type and bno are used to determine where in the allocation group the
> @@ -1402,38 +1444,11 @@ xfs_alloc_ag_vextent(
> ASSERT(0);
> /* NOTREACHED */
> }
> -
> - if (error || args->agbno == NULLAGBLOCK)
> + if (error)
> return error;
>
> - ASSERT(args->len >= args->minlen);
> - ASSERT(args->len <= args->maxlen);
> - ASSERT(!args->wasfromfl || args->resv != XFS_AG_RESV_AGFL);
> - ASSERT(args->agbno % args->alignment == 0);
> -
> - /* if not file data, insert new block into the reverse map btree */
> - if (!xfs_rmap_should_skip_owner_update(&args->oinfo)) {
> - error = xfs_rmap_alloc(args->tp, args->agbp, args->agno,
> - args->agbno, args->len, &args->oinfo);
> - if (error)
> - return error;
> - }
> -
> - if (!args->wasfromfl) {
> - error = xfs_alloc_update_counters(args->tp, args->pag,
> - args->agbp,
> - -((long)(args->len)));
> - if (error)
> - return error;
> -
> - ASSERT(!xfs_extent_busy_search(args->mp, args->agno,
> - args->agbno, args->len));
> - }
> -
> - xfs_ag_resv_alloc_extent(args->pag, args->resv, args);
> -
> - XFS_STATS_INC(args->mp, xs_allocx);
> - XFS_STATS_ADD(args->mp, xs_allocb, args->len);
> + if (args->agbno != NULLAGBLOCK)
> + error = xfs_alloc_ag_vextent_accounting(args);
> return error;
> }
>
> --
> 2.20.1
>