Re: [PATCH 2/7] xfs: put the xattr intent item op flags in their own namespace

Alli
2022-05-19T20:34:24Z

On Wed, 2022-05-18 at 11:55 -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> The flags that are stored in the extended attr intent log item really
> should have a separate namespace from the rest of the XFS_ATTR_*
> flags.
> Give them one to make it a little more obvious that they're intent
> item
> flags.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
I RVB'd this one already I think
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
> fs/xfs/libxfs/xfs_attr.c | 6 +++---
> fs/xfs/libxfs/xfs_attr.h | 2 +-
> fs/xfs/libxfs/xfs_log_format.h | 8 ++++----
> fs/xfs/xfs_attr_item.c | 20 ++++++++++----------
> 4 files changed, 18 insertions(+), 18 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 3838109ef288..56e56df9f9f0 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -918,7 +918,7 @@ xfs_attr_defer_add(
> struct xfs_attr_item *new;
> int error = 0;
>
> - error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_SET, &new);
> + error = xfs_attr_item_init(args, XFS_ATTRI_OP_FLAGS_SET, &new);
> if (error)
> return error;
>
> @@ -937,7 +937,7 @@ xfs_attr_defer_replace(
> struct xfs_attr_item *new;
> int error = 0;
>
> - error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_REPLACE,
> &new);
> + error = xfs_attr_item_init(args, XFS_ATTRI_OP_FLAGS_REPLACE,
> &new);
> if (error)
> return error;
>
> @@ -957,7 +957,7 @@ xfs_attr_defer_remove(
> struct xfs_attr_item *new;
> int error;
>
> - error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_REMOVE,
> &new);
> + error = xfs_attr_item_init(args, XFS_ATTRI_OP_FLAGS_REMOVE,
> &new);
> if (error)
> return error;
>
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 17746dcc2268..ccb4f45f474a 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -538,7 +538,7 @@ struct xfs_attr_item {
> enum xfs_delattr_state xattri_dela_state;
>
> /*
> - * Attr operation being performed - XFS_ATTR_OP_FLAGS_*
> + * Attr operation being performed - XFS_ATTRI_OP_FLAGS_*
> */
> unsigned int xattri_op_flags;
>
> diff --git a/fs/xfs/libxfs/xfs_log_format.h
> b/fs/xfs/libxfs/xfs_log_format.h
> index a9d08f3d4682..b351b9dc6561 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -906,10 +906,10 @@ struct xfs_icreate_log {
> * Flags for deferred attribute operations.
> * Upper bits are flags, lower byte is type code
> */
> -#define XFS_ATTR_OP_FLAGS_SET 1 /* Set the attribute
> */
> -#define XFS_ATTR_OP_FLAGS_REMOVE 2 /* Remove the attribute */
> -#define XFS_ATTR_OP_FLAGS_REPLACE 3 /* Replace the attribute */
> -#define XFS_ATTR_OP_FLAGS_TYPE_MASK 0xFF /* Flags type mask */
> +#define XFS_ATTRI_OP_FLAGS_SET 1 /* Set the attribute
> */
> +#define XFS_ATTRI_OP_FLAGS_REMOVE 2 /* Remove the attribute */
> +#define XFS_ATTRI_OP_FLAGS_REPLACE 3 /* Replace the attribute */
> +#define XFS_ATTRI_OP_FLAGS_TYPE_MASK 0xFF /* Flags type mask */
>
> /*
> * alfi_attr_filter captures the state of xfs_da_args.attr_filter,
> so it should
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index 9ef2c2455921..27b6bdc8a3aa 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -406,7 +406,7 @@ xfs_attr_log_item(
> */
> attrp = &attrip->attri_format;
> attrp->alfi_ino = attr->xattri_da_args->dp->i_ino;
> - ASSERT(!(attr->xattri_op_flags &
> ~XFS_ATTR_OP_FLAGS_TYPE_MASK));
> + ASSERT(!(attr->xattri_op_flags &
> ~XFS_ATTRI_OP_FLAGS_TYPE_MASK));
> attrp->alfi_op_flags = attr->xattri_op_flags;
> attrp->alfi_value_len = attr->xattri_nameval->anvl_value_len;
> attrp->alfi_name_len = attr->xattri_nameval->anvl_name_len;
> @@ -539,12 +539,12 @@ xfs_attri_validate(
> struct xfs_attri_log_format *attrp)
> {
> unsigned int op = attrp->alfi_op_flags &
> - XFS_ATTR_OP_FLAGS_TYPE_MAS
> K;
> + XFS_ATTRI_OP_FLAGS_TYPE_MA
> SK;
>
> if (attrp->__pad != 0)
> return false;
>
> - if (attrp->alfi_op_flags & ~XFS_ATTR_OP_FLAGS_TYPE_MASK)
> + if (attrp->alfi_op_flags & ~XFS_ATTRI_OP_FLAGS_TYPE_MASK)
> return false;
>
> if (attrp->alfi_attr_filter & ~XFS_ATTRI_FILTER_MASK)
> @@ -552,9 +552,9 @@ xfs_attri_validate(
>
> /* alfi_op_flags should be either a set or remove */
> switch (op) {
> - case XFS_ATTR_OP_FLAGS_SET:
> - case XFS_ATTR_OP_FLAGS_REPLACE:
> - case XFS_ATTR_OP_FLAGS_REMOVE:
> + case XFS_ATTRI_OP_FLAGS_SET:
> + case XFS_ATTRI_OP_FLAGS_REPLACE:
> + case XFS_ATTRI_OP_FLAGS_REMOVE:
> break;
> default:
> return false;
> @@ -613,7 +613,7 @@ xfs_attri_item_recover(
>
> attr->xattri_da_args = args;
> attr->xattri_op_flags = attrp->alfi_op_flags &
> - XFS_ATTR_OP_FLAGS_TYPE_
> MASK;
> + XFS_ATTRI_OP_FLAGS_TYPE
> _MASK;
>
> /*
> * We're reconstructing the deferred work state structure from
> the
> @@ -632,8 +632,8 @@ xfs_attri_item_recover(
> args->op_flags = XFS_DA_OP_RECOVERY | XFS_DA_OP_OKNOENT;
>
> switch (attr->xattri_op_flags) {
> - case XFS_ATTR_OP_FLAGS_SET:
> - case XFS_ATTR_OP_FLAGS_REPLACE:
> + case XFS_ATTRI_OP_FLAGS_SET:
> + case XFS_ATTRI_OP_FLAGS_REPLACE:
> args->value = xfs_attri_log_valbuf(attrip);
> args->valuelen = attrp->alfi_value_len;
> args->total = xfs_attr_calc_size(args, &local);
> @@ -642,7 +642,7 @@ xfs_attri_item_recover(
> else
> attr->xattri_dela_state =
> xfs_attr_init_add_state(args);
> break;
> - case XFS_ATTR_OP_FLAGS_REMOVE:
> + case XFS_ATTRI_OP_FLAGS_REMOVE:
> if (!xfs_inode_hasattr(args->dp))
> goto out;
> attr->xattri_dela_state =
> xfs_attr_init_remove_state(args);
>