]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
xfs: remove xfs_attri_remove_iter
authorDave Chinner <dchinner@redhat.com>
Wed, 22 Jun 2022 19:28:52 +0000 (14:28 -0500)
committerEric Sandeen <sandeen@sandeen.net>
Wed, 22 Jun 2022 19:28:52 +0000 (14:28 -0500)
Source kernel commit: 59782a236b622a983ff101b2cb1333f714e4ed4e

xfs_attri_remove_iter is not used anymore, so remove it and all the
infrastructure it uses and is needed to drive it. THe
xfs_attr_refillstate() function now throws an unused warning, so
isolate the xfs_attr_fillstate()/xfs_attr_refillstate() code pair
with an #if 0 and a comment explaining why we want to keep this code
and restore the optimisation it provides in the near future.

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>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
libxfs/xfs_attr.c

index 87d737237114c7395e36c8aea0489c49c4be670a..0d5e47297a75608e0a1a1362d524830fb3ac6b97 100644 (file)
@@ -61,10 +61,6 @@ STATIC int xfs_attr_node_addname_find_attr(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);
-STATIC int xfs_attr_refillstate(xfs_da_state_t *state);
-STATIC int xfs_attr_node_removename(struct xfs_da_args *args,
-                                   struct xfs_da_state *state);
 
 int
 xfs_inode_hasattr(
@@ -97,6 +93,123 @@ xfs_attr_is_leaf(
        return imap.br_startoff == 0 && imap.br_blockcount == 1;
 }
 
+/*
+ * XXX (dchinner): name path state saving and refilling is an optimisation to
+ * avoid needing to look up name entries after rolling transactions removing
+ * remote xattr blocks between the name entry lookup and name entry removal.
+ * This optimisation got sidelined when combining the set and remove state
+ * machines, but the code has been left in place because it is worthwhile to
+ * restore the optimisation once the combined state machine paths have settled.
+ *
+ * This comment is a public service announcement to remind Future Dave that he
+ * still needs to restore this code to working order.
+ */
+#if 0
+/*
+ * Fill in the disk block numbers in the state structure for the buffers
+ * that are attached to the state structure.
+ * This is done so that we can quickly reattach ourselves to those buffers
+ * after some set of transaction commits have released these buffers.
+ */
+static int
+xfs_attr_fillstate(xfs_da_state_t *state)
+{
+       xfs_da_state_path_t *path;
+       xfs_da_state_blk_t *blk;
+       int level;
+
+       trace_xfs_attr_fillstate(state->args);
+
+       /*
+        * Roll down the "path" in the state structure, storing the on-disk
+        * block number for those buffers in the "path".
+        */
+       path = &state->path;
+       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
+       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
+               if (blk->bp) {
+                       blk->disk_blkno = xfs_buf_daddr(blk->bp);
+                       blk->bp = NULL;
+               } else {
+                       blk->disk_blkno = 0;
+               }
+       }
+
+       /*
+        * Roll down the "altpath" in the state structure, storing the on-disk
+        * block number for those buffers in the "altpath".
+        */
+       path = &state->altpath;
+       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
+       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
+               if (blk->bp) {
+                       blk->disk_blkno = xfs_buf_daddr(blk->bp);
+                       blk->bp = NULL;
+               } else {
+                       blk->disk_blkno = 0;
+               }
+       }
+
+       return 0;
+}
+
+/*
+ * Reattach the buffers to the state structure based on the disk block
+ * numbers stored in the state structure.
+ * This is done after some set of transaction commits have released those
+ * buffers from our grip.
+ */
+static int
+xfs_attr_refillstate(xfs_da_state_t *state)
+{
+       xfs_da_state_path_t *path;
+       xfs_da_state_blk_t *blk;
+       int level, error;
+
+       trace_xfs_attr_refillstate(state->args);
+
+       /*
+        * Roll down the "path" in the state structure, storing the on-disk
+        * block number for those buffers in the "path".
+        */
+       path = &state->path;
+       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
+       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
+               if (blk->disk_blkno) {
+                       error = xfs_da3_node_read_mapped(state->args->trans,
+                                       state->args->dp, blk->disk_blkno,
+                                       &blk->bp, XFS_ATTR_FORK);
+                       if (error)
+                               return error;
+               } else {
+                       blk->bp = NULL;
+               }
+       }
+
+       /*
+        * Roll down the "altpath" in the state structure, storing the on-disk
+        * block number for those buffers in the "altpath".
+        */
+       path = &state->altpath;
+       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
+       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
+               if (blk->disk_blkno) {
+                       error = xfs_da3_node_read_mapped(state->args->trans,
+                                       state->args->dp, blk->disk_blkno,
+                                       &blk->bp, XFS_ATTR_FORK);
+                       if (error)
+                               return error;
+               } else {
+                       blk->bp = NULL;
+               }
+       }
+
+       return 0;
+}
+#else
+static int xfs_attr_fillstate(xfs_da_state_t *state) { return 0; }
+#endif
+
 /*========================================================================
  * Overall external interface routines.
  *========================================================================*/
