Re: [PATCH] xfs: split xfs_bmap_btalloc

[Dave Chinner <david@fromorbit.com>]

On Mon, Feb 15, 2010 at 06:34:42PM -0500, Christoph Hellwig wrote:
> Split out the nullfb case into a separate function to reduce the stack
> footprint and make the code more readable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I'm not sure this does reduce stack footprint - whenever we allocate
the first block of a file we hit the nullfb case, and this adds
another function call to that stack.

However, from the cleanup POV, I can't complain ;)

> 
> Index: xfs/fs/xfs/xfs_bmap.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_bmap.c	2010-01-17 14:46:43.326254284 +0100
> +++ xfs/fs/xfs/xfs_bmap.c	2010-01-17 15:04:47.016005768 +0100
> @@ -2550,22 +2550,134 @@ xfs_bmap_rtalloc(
>  }
>  
>  STATIC int
> +xfs_bmap_btalloc_nullfb(
> +	struct xfs_bmalloca	*ap,
> +	struct xfs_alloc_arg	*args,
> +	xfs_extlen_t		*blen)
> +{
> +	struct xfs_mount	*mp = ap->ip->i_mount;
> +	struct xfs_perag	*pag;
> +	xfs_agnumber_t		ag, startag;
> +	int			notinit = 0;
> +	int			error;
> +
> +	if (ap->userdata && xfs_inode_is_filestream(ap->ip))
> +		args->type = XFS_ALLOCTYPE_NEAR_BNO;
> +	else
> +		args->type = XFS_ALLOCTYPE_START_BNO;
> +	args->total = ap->total;
> +
> +	/*
> +	 * Search for an allocation group with a single extent large enough
> +	 * for the request.  If one isn't found, then adjust the minimum
> +	 * allocation size to the largest space found.
> +	 */
> +	startag = ag = XFS_FSB_TO_AGNO(mp, args->fsbno);
> +	if (startag == NULLAGNUMBER)
> +		startag = ag = 0;
> +
> +	pag = xfs_perag_get(mp, ag);
> +	while (*blen < ap->alen) {
> +		if (!pag->pagf_init) {
> +			error = xfs_alloc_pagf_init(mp, args->tp, ag,
> +						    XFS_ALLOC_FLAG_TRYLOCK);
> +			if (error) {
> +				xfs_perag_put(pag);
> +				return error;
> +			}
> +		}
> +
> +		/*
> +		 * See xfs_alloc_fix_freelist...
> +		 */

This comment is pretty much useless - if we are changing code then
I'd say kill it. Also the other comments in this function could be
reformatted as the indent has changed and so might be a bit more
readable with longer lines. This is not really that critical,
though, so it's good as it stands.

Reviewed-by: Dave Chinner <david@fromorbit.com>

-- 
Dave Chinner
david@fromorbit.com

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