On Sun, Feb 23, 2020 at 4:07 AM Allison Collins <allison.henderson@xxxxxxxxxx> 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 Typo: redunant > associated with storing and retrieving remote blocks during the attr set operations. > > Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx> > --- > 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); I believe the common naming pattern in the kernel for the helper to match with xx_restore of state is xxx_save (and not xxx_store). Otherwise, cleanup looks good and doesn't change logic. Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx> Thanks, Amir.