xfs: clean up final attr removal in xfs_attr_set_iter

Clean up the final leaf/node states in xfs_attr_set_iter() to
further simplify the high level state machine and to set the
completion state correctly. As we are adding a separate state
for node format removal, we need to ensure that node formats
are collapsed back to shortform or empty correctly.

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 c0e72e9c4f5371df330184bd8d3519e1507a6640..467e2360200593817bc0b0ab5cb832d031aee116 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -60,7 +60,7 @@ STATIC int xfs_attr_node_get(xfs_da_args_t *args);
 STATIC void xfs_attr_restore_rmt_blk(struct xfs_da_args *args);
 static int xfs_attr_node_try_addname(struct xfs_attr_item *attr);
 STATIC int xfs_attr_node_addname_find_attr(struct xfs_attr_item *attr);
-STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_attr_item *attr);
+STATIC int xfs_attr_node_remove_attr(struct xfs_attr_item *attr);
 STATIC int xfs_attr_node_hasname(xfs_da_args_t *args,
                                 struct xfs_da_state **state);
 STATIC int xfs_attr_fillstate(xfs_da_state_t *state);
@@ -443,6 +443,77 @@ out:
        return error;
 }
 
+/*
+ * Remove the original attr we have just replaced. This is dependent on the
+ * original lookup and insert placing the old attr in args->blkno/args->index
+ * and the new attr in args->blkno2/args->index2.
+ */
+static int
+xfs_attr_leaf_remove_attr(
+       struct xfs_attr_item            *attr)
+{
+       struct xfs_da_args              *args = attr->xattri_da_args;
+       struct xfs_inode                *dp = args->dp;
+       struct xfs_buf                  *bp = NULL;
+       int                             forkoff;
+       int                             error;
+
+       error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno,
+                                  &bp);
+       if (error)
+               return error;
+
+       xfs_attr3_leaf_remove(bp, args);
+
+       forkoff = xfs_attr_shortform_allfit(bp, dp);
+       if (forkoff)
+               error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
+               /* bp is gone due to xfs_da_shrink_inode */
+
+       return error;
+}
+
+/*
+ * Shrink an attribute from leaf to shortform. Used by the node format remove
+ * path when the node format collapses to a single block and so we have to check
+ * if it can be collapsed further.
+ */
+static int
+xfs_attr_leaf_shrink(
+       struct xfs_da_args      *args,
+       struct xfs_da_state     *state)
+{
+       struct xfs_inode        *dp = args->dp;
+       int                     error, forkoff;
+       struct xfs_buf          *bp;
+
+       if (!xfs_attr_is_leaf(dp))
+               return 0;
+
+       /*
+        * Have to get rid of the copy of this dabuf in the state.
+        */
+       if (state) {
+               ASSERT(state->path.active == 1);
+               ASSERT(state->path.blk[0].bp);
+               state->path.blk[0].bp = NULL;
+       }
+
+       error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
+       if (error)
+               return error;
+
+       forkoff = xfs_attr_shortform_allfit(bp, dp);
+       if (forkoff) {
+               error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
+               /* bp is gone due to xfs_da_shrink_inode */
+       } else {
+               xfs_trans_brelse(args->trans, bp);
+       }
+
+       return error;
+}
+
 /*
  * Set the attribute specified in @args.
  * This routine is meant to function as a delayed operation, and may return
@@ -455,9 +526,7 @@ xfs_attr_set_iter(
        struct xfs_attr_item            *attr)
 {
        struct xfs_da_args              *args = attr->xattri_da_args;
-       struct xfs_inode                *dp = args->dp;
-       struct xfs_buf                  *bp = NULL;
-       int                             forkoff, error = 0;
+       int                             error = 0;
 
        /* State machine switch */
 next_state:
@@ -548,32 +617,16 @@ next_state:
                attr->xattri_dela_state++;
                break;
 
-       case XFS_DAS_RD_LEAF:
-               /*
-                * This is the last step for leaf format. Read the block with
-                * the old attr, remove the old attr, check for shortform
-                * conversion and return.
-                */
-               error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno,
-                                          &bp);
-               if (error)
-                       return error;
-
-               xfs_attr3_leaf_remove(bp, args);
-
-               forkoff = xfs_attr_shortform_allfit(bp, dp);
-               if (forkoff)
-                       error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
-                       /* bp is gone due to xfs_da_shrink_inode */
-
-               return error;
+       case XFS_DAS_LEAF_REMOVE_ATTR:
+               error = xfs_attr_leaf_remove_attr(attr);
+               attr->xattri_dela_state = XFS_DAS_DONE;
+               break;
 
-       case XFS_DAS_CLR_FLAG:
-               /*
-                * The last state for node format. Look up the old attr and
-                * remove it.
-                */
-               error = xfs_attr_node_addname_clear_incomplete(attr);
+       case XFS_DAS_NODE_REMOVE_ATTR:
+               error = xfs_attr_node_remove_attr(attr);
+               if (!error)
+                       error = xfs_attr_leaf_shrink(args, NULL);
+               attr->xattri_dela_state = XFS_DAS_DONE;
                break;
        default:
                ASSERT(0);
