xfs: use XFS_DA_OP flags in deferred attr ops

We currently store the high level attr operation in
args->attr_flags. This field contains what the VFS is telling us to
do, but don't necessarily match what we are doing in the low level
modification state machine. e.g. XATTR_REPLACE implies both
XFS_DA_OP_ADDNAME and XFS_DA_OP_RENAME because it is doing both a
remove and adding a new attr.

However, deep in the individual state machine operations, we check
errors against this high level VFS op flags, not the low level
XFS_DA_OP flags. Indeed, we don't even have a low level flag for
a REMOVE operation, so the only way we know we are doing a remove
is the complete absence of XATTR_REPLACE, XATTR_CREATE,
XFS_DA_OP_ADDNAME and XFS_DA_OP_RENAME. And because there are other
flags in these fields, this is a pain to check if we need to.

As the XFS_DA_OP flags are only needed once the deferred operations
are set up, set these flags appropriately when we set the initial
operation state. We also introduce a XFS_DA_OP_REMOVE flag to make
it easy to know that we are doing a remove operation.

With these, we can remove the use of XATTR_REPLACE and XATTR_CREATE
in low level lookup operations, and manipulate the low level flags
according to the low level context that is operating. e.g. log
recovery does not have a VFS xattr operation state to copy into
args->attr_flags, and the low level state machine ops we do for
recovery do not match the high level VFS operations that were in
progress when the system failed...

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
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 dc3e3de..5072c15 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -466,7 +466,7 @@ xfs_attr_leaf_addname(
         */
        if (args->rmtblkno)
                attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT;
-       else if (args->op_flags & XFS_DA_OP_RENAME)
+       else if (args->op_flags & XFS_DA_OP_REPLACE)
                xfs_attr_dela_state_set_replace(attr, XFS_DAS_LEAF_REPLACE);
        else
                attr->xattri_dela_state = XFS_DAS_DONE;
@@ -511,7 +511,7 @@ xfs_attr_node_addname(
 
        if (args->rmtblkno)
                attr->xattri_dela_state = XFS_DAS_NODE_SET_RMT;
-       else if (args->op_flags & XFS_DA_OP_RENAME)
+       else if (args->op_flags & XFS_DA_OP_REPLACE)
                xfs_attr_dela_state_set_replace(attr, XFS_DAS_NODE_REPLACE);
        else
                attr->xattri_dela_state = XFS_DAS_DONE;
@@ -547,7 +547,7 @@ xfs_attr_rmtval_alloc(
                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->op_flags & XFS_DA_OP_REPLACE)) {
                error = xfs_attr3_leaf_clearflag(args);
                attr->xattri_dela_state = XFS_DAS_DONE;
        } else {
@@ -966,8 +966,6 @@ xfs_attr_set(
 
        if (args->value) {
                XFS_STATS_INC(mp, xs_attr_set);
-
-               args->op_flags |= XFS_DA_OP_ADDNAME;
                args->total = xfs_attr_calc_size(args, &local);
 
                /*
@@ -1125,28 +1123,41 @@ static inline int xfs_attr_sf_totsize(struct xfs_inode *dp)
  * Add a name to the shortform attribute list structure
  * This is the external routine.
  */
-STATIC int
-xfs_attr_shortform_addname(xfs_da_args_t *args)
+static int
+xfs_attr_shortform_addname(
+       struct xfs_da_args      *args)
 {
-       int newsize, forkoff, retval;
+       int                     newsize, forkoff;
+       int                     error;
 
        trace_xfs_attr_sf_addname(args);
 
-       retval = xfs_attr_shortform_lookup(args);
-       if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
-               return retval;
-       if (retval == -EEXIST) {
-               if (args->attr_flags & XATTR_CREATE)
-                       return retval;
-               retval = xfs_attr_sf_removename(args);
-               if (retval)
-                       return retval;
+       error = xfs_attr_shortform_lookup(args);
+       switch (error) {
+       case -ENOATTR:
+               if (args->op_flags & XFS_DA_OP_REPLACE)
+                       return error;
+               break;
+       case -EEXIST:
+               if (!(args->op_flags & XFS_DA_OP_REPLACE))
+                       return error;
+
+               error = xfs_attr_sf_removename(args);
+               if (error)
+                       return error;
+
                /*
-                * Since we have removed the old attr, clear ATTR_REPLACE so
-                * that the leaf format add routine won't trip over the attr
-                * not being around.
+                * Since we have removed the old attr, clear XFS_DA_OP_REPLACE
+                * so that the new attr doesn't fit in shortform format, the
+                * leaf format add routine won't trip over the attr not being
+                * around.
                 */
-               args->attr_flags &= ~XATTR_REPLACE;
+               args->op_flags &= ~XFS_DA_OP_REPLACE;
+               break;
+       case 0:
+               break;
+       default:
+               return error;
        }
 
        if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
@@ -1169,8 +1180,8 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
  * External routines when attribute list is one block
  *========================================================================*/
 
-/* Store info about a remote block */
-STATIC void
+/* Save the current remote block info and clear the current pointers. */
+static void
 xfs_attr_save_rmt_blk(
        struct xfs_da_args      *args)
 {
@@ -1179,10 +1190,13 @@ xfs_attr_save_rmt_blk(
        args->rmtblkno2 = args->rmtblkno;
        args->rmtblkcnt2 = args->rmtblkcnt;
        args->rmtvaluelen2 = args->rmtvaluelen;
+       args->rmtblkno = 0;
+       args->rmtblkcnt = 0;
+       args->rmtvaluelen = 0;
 }
 
 /* Set stored info about a remote block */
-STATIC void
+static void
 xfs_attr_restore_rmt_blk(
        struct xfs_da_args      *args)
 {
@@ -1228,28 +1242,27 @@ xfs_attr_leaf_try_add(
         * Look up the xattr name to set the insertion point for the new xattr.
         */
        error = xfs_attr3_leaf_lookup_int(bp, args);
-       if (error != -ENOATTR && error != -EEXIST)
-               goto out_brelse;
-       if (error == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
-               goto out_brelse;
-       if (error == -EEXIST) {
-               if (args->attr_flags & XATTR_CREATE)
+       switch (error) {
+       case -ENOATTR:
+               if (args->op_flags & XFS_DA_OP_REPLACE)
+                       goto out_brelse;
+               break;
+       case -EEXIST:
+               if (!(args->op_flags & XFS_DA_OP_REPLACE))
                        goto out_brelse;
 
                trace_xfs_attr_leaf_replace(args);
-
-               /* save the attribute state for later removal*/
-               args->op_flags |= XFS_DA_OP_RENAME;     /* an atomic rename */
-               xfs_attr_save_rmt_blk(args);
-
                /*
-                * clear the remote attr state now that it is saved so that the
-                * values reflect the state of the attribute we are about to
+                * Save the existing remote attr state so that the current
+                * values reflect the state of the new attribute we are about to
                 * add, not the attribute we just found and will remove later.
                 */
-               args->rmtblkno = 0;
-               args->rmtblkcnt = 0;
-               args->rmtvaluelen = 0;
+               xfs_attr_save_rmt_blk(args);
+               break;
+       case 0:
+               break;
+       default:
+               goto out_brelse;
        }
 
        return xfs_attr3_leaf_add(bp, args);
@@ -1388,46 +1401,45 @@ xfs_attr_node_hasname(
 
 STATIC int
 xfs_attr_node_addname_find_attr(
-        struct xfs_attr_item           *attr)
+        struct xfs_attr_item   *attr)
 {
-       struct xfs_da_args              *args = attr->xattri_da_args;
-       int                             retval;
+       struct xfs_da_args      *args = attr->xattri_da_args;
+       int                     error;
 
        /*
         * Search to see if name already exists, and get back a pointer
         * to where it should go.
         */
-       retval = xfs_attr_node_hasname(args, &attr->xattri_da_state);
-       if (retval != -ENOATTR && retval != -EEXIST)
-               goto error;
-
-       if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
-               goto error;
-       if (retval == -EEXIST) {
-               if (args->attr_flags & XATTR_CREATE)
+       error = xfs_attr_node_hasname(args, &attr->xattri_da_state);
+       switch (error) {
+       case -ENOATTR:
+               if (args->op_flags & XFS_DA_OP_REPLACE)
+                       goto error;
+               break;
+       case -EEXIST:
+               if (!(args->op_flags & XFS_DA_OP_REPLACE))
                        goto error;
 
-               trace_xfs_attr_node_replace(args);
-
-               /* save the attribute state for later removal*/
-               args->op_flags |= XFS_DA_OP_RENAME;     /* atomic rename op */
-               xfs_attr_save_rmt_blk(args);
 
+               trace_xfs_attr_node_replace(args);
                /*
-                * clear the remote attr state now that it is saved so that the
-                * values reflect the state of the attribute we are about to
+                * Save the existing remote attr state so that the current
+                * values reflect the state of the new attribute we are about to
                 * add, not the attribute we just found and will remove later.
                 */
-               args->rmtblkno = 0;
-               args->rmtblkcnt = 0;
-               args->rmtvaluelen = 0;
+               xfs_attr_save_rmt_blk(args);
+               break;
+       case 0:
+               break;
+       default:
+               goto error;
        }
 
        return 0;
 error:
        if (attr->xattri_da_state)
                xfs_da_state_free(attr->xattri_da_state);
-       return retval;
+       return error;
 }
 
 /*

diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 41d70ad..689a966 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -584,7 +584,6 @@ xfs_attr_is_shortform(
 static inline enum xfs_delattr_state
 xfs_attr_init_add_state(struct xfs_da_args *args)
 {
-
        /*
         * When called from the completion of a attr remove to determine the
         * next state, the attribute fork may be null. This can occur only occur
@@ -595,6 +594,8 @@ xfs_attr_init_add_state(struct xfs_da_args *args)
         */
        if (!args->dp->i_afp)
                return XFS_DAS_DONE;
+
+       args->op_flags |= XFS_DA_OP_ADDNAME;
        if (xfs_attr_is_shortform(args->dp))
                return XFS_DAS_SF_ADD;
        if (xfs_attr_is_leaf(args->dp))
@@ -605,6 +606,7 @@ xfs_attr_init_add_state(struct xfs_da_args *args)
 static inline enum xfs_delattr_state
 xfs_attr_init_remove_state(struct xfs_da_args *args)
 {
+       args->op_flags |= XFS_DA_OP_REMOVE;
        if (xfs_attr_is_shortform(args->dp))
                return XFS_DAS_SF_REMOVE;
        if (xfs_attr_is_leaf(args->dp))
@@ -615,6 +617,7 @@ xfs_attr_init_remove_state(struct xfs_da_args *args)
 static inline enum xfs_delattr_state
 xfs_attr_init_replace_state(struct xfs_da_args *args)
 {
+       args->op_flags |= XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE;
        return xfs_attr_init_add_state(args);
 }
 

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index e90bfd9..53d02ce 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -1492,7 +1492,7 @@ xfs_attr3_leaf_add_work(
        entry->flags = args->attr_filter;
        if (tmp)
                entry->flags |= XFS_ATTR_LOCAL;
-       if (args->op_flags & XFS_DA_OP_RENAME) {
+       if (args->op_flags & XFS_DA_OP_REPLACE) {
                if (!xfs_has_larp(mp))
                        entry->flags |= XFS_ATTR_INCOMPLETE;
                if ((args->blkno2 == args->blkno) &&

diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index deb368d..468ca70 100644 (file)
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -85,19 +85,21 @@ typedef struct xfs_da_args {
  * Operation flags:
  */
 #define XFS_DA_OP_JUSTCHECK    (1u << 0) /* check for ok with no space */
-#define XFS_DA_OP_RENAME       (1u << 1) /* this is an atomic rename op */
+#define XFS_DA_OP_REPLACE      (1u << 1) /* this is an atomic replace op */
 #define XFS_DA_OP_ADDNAME      (1u << 2) /* this is an add operation */
 #define XFS_DA_OP_OKNOENT      (1u << 3) /* lookup op, ENOENT ok, else die */
 #define XFS_DA_OP_CILOOKUP     (1u << 4) /* lookup returns CI name if found */
 #define XFS_DA_OP_NOTIME       (1u << 5) /* don't update inode timestamps */
+#define XFS_DA_OP_REMOVE       (1u << 6) /* this is a remove operation */
 
 #define XFS_DA_OP_FLAGS \
        { XFS_DA_OP_JUSTCHECK,  "JUSTCHECK" }, \
-       { XFS_DA_OP_RENAME,     "RENAME" }, \
+       { XFS_DA_OP_REPLACE,    "REPLACE" }, \
        { XFS_DA_OP_ADDNAME,    "ADDNAME" }, \
        { XFS_DA_OP_OKNOENT,    "OKNOENT" }, \
        { XFS_DA_OP_CILOOKUP,   "CILOOKUP" }, \
-       { XFS_DA_OP_NOTIME,     "NOTIME" }
+       { XFS_DA_OP_NOTIME,     "NOTIME" }, \
+       { XFS_DA_OP_REMOVE,     "REMOVE" }
 
 /*
  * Storage for holding state during Btree searches and split/join ops.