From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 07/16] xfs: split remote attr setting out from replace path
Date: Thu, 14 Apr 2022 19:44:25 +1000 [thread overview]
Message-ID: <20220414094434.2508781-8-david@fromorbit.com> (raw)
In-Reply-To: <20220414094434.2508781-1-david@fromorbit.com>
From: Dave Chinner <dchinner@redhat.com>
When we set a new xattr, we have three exit paths:
1. nothign else to do
2. allocate and set the remote xattr value
3. perform the rest of a replace operation
Currently we push both 2 and 3 into the same state, regardless of
whether we just set a remote attribute or not. Once we've set the
remote xattr, we have two exit states:
1. nothign else to do
2. perform the rest of a replace operation
Hence we can split the remote xattr allocation and setting into
their own states and factor it out of xfs_attr_set_iter() to further
clean up the state machine and the implementation of the state
machine.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_attr.c | 113 +++++++++++++++++++++------------------
fs/xfs/libxfs/xfs_attr.h | 14 +++--
fs/xfs/xfs_trace.h | 9 ++--
3 files changed, 77 insertions(+), 59 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 655e4388dfec..4b9c107fe5de 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -347,9 +347,11 @@ xfs_attr_leaf_addname(
* or perform more xattr manipulations. Otherwise there is nothing more
* to do and we can return success.
*/
- if (args->rmtblkno ||
- (args->op_flags & XFS_DA_OP_RENAME)) {
- attr->xattri_dela_state = XFS_DAS_FOUND_LBLK;
+ if (args->rmtblkno) {
+ attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT;
+ error = -EAGAIN;
+ } else if (args->op_flags & XFS_DA_OP_RENAME) {
+ attr->xattri_dela_state = XFS_DAS_LEAF_REPLACE;
error = -EAGAIN;
} else {
attr->xattri_dela_state = XFS_DAS_DONE;
@@ -376,9 +378,11 @@ xfs_attr_node_addname(
if (error)
return error;
- if (args->rmtblkno ||
- (args->op_flags & XFS_DA_OP_RENAME)) {
- attr->xattri_dela_state = XFS_DAS_FOUND_NBLK;
+ if (args->rmtblkno) {
+ attr->xattri_dela_state = XFS_DAS_NODE_SET_RMT;
+ error = -EAGAIN;
+ } else if (args->op_flags & XFS_DA_OP_RENAME) {
+ attr->xattri_dela_state = XFS_DAS_NODE_REPLACE;
error = -EAGAIN;
} else {
attr->xattri_dela_state = XFS_DAS_DONE;
@@ -388,6 +392,40 @@ xfs_attr_node_addname(
return error;
}
+static int
+xfs_attr_rmtval_alloc(
+ struct xfs_attr_item *attr)
+{
+ struct xfs_da_args *args = attr->xattri_da_args;
+ int error = 0;
+
+ /*
+ * If there was an out-of-line value, allocate the blocks we
+ * identified for its storage and copy the value. This is done
+ * after we create the attribute so that we don't overflow the
+ * maximum size of a transaction and/or hit a deadlock.
+ */
+ if (attr->xattri_blkcnt > 0) {
+ error = xfs_attr_rmtval_set_blk(attr);
+ if (error)
+ return error;
+ error = -EAGAIN;
+ goto out;
+ }
+
+ error = xfs_attr_rmtval_set_value(args);
+ if (error)
+ return error;
+
+ /* If this is not a rename, clear the incomplete flag and we're done. */
+ if (!(args->op_flags & XFS_DA_OP_RENAME)) {
+ error = xfs_attr3_leaf_clearflag(args);
+ attr->xattri_dela_state = XFS_DAS_DONE;
+ }
+out:
+ trace_xfs_attr_rmtval_alloc(attr->xattri_dela_state, args->dp);
+ return error;
+}
/*
* Set the attribute specified in @args.
@@ -419,54 +457,26 @@ xfs_attr_set_iter(
case XFS_DAS_NODE_ADD:
return xfs_attr_node_addname(attr);
- case XFS_DAS_FOUND_LBLK:
- case XFS_DAS_FOUND_NBLK:
- /*
- * Find space for remote blocks and fall into the allocation
- * state.
- */
- if (args->rmtblkno > 0) {
- error = xfs_attr_rmtval_find_space(attr);
- if (error)
- return error;
- }
+ case XFS_DAS_LEAF_SET_RMT:
+ case XFS_DAS_NODE_SET_RMT:
+ error = xfs_attr_rmtval_find_space(attr);
+ if (error)
+ return error;
attr->xattri_dela_state++;
fallthrough;
+
case XFS_DAS_LEAF_ALLOC_RMT:
case XFS_DAS_NODE_ALLOC_RMT:
-
- /*
- * If there was an out-of-line value, allocate the blocks we
- * identified for its storage and copy the value. This is done
- * after we create the attribute so that we don't overflow the
- * maximum size of a transaction and/or hit a deadlock.
- */
- if (args->rmtblkno > 0) {
- if (attr->xattri_blkcnt > 0) {
- error = xfs_attr_rmtval_set_blk(attr);
- if (error)
- return error;
- trace_xfs_attr_set_iter_return(
- attr->xattri_dela_state,
- args->dp);
- return -EAGAIN;
- }
-
- error = xfs_attr_rmtval_set_value(args);
- if (error)
- return error;
- }
-
- /*
- * If this is not a rename, clear the incomplete flag and we're
- * done.
- */
- if (!(args->op_flags & XFS_DA_OP_RENAME)) {
- if (args->rmtblkno > 0)
- error = xfs_attr3_leaf_clearflag(args);
+ error = xfs_attr_rmtval_alloc(attr);
+ if (error)
return error;
- }
+ if (attr->xattri_dela_state == XFS_DAS_DONE)
+ break;
+ attr->xattri_dela_state++;
+ fallthrough;
+ case XFS_DAS_LEAF_REPLACE:
+ case XFS_DAS_NODE_REPLACE:
/*
* If this is an atomic rename operation, we must "flip" the
* incomplete flags on the "new" and "old" attribute/value pairs
@@ -484,10 +494,9 @@ xfs_attr_set_iter(
* Commit the flag value change and start the next trans
* in series at FLIP_FLAG.
*/
+ error = -EAGAIN;
attr->xattri_dela_state++;
- trace_xfs_attr_set_iter_return(attr->xattri_dela_state,
- args->dp);
- return -EAGAIN;
+ break;
}
attr->xattri_dela_state++;
@@ -562,6 +571,8 @@ xfs_attr_set_iter(
ASSERT(0);
break;
}
+
+ trace_xfs_attr_set_iter_return(attr->xattri_dela_state, args->dp);
return error;
}
diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 0ad78f9279ac..1de6c06b7f19 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -452,15 +452,17 @@ enum xfs_delattr_state {
XFS_DAS_RM_SHRINK, /* We are shrinking the tree */
/* Leaf state set sequence */
- XFS_DAS_FOUND_LBLK, /* We found leaf blk for attr */
+ XFS_DAS_LEAF_SET_RMT, /* set a remote xattr from a leaf */
XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */
+ XFS_DAS_LEAF_REPLACE, /* Perform replace ops on a leaf */
XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */
XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */
XFS_DAS_RD_LEAF, /* Read in the new leaf */
/* Node state set sequence, must match leaf state above */
- XFS_DAS_FOUND_NBLK, /* We found node blk for attr */
+ XFS_DAS_NODE_SET_RMT, /* set a remote xattr from a node */
XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */
+ XFS_DAS_NODE_REPLACE, /* Perform replace ops on a node */
XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */
XFS_DAS_RM_NBLK, /* A rename is removing node blocks */
XFS_DAS_CLR_FLAG, /* Clear incomplete flag */
@@ -476,13 +478,15 @@ enum xfs_delattr_state {
{ XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \
{ XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \
{ XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \
- { XFS_DAS_FOUND_LBLK, "XFS_DAS_FOUND_LBLK" }, \
+ { XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \
{ XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \
- { XFS_DAS_FOUND_NBLK, "XFS_DAS_FOUND_NBLK" }, \
- { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \
+ { XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \
{ XFS_DAS_FLIP_LFLAG, "XFS_DAS_FLIP_LFLAG" }, \
{ XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \
{ XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \
+ { XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \
+ { XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \
+ { XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \
{ XFS_DAS_FLIP_NFLAG, "XFS_DAS_FLIP_NFLAG" }, \
{ XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \
{ XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 8739cc1e0561..cf99efc5ba5a 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -4105,13 +4105,15 @@ TRACE_DEFINE_ENUM(XFS_DAS_NODE_ADD);
TRACE_DEFINE_ENUM(XFS_DAS_RMTBLK);
TRACE_DEFINE_ENUM(XFS_DAS_RM_NAME);
TRACE_DEFINE_ENUM(XFS_DAS_RM_SHRINK);
-TRACE_DEFINE_ENUM(XFS_DAS_FOUND_LBLK);
+TRACE_DEFINE_ENUM(XFS_DAS_LEAF_SET_RMT);
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT);
-TRACE_DEFINE_ENUM(XFS_DAS_FOUND_NBLK);
-TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT);
+TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE);
TRACE_DEFINE_ENUM(XFS_DAS_FLIP_LFLAG);
TRACE_DEFINE_ENUM(XFS_DAS_RM_LBLK);
TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF);
+TRACE_DEFINE_ENUM(XFS_DAS_NODE_SET_RMT);
+TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT);
+TRACE_DEFINE_ENUM(XFS_DAS_NODE_REPLACE);
TRACE_DEFINE_ENUM(XFS_DAS_FLIP_NFLAG);
TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK);
TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG);
@@ -4141,6 +4143,7 @@ DEFINE_DAS_STATE_EVENT(xfs_attr_set_iter_return);
DEFINE_DAS_STATE_EVENT(xfs_attr_leaf_addname_return);
DEFINE_DAS_STATE_EVENT(xfs_attr_node_addname_return);
DEFINE_DAS_STATE_EVENT(xfs_attr_remove_iter_return);
+DEFINE_DAS_STATE_EVENT(xfs_attr_rmtval_alloc);
DEFINE_DAS_STATE_EVENT(xfs_attr_rmtval_remove_return);
TRACE_EVENT(xfs_force_shutdown,
--
2.35.1
next prev parent reply other threads:[~2022-04-14 9:46 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-14 9:44 [PATCH 00/10 v2] xfs: LARP - clean up xfs_attr_set_iter state machine Dave Chinner
2022-04-14 9:44 ` [PATCH 01/16] xfs: avoid empty xattr transaction when attrs are inline Dave Chinner
2022-04-22 0:37 ` Alli
2022-04-14 9:44 ` [PATCH 02/16] xfs: initialise attrd item to zero Dave Chinner
2022-04-22 0:37 ` Alli
2022-04-14 9:44 ` [PATCH 03/16] xfs: make xattri_leaf_bp more useful Dave Chinner
2022-04-22 0:37 ` Alli
2022-04-14 9:44 ` [PATCH 04/16] xfs: separate out initial attr_set states Dave Chinner
2022-04-22 0:38 ` Alli
2022-04-26 23:50 ` Dave Chinner
2022-04-14 9:44 ` [PATCH 05/16] xfs: kill XFS_DAC_LEAF_ADDNAME_INIT Dave Chinner
2022-04-22 0:38 ` Alli
2022-04-14 9:44 ` [PATCH 06/16] xfs: consolidate leaf/node states in xfs_attr_set_iter Dave Chinner
2022-04-23 1:05 ` Alli
2022-04-14 9:44 ` Dave Chinner [this message]
2022-04-23 1:06 ` [PATCH 07/16] xfs: split remote attr setting out from replace path Alli
2022-04-14 9:44 ` [PATCH 08/16] xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP Dave Chinner
2022-04-23 1:06 ` Alli
2022-04-14 9:44 ` [PATCH 09/16] xfs: remote xattr removal in xfs_attr_set_iter() is conditional Dave Chinner
2022-04-23 1:07 ` Alli
2022-04-14 9:44 ` [PATCH 10/16] xfs: clean up final attr removal in xfs_attr_set_iter Dave Chinner
2022-04-23 1:06 ` Alli
2022-04-27 0:32 ` Alli
2022-04-14 9:44 ` [PATCH 11/16] xfs: xfs_attr_set_iter() does not need to return EAGAIN Dave Chinner
2022-04-27 0:32 ` Alli
2022-04-14 9:44 ` [PATCH 12/16] xfs: introduce attr remove initial states into xfs_attr_set_iter Dave Chinner
2022-04-27 0:33 ` Alli
2022-04-14 9:44 ` [PATCH 13/16] xfs: switch attr remove to xfs_attri_set_iter Dave Chinner
2022-04-27 0:34 ` Alli
2022-05-03 6:43 ` Dave Chinner
2022-04-14 9:44 ` [PATCH 14/16] xfs: remove xfs_attri_remove_iter Dave Chinner
2022-04-27 0:34 ` Alli
2022-05-03 6:53 ` Dave Chinner
2022-04-14 9:44 ` [PATCH 15/16] xfs: split attr replace op setup from create op setup Dave Chinner
2022-04-27 1:10 ` Alli
2022-04-14 9:44 ` [PATCH 16/16] xfs: ATTR_REPLACE algorithm with LARP enabled needs rework Dave Chinner
2022-04-28 7:02 ` Alli
2022-05-03 7:40 ` Dave Chinner
2022-05-04 1:30 ` Alli
2022-05-04 5:49 ` Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220414094434.2508781-8-david@fromorbit.com \
--to=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.