Re: [PATCH v13 09/10] xfs: Remove unused xfs_attr_*_args

Darrick J. Wong
2020-11-10T20:07:07Z

On Thu, Oct 22, 2020 at 11:34:34PM -0700, Allison Henderson wrote:
> Remove xfs_attr_set_args, xfs_attr_remove_args, and xfs_attr_trans_roll.
> These high level loops are now driven by the delayed operations code,
> and can be removed.
>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr.c | 97 +----------------------------------------
> fs/xfs/libxfs/xfs_attr.h | 9 ++--
> fs/xfs/libxfs/xfs_attr_remote.c | 4 +-
> 3 files changed, 5 insertions(+), 105 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index edd5d10..b5e1e84 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -262,65 +262,6 @@ xfs_attr_set_shortform(
> }
>
> /*
> - * Checks to see if a delayed attribute transaction should be rolled. If so,
> - * also checks for a defer finish. Transaction is finished and rolled as
> - * needed, and returns true of false if the delayed operation should continue.
> - */
> -STATIC int
> -xfs_attr_trans_roll(
> - struct xfs_delattr_context *dac)
> -{
> - struct xfs_da_args *args = dac->da_args;
> - int error = 0;
> -
> - if (dac->flags & XFS_DAC_DEFER_FINISH) {
> - /*
> - * The caller wants us to finish all the deferred ops so that we
> - * avoid pinning the log tail with a large number of deferred
> - * ops.
> - */
> - dac->flags &= ~XFS_DAC_DEFER_FINISH;
> - error = xfs_defer_finish(&args->trans);
> - if (error)
> - return error;
> - }
> -
> - return xfs_trans_roll_inode(&args->trans, args->dp);
> -}
> -
> -/*
> - * Set the attribute specified in @args.
> - */
> -int
> -xfs_attr_set_args(
> - struct xfs_da_args *args)
> -{
> - struct xfs_buf *leaf_bp = NULL;
> - int error = 0;
> - struct xfs_delattr_context dac = {
> - .da_args = args,
> - };
> -
> - do {
> - error = xfs_attr_set_iter(&dac, &leaf_bp);

Now that there's only one caller of xfs_attr_set_iter and it passes
&dac->leaf_bp, I think you can get rid of this second parameter, right?

It's nice to see so much code disappear now that we track attr
operations with deferred ops. Everything else looks ok here. :)

--D

> - if (error != -EAGAIN)
> - break;
> -
> - error = xfs_attr_trans_roll(&dac);
> - if (error)
> - return error;
> -
> - if (leaf_bp) {
> - xfs_trans_bjoin(args->trans, leaf_bp);
> - xfs_trans_bhold(args->trans, leaf_bp);
> - }
> -
> - } while (true);
> -
> - return error;
> -}
> -
> -/*
> * Set the attribute specified in @args.
> * This routine is meant to function as a delayed operation, and may return
> * -EAGAIN when the transaction needs to be rolled. Calling functions will need
> @@ -363,11 +304,7 @@ xfs_attr_set_iter(
> * continue. Otherwise, is it converted from shortform to leaf
> * and -EAGAIN is returned.
> */
> - error = xfs_attr_set_shortform(args, leaf_bp);
> - if (error == -EAGAIN)
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> -
> - return error;
> + return xfs_attr_set_shortform(args, leaf_bp);
> }
>
> /*
> @@ -398,7 +335,6 @@ xfs_attr_set_iter(
> * same state (inode locked and joined, transaction
> * clean) no matter how we got to this step.
> */
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> return -EAGAIN;
> case 0:
> dac->dela_state = XFS_DAS_FOUND_LBLK;
> @@ -455,32 +391,6 @@ xfs_has_attr(
>
> /*
> * Remove the attribute specified in @args.
> - */
> -int
> -xfs_attr_remove_args(
> - struct xfs_da_args *args)
> -{
> - int error = 0;
> - struct xfs_delattr_context dac = {
> - .da_args = args,
> - };
> -
> - do {
> - error = xfs_attr_remove_iter(&dac);
> - if (error != -EAGAIN)
> - break;
> -
> - error = xfs_attr_trans_roll(&dac);
> - if (error)
> - return error;
> -
> - } while (true);
> -
> - return error;
> -}
> -
> -/*
> - * Remove the attribute specified in @args.
> *
> * This function may return -EAGAIN to signal that the transaction needs to be
> * rolled. Callers should continue calling this function until they receive a
> @@ -895,7 +805,6 @@ xfs_attr_leaf_addname(
> if (error)
> return error;
>
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> return -EAGAIN;
> }
>
> @@ -1192,7 +1101,6 @@ xfs_attr_node_addname(
> * Restart routine from the top. No need to set the
> * state
> */
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> return -EAGAIN;
> }
>
> @@ -1205,7 +1113,6 @@ xfs_attr_node_addname(
> error = xfs_da3_split(state);
> if (error)
> goto out;
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> } else {
> /*
> * Addition succeeded, update Btree hashvals.
> @@ -1246,7 +1153,6 @@ xfs_attr_node_addname(
> if (error)
> return error;
>
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> dac->dela_state = XFS_DAS_ALLOC_NODE;
> return -EAGAIN;
> }
> @@ -1516,7 +1422,6 @@ xfs_attr_node_remove_step(
> if (error)
> return error;
>
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> dac->dela_state = XFS_DAS_RM_SHRINK;
> return -EAGAIN;
> }
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 8a08411..6d90301 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -244,10 +244,9 @@ enum xfs_delattr_state {
> /*
> * Defines for xfs_delattr_context.flags
> */
> -#define XFS_DAC_DEFER_FINISH 0x01 /* finish the transaction */
> -#define XFS_DAC_NODE_RMVNAME_INIT 0x02 /* xfs_attr_node_removename init */
> -#define XFS_DAC_LEAF_ADDNAME_INIT 0x04 /* xfs_attr_leaf_addname init*/
> -#define XFS_DAC_DELAYED_OP_INIT 0x08 /* delayed operations init*/
> +#define XFS_DAC_NODE_RMVNAME_INIT 0x01 /* xfs_attr_node_removename init */
> +#define XFS_DAC_LEAF_ADDNAME_INIT 0x02 /* xfs_attr_leaf_addname init*/
> +#define XFS_DAC_DELAYED_OP_INIT 0x04 /* delayed operations init*/
>
> /*
> * Context used for keeping track of delayed attribute operations
> @@ -297,11 +296,9 @@ int xfs_inode_hasattr(struct xfs_inode *ip);
> int xfs_attr_get_ilocked(struct xfs_da_args *args);
> int xfs_attr_get(struct xfs_da_args *args);
> int xfs_attr_set(struct xfs_da_args *args);
> -int xfs_attr_set_args(struct xfs_da_args *args);
> int xfs_attr_set_iter(struct xfs_delattr_context *dac,
> struct xfs_buf **leaf_bp);
> int xfs_has_attr(struct xfs_da_args *args);
> -int xfs_attr_remove_args(struct xfs_da_args *args);
> int xfs_attr_remove_iter(struct xfs_delattr_context *dac);
> bool xfs_attr_namecheck(const void *name, size_t length);
> void xfs_delattr_context_init(struct xfs_delattr_context *dac,
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index 45c4bc5..262d1870 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -751,10 +751,8 @@ xfs_attr_rmtval_remove(
> if (error)
> return error;
>
> - if (!done) {
> - dac->flags |= XFS_DAC_DEFER_FINISH;
> + if (!done)
> return -EAGAIN;
> - }
>
> return error;
> }
> --
> 2.7.4
>