On Mon, May 09, 2022 at 10:41:25AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> We current use XFS_DAS_UNINIT for several steps in the attr_set
> state machine. We use it for setting shortform xattrs, converting
> from shortform to leaf, leaf add, leaf-to-node and leaf add. All of
> these things are essentially known before we start the state machine
> iterating, so we really should separate them out:
>
> XFS_DAS_SF_ADD:
> - tries to do a shortform add
> - on success -> done
> - on ENOSPC converts to leaf, -> XFS_DAS_LEAF_ADD
> - on error, dies.
>
> XFS_DAS_LEAF_ADD:
> - tries to do leaf add
> - on success:
> - inline attr -> done
> - remote xattr || REPLACE -> XFS_DAS_FOUND_LBLK
> - on ENOSPC converts to node, -> XFS_DAS_NODE_ADD
> - on error, dies
>
> XFS_DAS_NODE_ADD:
> - tries to do node add
> - on success:
> - inline attr -> done
> - remote xattr || REPLACE -> XFS_DAS_FOUND_NBLK
> - on error, dies
>
> This makes it easier to understand how the state machine starts
> up and sets us up on the path to further state machine
> simplifications.
Yes!!
> This also converts the DAS state tracepoints to use strings rather
> than numbers, as converting between enums and numbers requires
> manual counting rather than just reading the name.
>
> This also introduces a XFS_DAS_DONE state so that we can trace
> successful operation completions easily.
Yes!!
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Reviewed-by: Allison Henderson<allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 161 +++++++++++++++++++-------------------
> fs/xfs/libxfs/xfs_attr.h | 80 ++++++++++++++++---
> fs/xfs/libxfs/xfs_defer.c | 2 +
> fs/xfs/xfs_acl.c | 4 +-
> fs/xfs/xfs_attr_item.c | 13 ++-
> fs/xfs/xfs_ioctl.c | 4 +-
> fs/xfs/xfs_trace.h | 22 +++++-
> fs/xfs/xfs_xattr.c | 2 +-
> 8 files changed, 185 insertions(+), 103 deletions(-)
>
<snip>
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index c9c867e3406c..ad52b5dc59e4 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -530,4 +553,35 @@ void xfs_attri_destroy_cache(void);
> int __init xfs_attrd_init_cache(void);
> void xfs_attrd_destroy_cache(void);
>
> +/*
> + * Check to see if the attr should be upgraded from non-existent or shortform to
> + * single-leaf-block attribute list.
> + */
> +static inline bool
> +xfs_attr_is_shortform(
> + struct xfs_inode *ip)
> +{
> + return ip->i_afp->if_format == XFS_DINODE_FMT_LOCAL ||
> + (ip->i_afp->if_format == XFS_DINODE_FMT_EXTENTS &&
> + ip->i_afp->if_nextents == 0);
> +}
> +
> +static inline enum xfs_delattr_state
> +xfs_attr_init_add_state(struct xfs_da_args *args)
> +{
> + if (!args->dp->i_afp)
> + return XFS_DAS_DONE;
If we're in add/replace attr call without an attr fork, why do we go
straight to finished?
--D