@@ -545,25 +658,16 @@ xfs_attr_leaf_remove_attr(
  */
 static int
 xfs_attr_leaf_shrink(
-       struct xfs_da_args      *args,
-       struct xfs_da_state     *state)
+       struct xfs_da_args      *args)
 {
        struct xfs_inode        *dp = args->dp;
-       int                     error, forkoff;
        struct xfs_buf          *bp;
+       int                     forkoff;
+       int                     error;
 
        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;
@@ -706,7 +810,7 @@ next_state:
        case XFS_DAS_NODE_REMOVE_ATTR:
                error = xfs_attr_node_remove_attr(attr);
                if (!error)
-                       error = xfs_attr_leaf_shrink(args, NULL);
+                       error = xfs_attr_leaf_shrink(args);
                attr->xattri_dela_state = XFS_DAS_DONE;
                break;
        default:
@@ -1377,6 +1481,24 @@ out:
        return error;
 }
 
+static int
+xfs_attr_node_removename(
+       struct xfs_da_args      *args,
+       struct xfs_da_state     *state)
+{
+       struct xfs_da_state_blk *blk;
+       int                     retval;
+
+       /*
+        * Remove the name and update the hashvals in the tree.
+        */
+       blk = &state->path.blk[state->path.active-1];
+       ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
+       retval = xfs_attr3_leaf_remove(blk->bp, args);
+       xfs_da3_fixhashpath(state, &state->path);
+
+       return retval;
+}
 
 static int
 xfs_attr_node_remove_attr(
@@ -1419,268 +1541,6 @@ out:
        return retval;
 }
 
