From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 09/17] xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP
Date: Fri, 6 May 2022 19:45:45 +1000 [thread overview]
Message-ID: <20220506094553.512973-10-david@fromorbit.com> (raw)
In-Reply-To: <20220506094553.512973-1-david@fromorbit.com>
From: Dave Chinner <dchinner@redhat.com>
We can skip the REPLACE state when LARP is enabled, but that means
the XFS_DAS_FLIP_LFLAG state is now poorly named - it indicates
something that has been done rather than what the state is going to
do. Rename it to "REMOVE_OLD" to indicate that we are now going to
perform removal of the old attr.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_attr.c | 81 +++++++++++++++++++++++++---------------
fs/xfs/libxfs/xfs_attr.h | 44 +++++++++++-----------
fs/xfs/xfs_trace.h | 4 +-
3 files changed, 75 insertions(+), 54 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d2b29f7e103a..0f4636e2e246 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -296,6 +296,26 @@ xfs_attr_sf_addname(
return error;
}
+/*
+ * When we bump the state to REPLACE, we may actually need to skip over the
+ * state. When LARP mode is enabled, we don't need to run the atomic flags flip,
+ * so we skip straight over the REPLACE state and go on to REMOVE_OLD.
+ */
+static void
+xfs_attr_dela_state_set_replace(
+ struct xfs_attr_item *attr,
+ enum xfs_delattr_state replace)
+{
+ struct xfs_da_args *args = attr->xattri_da_args;
+
+ ASSERT(replace == XFS_DAS_LEAF_REPLACE ||
+ replace == XFS_DAS_NODE_REPLACE);
+
+ attr->xattri_dela_state = replace;
+ if (xfs_has_larp(args->dp->i_mount))
+ attr->xattri_dela_state++;
+}
+
static int
xfs_attr_leaf_addname(
struct xfs_attr_item *attr)
@@ -338,7 +358,7 @@ xfs_attr_leaf_addname(
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;
+ xfs_attr_dela_state_set_replace(attr, XFS_DAS_LEAF_REPLACE);
error = -EAGAIN;
} else {
attr->xattri_dela_state = XFS_DAS_DONE;
@@ -369,7 +389,7 @@ xfs_attr_node_addname(
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;
+ xfs_attr_dela_state_set_replace(attr, XFS_DAS_NODE_REPLACE);
error = -EAGAIN;
} else {
attr->xattri_dela_state = XFS_DAS_DONE;
@@ -396,8 +416,11 @@ xfs_attr_rmtval_alloc(
error = xfs_attr_rmtval_set_blk(attr);
if (error)
return error;
- error = -EAGAIN;
- goto out;
+ /* Roll the transaction only if there is more to allocate. */
+ if (attr->xattri_blkcnt > 0) {
+ error = -EAGAIN;
+ goto out;
+ }
}
error = xfs_attr_rmtval_set_value(args);
@@ -408,6 +431,13 @@ xfs_attr_rmtval_alloc(
if (!(args->op_flags & XFS_DA_OP_RENAME)) {
error = xfs_attr3_leaf_clearflag(args);
attr->xattri_dela_state = XFS_DAS_DONE;
+ } else {
+ /*
+ * We are running a REPLACE operation, so we need to bump the
+ * state to the step in that operation.
+ */
+ attr->xattri_dela_state++;
+ xfs_attr_dela_state_set_replace(attr, attr->xattri_dela_state);
}
out:
trace_xfs_attr_rmtval_alloc(attr->xattri_dela_state, args->dp);
@@ -429,7 +459,6 @@ xfs_attr_set_iter(
struct xfs_inode *dp = args->dp;
struct xfs_buf *bp = NULL;
int forkoff, error = 0;
- struct xfs_mount *mp = args->dp->i_mount;
/* State machine switch */
next_state:
@@ -459,37 +488,29 @@ xfs_attr_set_iter(
return error;
if (attr->xattri_dela_state == XFS_DAS_DONE)
break;
- attr->xattri_dela_state++;
- fallthrough;
+ goto next_state;
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
- * so that one disappears and one appears atomically. Then we
- * must remove the "old" attribute/value pair.
- *
- * In a separate transaction, set the incomplete flag on the
- * "old" attr and clear the incomplete flag on the "new" attr.
+ * We must "flip" the incomplete flags on the "new" and "old"
+ * attribute/value pairs so that one disappears and one appears
+ * atomically. Then we must remove the "old" attribute/value
+ * pair.
*/
- if (!xfs_has_larp(mp)) {
- error = xfs_attr3_leaf_flipflags(args);
- if (error)
- return error;
- /*
- * Commit the flag value change and start the next trans
- * in series at FLIP_FLAG.
- */
- error = -EAGAIN;
- attr->xattri_dela_state++;
- break;
- }
-
+ error = xfs_attr3_leaf_flipflags(args);
+ if (error)
+ return error;
+ /*
+ * Commit the flag value change and start the next trans
+ * in series at REMOVE_OLD.
+ */
+ error = -EAGAIN;
attr->xattri_dela_state++;
- fallthrough;
- case XFS_DAS_FLIP_LFLAG:
- case XFS_DAS_FLIP_NFLAG:
+ break;
+
+ case XFS_DAS_LEAF_REMOVE_OLD:
+ case XFS_DAS_NODE_REMOVE_OLD:
/*
* Dismantle the "old" attribute/value pair by removing a
* "remote" value (if it exists).
diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 1749fd8f7ddd..3f1234272f3a 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -455,7 +455,7 @@ enum xfs_delattr_state {
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_LEAF_REMOVE_OLD, /* Start removing old attr from leaf */
XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */
XFS_DAS_RD_LEAF, /* Read in the new leaf */
@@ -463,7 +463,7 @@ enum xfs_delattr_state {
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_NODE_REMOVE_OLD, /* Start removing old attr from node */
XFS_DAS_RM_NBLK, /* A rename is removing node blocks */
XFS_DAS_CLR_FLAG, /* Clear incomplete flag */
@@ -471,26 +471,26 @@ enum xfs_delattr_state {
};
#define XFS_DAS_STRINGS \
- { XFS_DAS_UNINIT, "XFS_DAS_UNINIT" }, \
- { XFS_DAS_SF_ADD, "XFS_DAS_SF_ADD" }, \
- { XFS_DAS_LEAF_ADD, "XFS_DAS_LEAF_ADD" }, \
- { XFS_DAS_NODE_ADD, "XFS_DAS_NODE_ADD" }, \
- { XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \
- { XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \
- { XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \
- { XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \
- { XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_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" }, \
- { XFS_DAS_DONE, "XFS_DAS_DONE" }
+ { XFS_DAS_UNINIT, "XFS_DAS_UNINIT" }, \
+ { XFS_DAS_SF_ADD, "XFS_DAS_SF_ADD" }, \
+ { XFS_DAS_LEAF_ADD, "XFS_DAS_LEAF_ADD" }, \
+ { XFS_DAS_NODE_ADD, "XFS_DAS_NODE_ADD" }, \
+ { XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \
+ { XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \
+ { XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \
+ { XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \
+ { XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \
+ { XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \
+ { XFS_DAS_LEAF_REMOVE_OLD, "XFS_DAS_LEAF_REMOVE_OLD" }, \
+ { 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_NODE_REMOVE_OLD, "XFS_DAS_NODE_REMOVE_OLD" }, \
+ { XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \
+ { XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \
+ { XFS_DAS_DONE, "XFS_DAS_DONE" }
/*
* Defines for xfs_attr_item.xattri_flags
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index cb9122327114..b528c0f375c2 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -4139,13 +4139,13 @@ TRACE_DEFINE_ENUM(XFS_DAS_RM_SHRINK);
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_SET_RMT);
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT);
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE);
-TRACE_DEFINE_ENUM(XFS_DAS_FLIP_LFLAG);
+TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_OLD);
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_NODE_REMOVE_OLD);
TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK);
TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG);
--
2.35.1
next prev parent reply other threads:[~2022-05-06 9:46 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-06 9:45 Dave Chinner
2022-05-06 9:45 ` [PATCH 01/17] xfs: avoid empty xattr transaction when attrs are inline Dave Chinner
2022-05-06 9:45 ` [PATCH 02/17] xfs: initialise attrd item to zero Dave Chinner
2022-05-06 9:45 ` [PATCH 03/17] xfs: make xattri_leaf_bp more useful Dave Chinner
2022-05-06 9:45 ` [PATCH 04/17] xfs: rework deferred attribute operation setup Dave Chinner
2022-05-06 23:43 ` Alli
2022-05-06 9:45 ` [PATCH 05/17] xfs: separate out initial attr_set states Dave Chinner
2022-05-06 23:44 ` Alli
2022-05-06 9:45 ` [PATCH 06/17] xfs: kill XFS_DAC_LEAF_ADDNAME_INIT Dave Chinner
2022-05-06 9:45 ` [PATCH 07/17] xfs: consolidate leaf/node states in xfs_attr_set_iter Dave Chinner
2022-05-06 9:45 ` [PATCH 08/17] xfs: split remote attr setting out from replace path Dave Chinner
2022-05-06 9:45 ` Dave Chinner [this message]
2022-05-06 23:44 ` [PATCH 09/17] xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP Alli
2022-05-06 9:45 ` [PATCH 10/17] xfs: remote xattr removal in xfs_attr_set_iter() is conditional Dave Chinner
2022-05-06 9:45 ` [PATCH 11/17] xfs: clean up final attr removal in xfs_attr_set_iter Dave Chinner
2022-05-06 9:45 ` [PATCH 12/17] xfs: xfs_attr_set_iter() does not need to return EAGAIN Dave Chinner
2022-05-06 9:45 ` [PATCH 13/17] xfs: introduce attr remove initial states into xfs_attr_set_iter Dave Chinner
2022-05-06 9:45 ` [PATCH 14/17] xfs: switch attr remove to xfs_attri_set_iter Dave Chinner
2022-05-06 23:44 ` Alli
2022-05-06 9:45 ` [PATCH 15/17] xfs: remove xfs_attri_remove_iter Dave Chinner
2022-05-06 9:45 ` [PATCH 16/17] xfs: use XFS_DA_OP flags in deferred attr ops Dave Chinner
2022-05-09 8:17 ` Dave Chinner
2022-05-10 22:31 ` Alli
2022-05-06 9:45 ` [PATCH 17/17] xfs: ATTR_REPLACE algorithm with LARP enabled needs rework Dave Chinner
2022-05-07 6:01 ` Alli
2022-05-08 21:50 ` Dave Chinner
2022-05-08 23:24 ` Dave Chinner
2022-05-06 22:35 ` [PATCH 00/17 V3] XFS: LARP state machine and recovery rework 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=20220506094553.512973-10-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox