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

[Chandan Rajendra <chandan@linux.ibm.com>]

On Tuesday, February 25, 2020 9:41 PM Brian Foster wrote: 
> 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>

Thanks for the review. I will fix this up in the next iteration of the
patchset.

-- 
chandan