[PATCH v4 16/17] xfs: Add delay ready attr remove routines

Allison Collins
2019-11-07T01:28:56Z

This patch modifies the attr remove routines to be delay ready.
This means they no longer roll or commit transactions, but instead
return -EAGAIN to have the calling routine roll and refresh the
transaction. In this series, xfs_attr_remove_args has become
xfs_attr_remove_later, which uses a state machine to keep track
of where it was when EAGAIN was returned. xfs_attr_node_removename
has also been modified to use the state machine, and a new version of
xfs_attr_remove_args consists of a simple loop to refresh the
transaction until the operation is completed.

Signed-off-by: Allison Collins <allison.henderson@oracle.com>
---
fs/xfs/libxfs/xfs_attr.c | 123 +++++++++++++++++++++++++++++++++++++++--------
fs/xfs/libxfs/xfs_attr.h | 1 +
2 files changed, 104 insertions(+), 20 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 626d4a98..38d5c5c 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -369,10 +369,56 @@ xfs_has_attr(
*/
int
xfs_attr_remove_args(
+ struct xfs_da_args *args)
+{
+ int error = 0;
+ int err2 = 0;
+
+ do {
+ error = xfs_attr_remove_later(args);
+ if (error && error != -EAGAIN)
+ goto out;
+
+ xfs_trans_log_inode(args->trans, args->dp,
+ XFS_ILOG_CORE | XFS_ILOG_ADATA);
+
+ err2 = xfs_trans_roll(&args->trans);
+ if (err2) {
+ error = err2;
+ goto out;
+ }
+
+ /* Rejoin inode */
+ xfs_trans_ijoin(args->trans, args->dp, 0);
+
+ } while (error == -EAGAIN);
+out:
+ return error;
+}
+
+/*
+ * Remove the attribute specified in @args.
+ * This routine is meant to function as a delayed operation, and may return
+ * -EGAIN when the transaction needs to be rolled. Calling functions will need
+ * to handle this, and recall the function until a successful error code is
+ * returned.
+ */
+int
+xfs_attr_remove_later(
struct xfs_da_args *args)
{
struct xfs_inode *dp = args->dp;
- int error;
+ int error = 0;
+
+ /* State machine switch */
+ switch (args->dc.dc_state) {
+ case XFS_DC_RM_INVALIDATE:
+ case XFS_DC_RM_SHRINK:
+ case XFS_DC_RM_NODE_BLKS:
+ goto node;
+ default:
+ break;
+ }

if (!xfs_inode_hasattr(dp)) {
error = -ENOATTR;
@@ -382,6 +428,7 @@ xfs_attr_remove_args(
} else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
error = xfs_attr_leaf_removename(args);
} else {
+node:
error = xfs_attr_node_removename(args);
}

@@ -892,9 +939,6 @@ xfs_attr_leaf_removename(
/* bp is gone due to xfs_da_shrink_inode */
if (error)
return error;
- error = xfs_defer_finish(&args->trans);
- if (error)
- return error;
}
return 0;
}
@@ -1212,6 +1256,11 @@ xfs_attr_node_addname(
* 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 a delayed operation, and may return
+ * -EAGAIN when the transaction needs to be rolled. Calling functions
+ * will need to handle this, and recall the function until a successful error
+ * code is returned.
*/
STATIC int
xfs_attr_node_removename(
@@ -1222,12 +1271,29 @@ xfs_attr_node_removename(
struct xfs_buf *bp;
int retval, error, forkoff;
struct xfs_inode *dp = args->dp;
+ int done = 0;

trace_xfs_attr_node_removename(args);
+ state = args->dc.da_state;
+ blk = args->dc.blk;
+
+ /* State machine switch */
+ switch (args->dc.dc_state) {
+ case XFS_DC_RM_NODE_BLKS:
+ goto rm_node_blks;
+ case XFS_DC_RM_INVALIDATE:
+ goto rm_invalidate;
+ case XFS_DC_RM_SHRINK:
+ goto rm_shrink;
+ default:
+ break;
+ }

error = xfs_attr_node_hasname(args, &state);
if (error != -EEXIST)
goto out;
+ else
+ error = 0;

/*
* If there is an out-of-line value, de-allocate the blocks.
@@ -1237,6 +1303,14 @@ xfs_attr_node_removename(
blk = &state->path.blk[ state->path.active-1 ];
ASSERT(blk->bp != NULL);
ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
+
+ /*
+ * Store blk and state in the context incase we need to cycle out the
+ * transaction
+ */
+ args->dc.blk = blk;
+ args->dc.da_state = state;
+
if (args->rmtblkno > 0) {
/*
* Fill in disk block numbers in the state structure
@@ -1255,13 +1329,30 @@ xfs_attr_node_removename(
if (error)
goto out;

- error = xfs_trans_roll_inode(&args->trans, args->dp);
+ args->dc.dc_state = XFS_DC_RM_INVALIDATE;
+ return -EAGAIN;
+rm_invalidate:
+ error = xfs_attr_rmtval_invalidate(args);
if (error)
goto out;
+rm_node_blks:
+ /*
+ * Unmap value blocks for this attr. This is similar to
+ * xfs_attr_rmtval_remove, but open coded here to return EAGAIN
+ * for new transactions
+ */
+ while (!done && !error) {
+ error = xfs_bunmapi(args->trans, args->dp,
+ args->rmtblkno, args->rmtblkcnt,
+ XFS_BMAPI_ATTRFORK, 1, &done);
+ if (error)
+ return error;

- error = xfs_attr_rmtval_remove(args);
- if (error)
- goto out;
+ if (!done) {
+ args->dc.dc_state = XFS_DC_RM_NODE_BLKS;
+ return -EAGAIN;
+ }
+ }

/*
* Refill the state structure with buffers, the prior calls
@@ -1287,17 +1378,12 @@ xfs_attr_node_removename(
error = xfs_da3_join(state);
if (error)
goto out;
- error = xfs_defer_finish(&args->trans);
- if (error)
- goto out;
- /*
- * Commit the Btree join operation and start a new trans.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- goto out;
+
+ args->dc.dc_state = XFS_DC_RM_SHRINK;
+ return -EAGAIN;
}

+rm_shrink:
/*
* If the result is small enough, push it all into the inode.
*/
@@ -1319,9 +1405,6 @@ xfs_attr_node_removename(
/* bp is gone due to xfs_da_shrink_inode */
if (error)
goto out;
- error = xfs_defer_finish(&args->trans);
- if (error)
- goto out;
} else
xfs_trans_brelse(args->trans, bp);
}
diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 3b5dad4..fb8bf5b 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -152,6 +152,7 @@ int xfs_attr_set_args(struct xfs_da_args *args);
int xfs_attr_remove(struct xfs_inode *dp, struct xfs_name *name, int flags);
int xfs_has_attr(struct xfs_da_args *args);
int xfs_attr_remove_args(struct xfs_da_args *args);
+int xfs_attr_remove_later(struct xfs_da_args *args);
int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
int flags, struct attrlist_cursor_kern *cursor);
bool xfs_attr_namecheck(const void *name, size_t length);
--
2.7.4