Re: [PATCH v7 18/19] xfs: Add remote block helper functions

Chandan Rajendra
2020-03-04T04:56:58Z

On Sunday, February 23, 2020 7:36 AM 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 redunant code
> associated with storing and retrieving remote blocks during the attr set operations.
>

The changes look good to me,

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr.c | 48 ++++++++++++++++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index b9728d1..f88be36 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -786,6 +786,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_store_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
> *
> @@ -824,11 +848,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_store_rmt_blk(args);
>
> /*
> * clear the remote attr state now that it is saved so that the
> @@ -939,11 +959,7 @@ 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);
>
> args->dac.dela_state = XFS_DAS_FLIP_LFLAG;
> return -EAGAIN;
> @@ -1189,11 +1205,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_store_rmt_blk(args);
>
> /*
> * clear the remote attr state now that it is saved so that the
> @@ -1306,11 +1318,7 @@ 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);
>
> /*
> * Commit the flag value change and start the next trans in
>

--
chandan