On Thu, 2022-04-14 at 19:44 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> In preparation for having a different replace algorithm when LARP
> mode is active.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks fine
Reviewed-by: Allison Henderson<allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 194 ++++++++++++++++++++++---------------
> --
> fs/xfs/libxfs/xfs_attr.h | 2 -
> 2 files changed, 112 insertions(+), 84 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 34c31077b08f..772506d44bfa 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -724,6 +724,96 @@ xfs_attr_lookup(
> return xfs_attr_node_hasname(args, NULL);
> }
>
> +static int
> +xfs_attr_item_init(
> + struct xfs_da_args *args,
> + unsigned int op_flags, /* op flag (set or
> remove) */
> + struct xfs_attr_item **attr) /* new xfs_attr_item
> */
> +{
> +
> + struct xfs_attr_item *new;
> +
> + new = kmem_zalloc(sizeof(struct xfs_attr_item), KM_NOFS);
> + new->xattri_op_flags = op_flags;
> + new->xattri_da_args = args;
> +
> + *attr = new;
> + return 0;
> +}
> +
> +/* Sets an attribute for an inode as a deferred operation */
> +static int
> +xfs_attr_defer_add(
> + struct xfs_da_args *args)
> +{
> + struct xfs_attr_item *new;
> + int error = 0;
> +
> + error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_SET, &new);
> + if (error)
> + return error;
> +
> + if (xfs_attr_is_shortform(args->dp))
> + new->xattri_dela_state = XFS_DAS_SF_ADD;
> + else if (xfs_attr_is_leaf(args->dp))
> + new->xattri_dela_state = XFS_DAS_LEAF_ADD;
> + else
> + new->xattri_dela_state = XFS_DAS_NODE_ADD;
> +
> + xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new-
> >xattri_list);
> +
> + return 0;
> +}
> +
> +/* Sets an attribute for an inode as a deferred operation */
> +static int
> +xfs_attr_defer_replace(
> + struct xfs_da_args *args)
> +{
> + struct xfs_attr_item *new;
> + int error = 0;
> +
> + error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_SET, &new);
> + if (error)
> + return error;
> +
> + if (xfs_attr_is_shortform(args->dp))
> + new->xattri_dela_state = XFS_DAS_SF_ADD;
> + else if (xfs_attr_is_leaf(args->dp))
> + new->xattri_dela_state = XFS_DAS_LEAF_ADD;
> + else
> + new->xattri_dela_state = XFS_DAS_NODE_ADD;
> +
> + xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new-
> >xattri_list);
> +
> + return 0;
> +}
> +
> +/* Removes an attribute for an inode as a deferred operation */
> +static int
> +xfs_attr_defer_remove(
> + struct xfs_da_args *args)
> +{
> +
> + struct xfs_attr_item *new;
> + int error;
> +
> + error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_REMOVE,
> &new);
> + if (error)
> + return error;
> +
> + if (xfs_attr_is_shortform(args->dp))
> + new->xattri_dela_state = XFS_DAS_SF_REMOVE;
> + else if (xfs_attr_is_leaf(args->dp))
> + new->xattri_dela_state = XFS_DAS_LEAF_REMOVE;
> + else
> + new->xattri_dela_state = XFS_DAS_NODE_REMOVE;
> +
> + xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new-
> >xattri_list);
> +
> + return 0;
> +}
> +
> /*
> * Note: If args->value is NULL the attribute will be removed, just
> like the
> * Linux ->setattr API.
> @@ -812,29 +902,35 @@ xfs_attr_set(
> }
>
> error = xfs_attr_lookup(args);
> - if (args->value) {
> - if (error == -EEXIST && (args->attr_flags &
> XATTR_CREATE))
> - goto out_trans_cancel;
> - if (error == -ENOATTR && (args->attr_flags &
> XATTR_REPLACE))
> - goto out_trans_cancel;
> - if (error != -ENOATTR && error != -EEXIST)
> + switch (error) {
> + case -EEXIST:
> + /* if no value, we are performing a remove operation */
> + if (!args->value) {
> + error = xfs_attr_defer_remove(args);
> + break;
> + }
> + /* Pure create fails if the attr already exists */
> + if (args->attr_flags & XATTR_CREATE)
> goto out_trans_cancel;
>
> - error = xfs_attr_set_deferred(args);
> - if (error)
> + error = xfs_attr_defer_replace(args);
> + break;
> + case -ENOATTR:
> + /* Can't remove what isn't there. */
> + if (!args->value)
> goto out_trans_cancel;
>
> - /* shortform attribute has already been committed */
> - if (!args->trans)
> - goto out_unlock;
> - } else {
> - if (error != -EEXIST)
> + /* Pure replace fails if no existing attr to replace.
> */
> + if (args->attr_flags & XATTR_REPLACE)
> goto out_trans_cancel;
>
> - error = xfs_attr_remove_deferred(args);
> - if (error)
> - goto out_trans_cancel;
> + error = xfs_attr_defer_add(args);
> + break;
> + default:
> + goto out_trans_cancel;
> }
> + if (error)
> + goto out_trans_cancel;
>
> /*
> * If this is a synchronous mount, make sure that the
> @@ -898,72 +994,6 @@ xfs_attrd_destroy_cache(void)
> xfs_attrd_cache = NULL;
> }
>
> -STATIC int
> -xfs_attr_item_init(
> - struct xfs_da_args *args,
> - unsigned int op_flags, /* op flag (set or
> remove) */
> - struct xfs_attr_item **attr) /* new xfs_attr_item
> */
> -{
> -
> - struct xfs_attr_item *new;
> -
> - new = kmem_zalloc(sizeof(struct xfs_attr_item), KM_NOFS);
> - new->xattri_op_flags = op_flags;
> - new->xattri_da_args = args;
> -
> - *attr = new;
> - return 0;
> -}
> -
> -/* Sets an attribute for an inode as a deferred operation */
> -int
> -xfs_attr_set_deferred(
> - struct xfs_da_args *args)
> -{
> - struct xfs_attr_item *new;
> - int error = 0;
> -
> - error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_SET, &new);
> - if (error)
> - return error;
> -
> - if (xfs_attr_is_shortform(args->dp))
> - new->xattri_dela_state = XFS_DAS_SF_ADD;
> - else if (xfs_attr_is_leaf(args->dp))
> - new->xattri_dela_state = XFS_DAS_LEAF_ADD;
> - else
> - new->xattri_dela_state = XFS_DAS_NODE_ADD;
> -
> - xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new-
> >xattri_list);
> -
> - return 0;
> -}
> -
> -/* Removes an attribute for an inode as a deferred operation */
> -int
> -xfs_attr_remove_deferred(
> - struct xfs_da_args *args)
> -{
> -
> - struct xfs_attr_item *new;
> - int error;
> -
> - error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_REMOVE,
> &new);
> - if (error)
> - return error;
> -
> - if (xfs_attr_is_shortform(args->dp))
> - new->xattri_dela_state = XFS_DAS_SF_REMOVE;
> - else if (xfs_attr_is_leaf(args->dp))
> - new->xattri_dela_state = XFS_DAS_LEAF_REMOVE;
> - else
> - new->xattri_dela_state = XFS_DAS_NODE_REMOVE;
> -
> - xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new-
> >xattri_list);
> -
> - return 0;
> -}
> -
> /*==================================================================
> ======
> * External routines when attribute list is inside the inode
>
> *====================================================================
> ====*/
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index e4b11ac243d7..cac7dfcf2dbe 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -559,8 +559,6 @@ bool xfs_attr_namecheck(const void *name, size_t
> length);
> int xfs_attr_calc_size(struct xfs_da_args *args, int *local);
> void xfs_init_attr_trans(struct xfs_da_args *args, struct
> xfs_trans_res *tres,
> unsigned int *total);
> -int xfs_attr_set_deferred(struct xfs_da_args *args);
> -int xfs_attr_remove_deferred(struct xfs_da_args *args);
>
> extern struct kmem_cache *xfs_attri_cache;
> extern struct kmem_cache *xfs_attrd_cache;