@@ -1268,8 +1321,8 @@ out:
 }
 
 
-STATIC int
-xfs_attr_node_addname_clear_incomplete(
+static int
+xfs_attr_node_remove_attr(
        struct xfs_attr_item            *attr)
 {
        struct xfs_da_args              *args = attr->xattri_da_args;
@@ -1310,38 +1363,6 @@ out:
        return retval;
 }
 
-/*
- * Shrink an attribute from leaf to shortform
- */
-STATIC int
-xfs_attr_node_shrink(
-       struct xfs_da_args      *args,
-       struct xfs_da_state     *state)
-{
-       struct xfs_inode        *dp = args->dp;
-       int                     error, forkoff;
-       struct xfs_buf          *bp;
-
-       /*
-        * Have to get rid of the copy of this dabuf in the state.
-        */
-       ASSERT(state->path.active == 1);
-       ASSERT(state->path.blk[0].bp);
-       state->path.blk[0].bp = NULL;
-
-       error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
-       if (error)
-               return error;
-
-       forkoff = xfs_attr_shortform_allfit(bp, dp);
-       if (forkoff) {
-               error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
-               /* bp is gone due to xfs_da_shrink_inode */
-       } else
-               xfs_trans_brelse(args->trans, bp);
-
-       return error;
-}
 
 /*
  * Mark an attribute entry INCOMPLETE and save pointers to the relevant buffers
@@ -1550,7 +1571,7 @@ xfs_attr_remove_iter(
                 * transaction.
                 */
                if (xfs_attr_is_leaf(dp))
-                       error = xfs_attr_node_shrink(args, state);
+                       error = xfs_attr_leaf_shrink(args, state);
                ASSERT(error != -EAGAIN);
                break;
        default:

diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 1e038c23029a4c77b156b0c340d86e72b6b3aaa6..7b0a5a165725f3feb9c5e0de462d3e322bdb54b3 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -451,21 +451,21 @@ enum xfs_delattr_state {
        XFS_DAS_RM_NAME,                /* Remove attr name */
        XFS_DAS_RM_SHRINK,              /* We are shrinking the tree */
 
-       /* Leaf state set sequence */
+       /* Leaf state set/replace sequence */
        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_LEAF_REMOVE_OLD,        /* Start removing old attr from leaf */
        XFS_DAS_LEAF_REMOVE_RMT,        /* A rename is removing remote blocks */
-       XFS_DAS_RD_LEAF,                /* Read in the new leaf */
+       XFS_DAS_LEAF_REMOVE_ATTR,       /* Remove the old attr from a leaf */
 
-       /* Node state set sequence, must match leaf state above */
+       /* Node state set/replace sequence, must match leaf state above */
        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_NODE_REMOVE_OLD,        /* Start removing old attr from node */
        XFS_DAS_NODE_REMOVE_RMT,        /* A rename is removing remote blocks */
-       XFS_DAS_CLR_FLAG,               /* Clear incomplete flag */
+       XFS_DAS_NODE_REMOVE_ATTR,       /* Remove the old attr from a node */
 
        XFS_DAS_DONE,                   /* finished operation */
 };
@@ -483,13 +483,13 @@ enum xfs_delattr_state {
        { XFS_DAS_LEAF_REPLACE,         "XFS_DAS_LEAF_REPLACE" }, \
        { XFS_DAS_LEAF_REMOVE_OLD,      "XFS_DAS_LEAF_REMOVE_OLD" }, \
        { XFS_DAS_LEAF_REMOVE_RMT,      "XFS_DAS_LEAF_REMOVE_RMT" }, \
-       { XFS_DAS_RD_LEAF,              "XFS_DAS_RD_LEAF" }, \
+       { XFS_DAS_LEAF_REMOVE_ATTR,     "XFS_DAS_LEAF_REMOVE_ATTR" }, \
        { 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_NODE_REMOVE_RMT,      "XFS_DAS_NODE_REMOVE_RMT" }, \
-       { XFS_DAS_CLR_FLAG,             "XFS_DAS_CLR_FLAG" }, \
+       { XFS_DAS_NODE_REMOVE_ATTR,     "XFS_DAS_NODE_REMOVE_ATTR" }, \
        { XFS_DAS_DONE,                 "XFS_DAS_DONE" }
 
 /*

diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 793d2a86ab2cc6c62bccd5df5ccd89326f7a5599..260760ce2d051e6048563aa1660aed59678d6aa5 100644 (file)
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -4141,13 +4141,14 @@ TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT);
 TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE);
 TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_OLD);
 TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_RMT);
-TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF);
+TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_ATTR);
 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_NODE_REMOVE_OLD);
 TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_RMT);
-TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG);
+TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_ATTR);
+TRACE_DEFINE_ENUM(XFS_DAS_DONE);
 
 DECLARE_EVENT_CLASS(xfs_das_state_class,
        TP_PROTO(int das, struct xfs_inode *ip),