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

[Dave Chinner <david@fromorbit.com>]

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@lst.de>

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@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs