Re: [PATCH v8 15/20] xfs: Add remote block helper functions

[Allison Collins <allison.henderson@xxxxxxxxxx>]

On 4/8/20 5:09 AM, Brian Foster wrote:
> On Fri, Apr 03, 2020 at 03:12:24PM -0700, Allison Collins wrote:
> > This patch adds two new helper functions xfs_attr_store_rmt_blk and
> > xfs_attr_restore_rmt_blk. These two helpers assist to remove redundant
> > code associated with storing and retrieving remote blocks during the
> > attr set operations.
> >
> > Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
> > Reviewed-by: Chandan Rajendra <chandanrlinux@xxxxxxxxx>
> > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
Alrighty, thank you!

Allison
> >   fs/xfs/libxfs/xfs_attr.c | 50 +++++++++++++++++++++++++++++-------------------
> >   1 file changed, 30 insertions(+), 20 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> > index 8d7a5db..f70b4f2 100644
> > --- a/fs/xfs/libxfs/xfs_attr.c
> > +++ b/fs/xfs/libxfs/xfs_attr.c
> > @@ -565,6 +565,30 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
> >    * External routines when attribute list is one block
> >    *========================================================================*/
> >   
> > +/* Store info about a remote block */
> > +STATIC void
> > +xfs_attr_save_rmt_blk(
> > +	struct xfs_da_args	*args)
> > +{
> > +	args->blkno2 = args->blkno;
> > +	args->index2 = args->index;
> > +	args->rmtblkno2 = args->rmtblkno;
> > +	args->rmtblkcnt2 = args->rmtblkcnt;
> > +	args->rmtvaluelen2 = args->rmtvaluelen;
> > +}
> > +
> > +/* Set stored info about a remote block */
> > +STATIC void
> > +xfs_attr_restore_rmt_blk(
> > +	struct xfs_da_args	*args)
> > +{
> > +	args->blkno = args->blkno2;
> > +	args->index = args->index2;
> > +	args->rmtblkno = args->rmtblkno2;
> > +	args->rmtblkcnt = args->rmtblkcnt2;
> > +	args->rmtvaluelen = args->rmtvaluelen2;
> > +}
> > +
> >   /*
> >    * Tries to add an attribute to an inode in leaf form
> >    *
> > @@ -599,11 +623,7 @@ xfs_attr_leaf_try_add(
> >   
> >   		/* save the attribute state for later removal*/
> >   		args->op_flags |= XFS_DA_OP_RENAME;	/* an atomic rename */
> > -		args->blkno2 = args->blkno;		/* set 2nd entry info*/
> > -		args->index2 = args->index;
> > -		args->rmtblkno2 = args->rmtblkno;
> > -		args->rmtblkcnt2 = args->rmtblkcnt;
> > -		args->rmtvaluelen2 = args->rmtvaluelen;
> > +		xfs_attr_save_rmt_blk(args);
> >   
> >   		/*
> >   		 * clear the remote attr state now that it is saved so that the
> > @@ -702,11 +722,8 @@ xfs_attr_leaf_addname(
> >   		 * Dismantle the "old" attribute/value pair by removing
> >   		 * a "remote" value (if it exists).
> >   		 */
> > -		args->index = args->index2;
> > -		args->blkno = args->blkno2;
> > -		args->rmtblkno = args->rmtblkno2;
> > -		args->rmtblkcnt = args->rmtblkcnt2;
> > -		args->rmtvaluelen = args->rmtvaluelen2;
> > +		xfs_attr_restore_rmt_blk(args);
> > +
> >   		if (args->rmtblkno) {
> >   			error = xfs_attr_rmtval_remove(args);
> >   			if (error)
> > @@ -934,11 +951,7 @@ xfs_attr_node_addname(
> >   
> >   		/* save the attribute state for later removal*/
> >   		args->op_flags |= XFS_DA_OP_RENAME;	/* atomic rename op */
> > -		args->blkno2 = args->blkno;		/* set 2nd entry info*/
> > -		args->index2 = args->index;
> > -		args->rmtblkno2 = args->rmtblkno;
> > -		args->rmtblkcnt2 = args->rmtblkcnt;
> > -		args->rmtvaluelen2 = args->rmtvaluelen;
> > +		xfs_attr_save_rmt_blk(args);
> >   
> >   		/*
> >   		 * clear the remote attr state now that it is saved so that the
> > @@ -1050,11 +1063,8 @@ xfs_attr_node_addname(
> >   		 * Dismantle the "old" attribute/value pair by removing
> >   		 * a "remote" value (if it exists).
> >   		 */
> > -		args->index = args->index2;
> > -		args->blkno = args->blkno2;
> > -		args->rmtblkno = args->rmtblkno2;
> > -		args->rmtblkcnt = args->rmtblkcnt2;
> > -		args->rmtvaluelen = args->rmtvaluelen2;
> > +		xfs_attr_restore_rmt_blk(args);
> > +
> >   		if (args->rmtblkno) {
> >   			error = xfs_attr_rmtval_remove(args);
> >   			if (error)
> > --
> > 2.7.4