[Cluster-devel] [GFS2 PATCH] GFS2: combine gfs2_alloc_block and gfs2_alloc_di

[Steven Whitehouse]

Hi,

On Wed, 2011-11-16 at 14:59 -0500, Bob Peterson wrote:
> ----- Original Message -----
> | Hi,
> | 
> | As a follow up to this patch, I'd quite like to see rgblk_search
> | split
> | into two functions. One to do the search and another to actually make
> | changes to the bitmap when a block is found. That should help
> | development of further alloc changes on top.
> 
> Hi,
> 
> So something like the patch that follows?
> 
> This patch splits function rgblk_search into two functions:
> the finding and the bit setting function, now called gfs2_alloc_extent.
> 
Yes, but lets make gfs2_alloc_extent() a totally separate function and
not call it from rgblk_search, that way when looking for unlinked inodes
we don't need to care about gfs2_alloc_extent at all. The only issue is
passing a couple of things from rgblk_search to gfs2_alloc_extent (the
bi (either by index or by pointer) and the current position in the
bitmap.

However my thought was to separate these two completely rather than
leaving one calling the other,

Steve.

> Regards,
> 
> Bob Peterson
> Red Hat File Systems
> --
>  fs/gfs2/rgrp.c |   69 ++++++++++++++++++++++++++++++++-----------------------
>  1 files changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 131289b..f67610e 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1109,6 +1109,44 @@ static unsigned char gfs2_get_block_type(struct gfs2_rgrpd *rgd, u64 block)
>  }
>  
>  /**
> + * gfs2_alloc_extent - allocate an extent from a given bitmap
> + *
> + * @rgd: the resource group descriptor
> + * @bi: the bitmap within the rgrp
> + * blk: the block within the RG
> + * dinode: TRUE if the first block we allocate is for a dinode
> + * @n: The extent length
> + */
> +static void gfs2_alloc_extent(struct gfs2_rgrpd *rgd, struct gfs2_bitmap *bi,
> +			      u32 blk, bool dinode, unsigned int *n)
> +{
> +	unsigned char new_state = dinode ? GFS2_BLKST_DINODE : GFS2_BLKST_USED;
> +	const unsigned int elen = *n;
> +	u32 goal;
> +	const u8 *buffer = NULL;
> +
> +	buffer = bi->bi_bh->b_data + bi->bi_offset;
> +	gfs2_trans_add_bh(rgd->rd_gl, bi->bi_bh, 1);
> +	gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> +		    bi, blk, new_state);
> +	if (new_state == GFS2_BLKST_USED)
> +		*n = 1;
> +	new_state = GFS2_BLKST_USED; /* for extents, we need data blocks */
> +	goal = blk;
> +	while (*n < elen) {
> +		goal++;
> +		if (goal >= (bi->bi_len * GFS2_NBBY))
> +			break;
> +		if (gfs2_testbit(rgd, buffer, bi->bi_len, goal) !=
> +		    GFS2_BLKST_FREE)
> +			break;
> +		gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> +			    bi, goal, new_state);
> +		(*n)++;
> +	}
> +}
> +
> +/**
>   * rgblk_search - find a block in @old_state, change allocation
>   *           state to @new_state
>   * @rgd: the resource group descriptor
> @@ -1137,16 +1175,8 @@ static u32 rgblk_search(struct gfs2_rgrpd *rgd, u32 goal,
>  	const u32 length = rgd->rd_length;
>  	u32 blk = BFITNOENT;
>  	unsigned int buf, x;
> -	const unsigned int elen = *n;
>  	const u8 *buffer = NULL;
> -	unsigned char new_state;
>  
> -	if (old_state == GFS2_BLKST_UNLINKED)
> -		new_state = GFS2_BLKST_UNLINKED;
> -	else if (dinode)
> -		new_state = GFS2_BLKST_DINODE;
> -	else
> -		new_state = GFS2_BLKST_USED;
>  	*n = 0;
>  	/* Find bitmap block that contains bits for goal block */
>  	for (buf = 0; buf < length; buf++) {
> @@ -1197,28 +1227,9 @@ skip:
>  	if (blk == BFITNOENT)
>  		return blk;
>  
> -	if (old_state == new_state)
> -		goto out;
> +	if (old_state != GFS2_BLKST_UNLINKED)
> +		gfs2_alloc_extent(rgd, bi, blk, dinode, n);
>  
> -	gfs2_trans_add_bh(rgd->rd_gl, bi->bi_bh, 1);
> -	gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> -		    bi, blk, new_state);
> -	if (new_state == GFS2_BLKST_USED)
> -		*n = 1;
> -	new_state = GFS2_BLKST_USED; /* for extents, we need data blocks */
> -	goal = blk;
> -	while (*n < elen) {
> -		goal++;
> -		if (goal >= (bi->bi_len * GFS2_NBBY))
> -			break;
> -		if (gfs2_testbit(rgd, buffer, bi->bi_len, goal) !=
> -		    GFS2_BLKST_FREE)
> -			break;
> -		gfs2_setbit(rgd, bi->bi_bh->b_data, bi->bi_clone, bi->bi_offset,
> -			    bi, goal, new_state);
> -		(*n)++;
> -	}
> -out:
>  	return (bi->bi_start * GFS2_NBBY) + blk;
>  }
>