xfs
[Top] [All Lists]

Re: [PATCH 2/9] xfs: split xfs_bmap_btalloc_nullfb

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 2/9] xfs: split xfs_bmap_btalloc_nullfb
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 14 Apr 2014 11:55:44 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1397289723-26243-3-git-send-email-hch@xxxxxx>
References: <1397289723-26243-1-git-send-email-hch@xxxxxx> <1397289723-26243-3-git-send-email-hch@xxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Sat, Apr 12, 2014 at 10:01:56AM +0200, Christoph Hellwig wrote:
> Split xfs_bmap_btalloc_nullfb into one function for filestream allocations
> and one for everything else that share a few helpers.  This dramatically
> simplifies the control flow.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Small comment below.

> ---
>  fs/xfs/xfs_bmap.c |  200 
> +++++++++++++++++++++++++++++++----------------------
>  1 file changed, 117 insertions(+), 83 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
> index 3340f0e..d5d52df 100644
> --- a/fs/xfs/xfs_bmap.c
> +++ b/fs/xfs/xfs_bmap.c
> @@ -3517,6 +3517,67 @@ xfs_bmap_adjacent(
>  #undef ISVALID
>  }
>  
> +static int
> +xfs_bmap_longest_free_extent(
> +     struct xfs_trans        *tp,
> +     xfs_agnumber_t          ag,
> +     xfs_extlen_t            *blen,
> +     int                     *notinit)
> +{
> +     struct xfs_mount        *mp = tp->t_mountp;
> +     struct xfs_perag        *pag;
> +     xfs_extlen_t            longest;
> +     int                     error = 0;
> +
> +     pag = xfs_perag_get(mp, ag);
> +     if (!pag->pagf_init) {
> +             error = xfs_alloc_pagf_init(mp, tp, ag, XFS_ALLOC_FLAG_TRYLOCK);
> +             if (error)
> +                     goto out;
> +
> +             if (!pag->pagf_init) {
> +                     *notinit = 1;
> +                     goto out;
> +             }
> +     }
> +
> +     longest = xfs_alloc_longest_free_extent(mp, pag);
> +     if (*blen < longest)
> +             *blen = longest;
> +
> +out:
> +     xfs_perag_put(pag);
> +     return error;
> +}
> +
> +static void
> +xfs_bmap_fix_args(
> +     struct xfs_bmalloca     *ap,
> +     struct xfs_alloc_arg    *args,
> +     xfs_extlen_t            *blen,
> +     int                     notinit)

Not a big fan of the "fix_args" name here. What it's actually doing
is setting the minimum allocation length. Also, it could return the
minimum length rather than set it inside the function, and pass
blen by value rather than reference as it isn't modified in the
function...

Other than that, it's a good cleanup.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>