-
-STATIC int
-xfs_attr_node_removename(
-       struct xfs_da_args      *args,
-       struct xfs_da_state     *state)
-{
-       struct xfs_da_state_blk *blk;
-       int                     retval;
-
-       /*
-        * Remove the name and update the hashvals in the tree.
-        */
-       blk = &state->path.blk[state->path.active-1];
-       ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
-       retval = xfs_attr3_leaf_remove(blk->bp, args);
-       xfs_da3_fixhashpath(state, &state->path);
-
-       return retval;
-}
-
-/*
- * Remove the attribute specified in @args.
- *
- * This will involve walking down the Btree, and may involve joining
- * leaf nodes and even joining intermediate nodes up to and including
- * the root node (a special case of an intermediate node).
- *
- * This routine is meant to function as either an in-line or delayed operation,
- * and may return -EAGAIN when the transaction needs to be rolled.  Calling
- * functions will need to handle this, and call the function until a
- * successful error code is returned.
- */
-int
-xfs_attr_remove_iter(
-       struct xfs_attr_item            *attr)
-{
-       struct xfs_da_args              *args = attr->xattri_da_args;
-       struct xfs_da_state             *state = attr->xattri_da_state;
-       int                             retval, error = 0;
-       struct xfs_inode                *dp = args->dp;
-
-       trace_xfs_attr_node_removename(args);
-
-       switch (attr->xattri_dela_state) {
-       case XFS_DAS_UNINIT:
-               if (!xfs_inode_hasattr(dp))
-                       return -ENOATTR;
-
-               /*
-                * Shortform or leaf formats don't require transaction rolls and
-                * thus state transitions. Call the right helper and return.
-                */
-               if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
-                       return xfs_attr_sf_removename(args);
-
-               if (xfs_attr_is_leaf(dp))
-                       return xfs_attr_leaf_removename(args);
-
-               /*
-                * Node format may require transaction rolls. Set up the
-                * state context and fall into the state machine.
-                */
-               if (!attr->xattri_da_state) {
-                       error = xfs_attr_node_removename_setup(attr);
-                       if (error)
-                               return error;
-                       state = attr->xattri_da_state;
-               }
-
-               fallthrough;
-       case XFS_DAS_RMTBLK:
-               attr->xattri_dela_state = XFS_DAS_RMTBLK;
-
-               /*
-                * If there is an out-of-line value, de-allocate the blocks.
-                * This is done before we remove the attribute so that we don't
-                * overflow the maximum size of a transaction and/or hit a
-                * deadlock.
-                */
-               if (args->rmtblkno > 0) {
-                       /*
-                        * May return -EAGAIN. Roll and repeat until all remote
-                        * blocks are removed.
-                        */
-                       error = xfs_attr_rmtval_remove(attr);
-                       if (error == -EAGAIN) {
-                               trace_xfs_attr_remove_iter_return(
-                                       attr->xattri_dela_state, args->dp);
-                               return error;
-                       } else if (error) {
-                               goto out;
-                       }
-
-                       /*
-                        * Refill the state structure with buffers (the prior
-                        * calls released our buffers) and close out this
-                        * transaction before proceeding.
-                        */
-                       ASSERT(args->rmtblkno == 0);
-                       error = xfs_attr_refillstate(state);
-                       if (error)
-                               goto out;
-
-                       attr->xattri_dela_state = XFS_DAS_RM_NAME;
-                       trace_xfs_attr_remove_iter_return(
-                                       attr->xattri_dela_state, args->dp);
-                       return -EAGAIN;
-               }
-
-               fallthrough;
-       case XFS_DAS_RM_NAME:
-               /*
-                * If we came here fresh from a transaction roll, reattach all
-                * the buffers to the current transaction.
-                */
-               if (attr->xattri_dela_state == XFS_DAS_RM_NAME) {
-                       error = xfs_attr_refillstate(state);
-                       if (error)
-                               goto out;
-               }
-
-               retval = xfs_attr_node_removename(args, state);
-
-               /*
-                * Check to see if the tree needs to be collapsed. If so, roll
-                * the transacton and fall into the shrink state.
-                */
-               if (retval && (state->path.active > 1)) {
-                       error = xfs_da3_join(state);
-                       if (error)
-                               goto out;
-
-                       attr->xattri_dela_state = XFS_DAS_RM_SHRINK;
-                       trace_xfs_attr_remove_iter_return(
-                                       attr->xattri_dela_state, args->dp);
-                       return -EAGAIN;
-               }
-
-               fallthrough;
-       case XFS_DAS_RM_SHRINK:
-               /*
-                * If the result is small enough, push it all into the inode.
-                * This is our final state so it's safe to return a dirty
-                * transaction.
-                */
-               if (xfs_attr_is_leaf(dp))
-                       error = xfs_attr_leaf_shrink(args, state);
-               ASSERT(error != -EAGAIN);
-               break;
-       default:
-               ASSERT(0);
-               error = -EINVAL;
-               goto out;
-       }
-out:
-       if (state)
-               xfs_da_state_free(state);
-       return error;
-}
-
-/*
- * Fill in the disk block numbers in the state structure for the buffers
- * that are attached to the state structure.
- * This is done so that we can quickly reattach ourselves to those buffers
- * after some set of transaction commits have released these buffers.
- */
-STATIC int
-xfs_attr_fillstate(xfs_da_state_t *state)
-{
-       xfs_da_state_path_t *path;
-       xfs_da_state_blk_t *blk;
-       int level;
-
-       trace_xfs_attr_fillstate(state->args);
-
-       /*
-        * Roll down the "path" in the state structure, storing the on-disk
-        * block number for those buffers in the "path".
-        */
-       path = &state->path;
-       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
-       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
-               if (blk->bp) {
-                       blk->disk_blkno = xfs_buf_daddr(blk->bp);
-                       blk->bp = NULL;
-               } else {
-                       blk->disk_blkno = 0;
-               }
-       }
-
-       /*
-        * Roll down the "altpath" in the state structure, storing the on-disk
-        * block number for those buffers in the "altpath".
-        */
-       path = &state->altpath;
-       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
-       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
-               if (blk->bp) {
-                       blk->disk_blkno = xfs_buf_daddr(blk->bp);
-                       blk->bp = NULL;
-               } else {
-                       blk->disk_blkno = 0;
-               }
-       }
-
-       return 0;
-}
-
-/*
- * Reattach the buffers to the state structure based on the disk block
- * numbers stored in the state structure.
- * This is done after some set of transaction commits have released those
- * buffers from our grip.
- */
-STATIC int
-xfs_attr_refillstate(xfs_da_state_t *state)
-{
-       xfs_da_state_path_t *path;
-       xfs_da_state_blk_t *blk;
-       int level, error;
-
-       trace_xfs_attr_refillstate(state->args);
-
-       /*
-        * Roll down the "path" in the state structure, storing the on-disk
-        * block number for those buffers in the "path".
-        */
-       path = &state->path;
-       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
-       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
-               if (blk->disk_blkno) {
-                       error = xfs_da3_node_read_mapped(state->args->trans,
-                                       state->args->dp, blk->disk_blkno,
-                                       &blk->bp, XFS_ATTR_FORK);
-                       if (error)
-                               return error;
-               } else {
-                       blk->bp = NULL;
-               }
-       }
-
-       /*
-        * Roll down the "altpath" in the state structure, storing the on-disk
-        * block number for those buffers in the "altpath".
-        */
-       path = &state->altpath;
-       ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
-       for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
-               if (blk->disk_blkno) {
-                       error = xfs_da3_node_read_mapped(state->args->trans,
-                                       state->args->dp, blk->disk_blkno,
-                                       &blk->bp, XFS_ATTR_FORK);
-                       if (error)
-                               return error;
-               } else {
-                       blk->bp = NULL;
-               }
-       }
-
-       return 0;
-}
-
 /*
  * Retrieve the attribute data from a node attribute list.
  *