xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP

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>
Reviewed-by: Allison Henderson<allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Dave Chinner <david@fromorbit.com>

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 513f0b1..9b5ef38 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -295,6 +295,26 @@ out:
        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)
@@ -337,7 +357,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;
@@ -368,7 +388,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;
@@ -395,8 +415,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);
@@ -407,6 +430,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);
@@ -428,7 +458,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:
@@ -458,37 +487,29 @@ next_state:
                        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 a0e631d..01a5061 100644 (file)
--- 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 cb91223..b528c0f 100644 (file)
--- 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);