[Cluster-devel] [PATCH 10/11] gfs2: Pass resource group to rgblk_free

[Steven Whitehouse <swhiteho@redhat.com>]

On 05/10/18 20:18, Andreas Gruenbacher wrote:
> Function rgblk_free can only deal with one resource group at a time, so
> pass that resource group is as a parameter.  Several of the callers
> already have the resource group at hand, so we only need additional
> lookup code in a few places.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
That looks like a good optimisation,

Steve.

> ---
>   fs/gfs2/bmap.c  |  4 ++--
>   fs/gfs2/dir.c   |  5 ++++-
>   fs/gfs2/rgrp.c  | 42 +++++++++++++++---------------------------
>   fs/gfs2/rgrp.h  |  6 ++++--
>   fs/gfs2/xattr.c | 16 +++++++++-------
>   5 files changed, 34 insertions(+), 39 deletions(-)
>
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index c192906bb5f6..55e8ad1a6e13 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -1566,7 +1566,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
>   			continue;
>   		}
>   		if (bstart) {
> -			__gfs2_free_blocks(ip, bstart, (u32)blen, meta);
> +			__gfs2_free_blocks(ip, rgd, bstart, (u32)blen, meta);
>   			(*btotal) += blen;
>   			gfs2_add_inode_blocks(&ip->i_inode, -blen);
>   		}
> @@ -1574,7 +1574,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
>   		blen = 1;
>   	}
>   	if (bstart) {
> -		__gfs2_free_blocks(ip, bstart, (u32)blen, meta);
> +		__gfs2_free_blocks(ip, rgd, bstart, (u32)blen, meta);
>   		(*btotal) += blen;
>   		gfs2_add_inode_blocks(&ip->i_inode, -blen);
>   	}
> diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> index 89c601e5e52f..f9c6c7ee89e1 100644
> --- a/fs/gfs2/dir.c
> +++ b/fs/gfs2/dir.c
> @@ -2039,6 +2039,8 @@ static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
>   	bh = leaf_bh;
>   
>   	for (blk = leaf_no; blk; blk = nblk) {
> +		struct gfs2_rgrpd *rgd;
> +
>   		if (blk != leaf_no) {
>   			error = get_leaf(dip, blk, &bh);
>   			if (error)
> @@ -2049,7 +2051,8 @@ static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
>   		if (blk != leaf_no)
>   			brelse(bh);
>   
> -		gfs2_free_meta(dip, blk, 1);
> +		rgd = gfs2_blk2rgrpd(sdp, blk, true);
> +		gfs2_free_meta(dip, rgd, blk, 1);
>   		gfs2_add_inode_blocks(&dip->i_inode, -1);
>   	}
>   
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 76a0a8073c11..8a6b41f3667c 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -2245,26 +2245,19 @@ static void gfs2_alloc_extent(const struct gfs2_rbm *rbm, bool dinode,
>   /**
>    * rgblk_free - Change alloc state of given block(s)
>    * @sdp: the filesystem
> + * @rgd: the resource group the blocks are in
>    * @bstart: the start of a run of blocks to free
>    * @blen: the length of the block run (all must lie within ONE RG!)
>    * @new_state: GFS2_BLKST_XXX the after-allocation block state
> - *
> - * Returns:  Resource group containing the block(s)
>    */
>   
> -static struct gfs2_rgrpd *rgblk_free(struct gfs2_sbd *sdp, u64 bstart,
> -				     u32 blen, unsigned char new_state)
> +static void rgblk_free(struct gfs2_sbd *sdp, struct gfs2_rgrpd *rgd,
> +		       u64 bstart, u32 blen, unsigned char new_state)
>   {
>   	struct gfs2_rbm rbm;
>   	struct gfs2_bitmap *bi, *bi_prev = NULL;
>   
> -	rbm.rgd = gfs2_blk2rgrpd(sdp, bstart, 1);
> -	if (!rbm.rgd) {
> -		if (gfs2_consist(sdp))
> -			fs_err(sdp, "block = %llu\n", (unsigned long long)bstart);
> -		return NULL;
> -	}
> -
> +	rbm.rgd = rgd;
>   	BUG_ON(gfs2_rbm_from_block(&rbm, bstart));
>   	while (blen--) {
>   		bi = rbm_bi(&rbm);
> @@ -2282,8 +2275,6 @@ static struct gfs2_rgrpd *rgblk_free(struct gfs2_sbd *sdp, u64 bstart,
>   		gfs2_setbit(&rbm, false, new_state);
>   		gfs2_rbm_incr(&rbm);
>   	}
> -
> -	return rbm.rgd;
>   }
>   
>   /**
> @@ -2499,20 +2490,19 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
>   /**
>    * __gfs2_free_blocks - free a contiguous run of block(s)
>    * @ip: the inode these blocks are being freed from
> + * @rgd: the resource group the blocks are in
>    * @bstart: first block of a run of contiguous blocks
>    * @blen: the length of the block run
>    * @meta: 1 if the blocks represent metadata
>    *
>    */
>   
> -void __gfs2_free_blocks(struct gfs2_inode *ip, u64 bstart, u32 blen, int meta)
> +void __gfs2_free_blocks(struct gfs2_inode *ip, struct gfs2_rgrpd *rgd,
> +			u64 bstart, u32 blen, int meta)
>   {
>   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> -	struct gfs2_rgrpd *rgd;
>   
> -	rgd = rgblk_free(sdp, bstart, blen, GFS2_BLKST_FREE);
> -	if (!rgd)
> -		return;
> +	rgblk_free(sdp, rgd, bstart, blen, GFS2_BLKST_FREE);
>   	trace_gfs2_block_alloc(ip, rgd, bstart, blen, GFS2_BLKST_FREE);
>   	rgd->rd_free += blen;
>   	rgd->rd_flags &= ~GFS2_RGF_TRIMMED;
> @@ -2527,16 +2517,18 @@ void __gfs2_free_blocks(struct gfs2_inode *ip, u64 bstart, u32 blen, int meta)
>   /**
>    * gfs2_free_meta - free a contiguous run of data block(s)
>    * @ip: the inode these blocks are being freed from
> + * @rgd: the resource group the blocks are in
>    * @bstart: first block of a run of contiguous blocks
>    * @blen: the length of the block run
>    *
>    */
>   
> -void gfs2_free_meta(struct gfs2_inode *ip, u64 bstart, u32 blen)
> +void gfs2_free_meta(struct gfs2_inode *ip, struct gfs2_rgrpd *rgd,
> +		    u64 bstart, u32 blen)
>   {
>   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>   
> -	__gfs2_free_blocks(ip, bstart, blen, 1);
> +	__gfs2_free_blocks(ip, rgd, bstart, blen, 1);
>   	gfs2_statfs_change(sdp, 0, +blen, 0);
>   	gfs2_quota_change(ip, -(s64)blen, ip->i_inode.i_uid, ip->i_inode.i_gid);
>   }
> @@ -2548,9 +2540,10 @@ void gfs2_unlink_di(struct inode *inode)
>   	struct gfs2_rgrpd *rgd;
>   	u64 blkno = ip->i_no_addr;
>   
> -	rgd = rgblk_free(sdp, blkno, 1, GFS2_BLKST_UNLINKED);
> +	rgd = gfs2_blk2rgrpd(sdp, blkno, true);
>   	if (!rgd)
>   		return;
> +	rgblk_free(sdp, rgd, blkno, 1, GFS2_BLKST_UNLINKED);
>   	trace_gfs2_block_alloc(ip, rgd, blkno, 1, GFS2_BLKST_UNLINKED);
>   	gfs2_trans_add_meta(rgd->rd_gl, rgd->rd_bits[0].bi_bh);
>   	gfs2_rgrp_out(rgd, rgd->rd_bits[0].bi_bh->b_data);
> @@ -2560,13 +2553,8 @@ void gfs2_unlink_di(struct inode *inode)
>   void gfs2_free_di(struct gfs2_rgrpd *rgd, struct gfs2_inode *ip)
>   {
>   	struct gfs2_sbd *sdp = rgd->rd_sbd;
> -	struct gfs2_rgrpd *tmp_rgd;
> -
> -	tmp_rgd = rgblk_free(sdp, ip->i_no_addr, 1, GFS2_BLKST_FREE);
> -	if (!tmp_rgd)
> -		return;
> -	gfs2_assert_withdraw(sdp, rgd == tmp_rgd);
>   
> +	rgblk_free(sdp, rgd, ip->i_no_addr, 1, GFS2_BLKST_FREE);
>   	if (!rgd->rd_dinodes)
>   		gfs2_consist_rgrpd(rgd);
>   	rgd->rd_dinodes--;
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index 09519ae10fb6..b596c3d17988 100644
> --- a/fs/gfs2/rgrp.h
> +++ b/fs/gfs2/rgrp.h
> @@ -51,8 +51,10 @@ extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
>   extern int gfs2_rsqa_alloc(struct gfs2_inode *ip);
>   extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
>   extern void gfs2_rsqa_delete(struct gfs2_inode *ip, atomic_t *wcount);
> -extern void __gfs2_free_blocks(struct gfs2_inode *ip, u64 bstart, u32 blen, int meta);
> -extern void gfs2_free_meta(struct gfs2_inode *ip, u64 bstart, u32 blen);
> +extern void __gfs2_free_blocks(struct gfs2_inode *ip, struct gfs2_rgrpd *rgd,
> +			       u64 bstart, u32 blen, int meta);
> +extern void gfs2_free_meta(struct gfs2_inode *ip, struct gfs2_rgrpd *rgd,
> +			   u64 bstart, u32 blen);
>   extern void gfs2_free_di(struct gfs2_rgrpd *rgd, struct gfs2_inode *ip);
>   extern void gfs2_unlink_di(struct inode *inode);
>   extern int gfs2_check_blk_type(struct gfs2_sbd *sdp, u64 no_addr,
> diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
> index e11f77f080a0..996c915a9c97 100644
> --- a/fs/gfs2/xattr.c
> +++ b/fs/gfs2/xattr.c
> @@ -283,7 +283,7 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
>   			blen++;
>   		else {
>   			if (bstart)
> -				gfs2_free_meta(ip, bstart, blen);
> +				gfs2_free_meta(ip, rgd, bstart, blen);
>   			bstart = bn;
>   			blen = 1;
>   		}
> @@ -292,7 +292,7 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
>   		gfs2_add_inode_blocks(&ip->i_inode, -1);
>   	}
>   	if (bstart)
> -		gfs2_free_meta(ip, bstart, blen);
> +		gfs2_free_meta(ip, rgd, bstart, blen);
>   
>   	if (prev && !leave) {
>   		u32 len;
> @@ -1250,6 +1250,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
>   {
>   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>   	struct gfs2_rgrp_list rlist;
> +	struct gfs2_rgrpd *rgd;
>   	struct buffer_head *indbh, *dibh;
>   	__be64 *eablk, *end;
>   	unsigned int rg_blocks = 0;
> @@ -1302,8 +1303,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
>   	gfs2_rlist_alloc(&rlist);
>   
>   	for (x = 0; x < rlist.rl_rgrps; x++) {
> -		struct gfs2_rgrpd *rgd = gfs2_glock2rgrp(rlist.rl_ghs[x].gh_gl);
> -
> +		rgd = gfs2_glock2rgrp(rlist.rl_ghs[x].gh_gl);
>   		rg_blocks += rgd->rd_length;
>   	}
>   
> @@ -1320,6 +1320,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
>   
>   	eablk = (__be64 *)(indbh->b_data + sizeof(struct gfs2_meta_header));
>   	bstart = 0;
> +	rgd = NULL;
>   	blen = 0;
>   
>   	for (; eablk < end; eablk++) {
> @@ -1333,8 +1334,9 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
>   			blen++;
>   		else {
>   			if (bstart)
> -				gfs2_free_meta(ip, bstart, blen);
> +				gfs2_free_meta(ip, rgd, bstart, blen);
>   			bstart = bn;
> +			rgd = gfs2_blk2rgrpd(sdp, bstart, true);
>   			blen = 1;
>   		}
>   
> @@ -1342,7 +1344,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
>   		gfs2_add_inode_blocks(&ip->i_inode, -1);
>   	}
>   	if (bstart)
> -		gfs2_free_meta(ip, bstart, blen);
> +		gfs2_free_meta(ip, rgd, bstart, blen);
>   
>   	ip->i_diskflags &= ~GFS2_DIF_EA_INDIRECT;
>   
> @@ -1391,7 +1393,7 @@ static int ea_dealloc_block(struct gfs2_inode *ip)
>   	if (error)
>   		goto out_gunlock;
>   
> -	gfs2_free_meta(ip, ip->i_eattr, 1);
> +	gfs2_free_meta(ip, rgd, ip->i_eattr, 1);
>   
>   	ip->i_eattr = 0;
>   	gfs2_add_inode_blocks(&ip->i_inode, -1);