Re: [PATCH v9 12/24] xfs: Add helper function xfs_attr_node_shrink

[Allison Collins <allison.henderson@xxxxxxxxxx>]

On 5/4/20 10:42 AM, Darrick J. Wong wrote:
> On Thu, Apr 30, 2020 at 03:50:04PM -0700, Allison Collins wrote:
> > This patch adds a new helper function xfs_attr_node_shrink used to
> > shrink an attr name into an inode if it is small enough.  This helps to
> > modularize the greater calling function xfs_attr_node_removename.
> >
> > Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
>
> Simple enough looking hoist;
> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Great, thanks!

Allison
> --D
>
> > ---
> >   fs/xfs/libxfs/xfs_attr.c | 68 ++++++++++++++++++++++++++++++------------------
> >   1 file changed, 42 insertions(+), 26 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> > index 4fdfab9..d83443c 100644
> > --- a/fs/xfs/libxfs/xfs_attr.c
> > +++ b/fs/xfs/libxfs/xfs_attr.c
> > @@ -1108,6 +1108,45 @@ xfs_attr_node_addname(
> >   }
> >   
> >   /*
> > + * Shrink an attribute from leaf to shortform
> > + */
> > +STATIC int
> > +xfs_attr_node_shrink(
> > +	struct xfs_da_args	*args,
> > +	struct xfs_da_state     *state)
> > +{
> > +	struct xfs_inode	*dp = args->dp;
> > +	int			error, forkoff;
> > +	struct xfs_buf		*bp;
> > +
> > +	/*
> > +	 * Have to get rid of the copy of this dabuf in the state.
> > +	 */
> > +	ASSERT(state->path.active == 1);
> > +	ASSERT(state->path.blk[0].bp);
> > +	state->path.blk[0].bp = NULL;
> > +
> > +	error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
> > +	if (error)
> > +		return error;
> > +
> > +	forkoff = xfs_attr_shortform_allfit(bp, dp);
> > +	if (forkoff) {
> > +		error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
> > +		/* bp is gone due to xfs_da_shrink_inode */
> > +		if (error)
> > +			return error;
> > +
> > +		error = xfs_defer_finish(&args->trans);
> > +		if (error)
> > +			return error;
> > +	} else
> > +		xfs_trans_brelse(args->trans, bp);
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> >    * Remove a name from a B-tree attribute list.
> >    *
> >    * This will involve walking down the Btree, and may involve joining
> > @@ -1120,8 +1159,7 @@ xfs_attr_node_removename(
> >   {
> >   	struct xfs_da_state	*state;
> >   	struct xfs_da_state_blk	*blk;
> > -	struct xfs_buf		*bp;
> > -	int			retval, error, forkoff;
> > +	int			retval, error;
> >   	struct xfs_inode	*dp = args->dp;
> >   
> >   	trace_xfs_attr_node_removename(args);
> > @@ -1206,30 +1244,8 @@ xfs_attr_node_removename(
> >   	/*
> >   	 * If the result is small enough, push it all into the inode.
> >   	 */
> > -	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
> > -		/*
> > -		 * Have to get rid of the copy of this dabuf in the state.
> > -		 */
> > -		ASSERT(state->path.active == 1);
> > -		ASSERT(state->path.blk[0].bp);
> > -		state->path.blk[0].bp = NULL;
> > -
> > -		error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
> > -		if (error)
> > -			goto out;
> > -
> > -		if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
> > -			error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
> > -			/* bp is gone due to xfs_da_shrink_inode */
> > -			if (error)
> > -				goto out;
> > -			error = xfs_defer_finish(&args->trans);
> > -			if (error)
> > -				goto out;
> > -		} else
> > -			xfs_trans_brelse(args->trans, bp);
> > -	}
> > -	error = 0;
> > +	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
> > +		error = xfs_attr_node_shrink(args, state);
> >   
> >   out:
> >   	if (state)
> > --
> > 2.7.4