Re: [PATCH V4 RESEND 2/7] xfs: xfs_attr_calc_size: Use local variables to track individual space components

[Brian Foster <bfoster@redhat.com>]

On Mon, Feb 24, 2020 at 09:30:39AM +0530, Chandan Rajendra wrote:
> The size calculated by xfs_attr_calc_size() is a sum of three components,
> 1. Number of dabtree blocks
> 2. Number of Bmbt blocks
> 3. Number of remote blocks
> 
> This commit introduces new local variables to track these numbers explicitly.
> 
> Signed-off-by: Chandan Rajendra <chandanrlinux@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_attr.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 1875210cc8e40..942ba552e0bdd 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -142,8 +142,10 @@ xfs_attr_calc_size(
>  	int			*local)
>  {
>  	struct xfs_mount	*mp = args->dp->i_mount;
> +	unsigned int		total_dablks;
> +	unsigned int		bmbt_blks;
> +	unsigned int		rmt_blks;
>  	int			size;
> -	int			nblks;
>  
>  	/*
>  	 * Determine space new attribute will use, and if it would be
> @@ -151,23 +153,26 @@ xfs_attr_calc_size(
>  	 */
>  	size = xfs_attr_leaf_newentsize(args->geo, args->namelen,
>  			args->valuelen, local);
> -	nblks = XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK);
> +	total_dablks = XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK);
> +	bmbt_blks = XFS_DAENTER_BMAPS(mp, XFS_ATTR_FORK);
>  	if (*local) {
>  		if (size > (args->geo->blksize / 2)) {
>  			/* Double split possible */
> -			nblks *= 2;
> +			total_dablks *= 2;
> +			bmbt_blks *= 2;
>  		}
> +		rmt_blks = 0;

I'd just initialize this one to zero above. Otherwise looks fine:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  	} else {
>  		/*
>  		 * Out of line attribute, cannot double split, but
>  		 * make room for the attribute value itself.
>  		 */
> -		uint	dblocks = xfs_attr3_rmt_blocks(mp, args->valuelen);
> -		nblks += dblocks;
> -		nblks += XFS_NEXTENTADD_SPACE_RES(mp, dblocks, XFS_ATTR_FORK);
> +		rmt_blks = xfs_attr3_rmt_blocks(mp, args->valuelen);
> +		bmbt_blks += XFS_NEXTENTADD_SPACE_RES(mp, rmt_blks,
> +				XFS_ATTR_FORK);
>  	}
>  
> -	return nblks;
> +	return total_dablks + rmt_blks + bmbt_blks;
>  }
>  
>  STATIC int
> -- 
> 2.19.1
>