* [PATCH v22 01/27] xfsprogs: Reverse apply 72b97ea40d
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 02/27] xfsprogs: Add xfs_attr_node_remove_name Allison Henderson
` (25 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Originally we added this patch to help modularize the attr code in
preparation for delayed attributes and the state machine it requires.
However, later reviews found that this slightly alters the transaction
handling as the helper function is ambiguous as to whether the
transaction is diry or clean. This may cause a dirty transaction to be
included in the next roll, where previously it had not. To preserve the
existing code flow, we reverse apply this commit.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
libxfs/xfs_attr.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index dce7ded..1c60bdd 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -1214,24 +1214,6 @@ int xfs_attr_node_removename_setup(
return 0;
}
-STATIC int
-xfs_attr_node_remove_rmt(
- struct xfs_da_args *args,
- struct xfs_da_state *state)
-{
- int error = 0;
-
- error = xfs_attr_rmtval_remove(args);
- if (error)
- return error;
-
- /*
- * Refill the state structure with buffers, the prior calls released our
- * buffers.
- */
- return xfs_attr_refillstate(state);
-}
-
/*
* Remove a name from a B-tree attribute list.
*
@@ -1260,7 +1242,15 @@ xfs_attr_node_removename(
* overflow the maximum size of a transaction and/or hit a deadlock.
*/
if (args->rmtblkno > 0) {
- error = xfs_attr_node_remove_rmt(args, state);
+ error = xfs_attr_rmtval_remove(args);
+ if (error)
+ goto out;
+
+ /*
+ * Refill the state structure with buffers, the prior calls
+ * released our buffers.
+ */
+ error = xfs_attr_refillstate(state);
if (error)
goto out;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 02/27] xfsprogs: Add xfs_attr_node_remove_name
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
2021-07-27 6:18 ` [PATCH v22 01/27] xfsprogs: Reverse apply 72b97ea40d Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 03/27] xfsprogs: Refactor xfs_attr_set_shortform Allison Henderson
` (24 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 4078af18570ef0f89fce18e9ed9c1fa0c827f37b
This patch pulls a new helper function xfs_attr_node_remove_name out
of xfs_attr_node_remove_step. This helps to modularize
xfs_attr_node_remove_step which will help make the delayed attribute
code easier to follow
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
libxfs/xfs_attr.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 1c60bdd..5da3ec3 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -1214,6 +1214,25 @@ int xfs_attr_node_removename_setup(
return 0;
}
+STATIC int
+xfs_attr_node_remove_name(
+ 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 a name from a B-tree attribute list.
*
@@ -1226,7 +1245,6 @@ xfs_attr_node_removename(
struct xfs_da_args *args)
{
struct xfs_da_state *state;
- struct xfs_da_state_blk *blk;
int retval, error;
struct xfs_inode *dp = args->dp;
@@ -1254,14 +1272,7 @@ xfs_attr_node_removename(
if (error)
goto out;
}
-
- /*
- * 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);
+ retval = xfs_attr_node_remove_name(args, state);
/*
* Check to see if the tree needs to be collapsed.
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 03/27] xfsprogs: Refactor xfs_attr_set_shortform
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
2021-07-27 6:18 ` [PATCH v22 01/27] xfsprogs: Reverse apply 72b97ea40d Allison Henderson
2021-07-27 6:18 ` [PATCH v22 02/27] xfsprogs: Add xfs_attr_node_remove_name Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 04/27] xfsprogs: Separate xfs_attr_node_addname and xfs_attr_node_addname_clear_incomplete Allison Henderson
` (23 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
This patch is actually the combination of patches from the previous
version (v18). Initially patch 3 hoisted xfs_attr_set_shortform, and
the next added the helper xfs_attr_set_fmt. xfs_attr_set_fmt is similar
the old xfs_attr_set_shortform. It returns 0 when the attr has been set
and no further action is needed. It returns -EAGAIN when shortform has
been transformed to leaf, and the calling function should proceed the
set the attr in leaf form.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
---
libxfs/xfs_attr.c | 42 ++++++++++++++----------------------------
1 file changed, 14 insertions(+), 28 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 5da3ec3..b181777 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -236,16 +236,11 @@ xfs_attr_is_shortform(
ip->i_afp->if_nextents == 0);
}
-/*
- * Attempts to set an attr in shortform, or converts short form to leaf form if
- * there is not enough room. If the attr is set, the transaction is committed
- * and set to NULL.
- */
STATIC int
-xfs_attr_set_shortform(
- struct xfs_da_args *args,
- struct xfs_buf **leaf_bp)
+xfs_attr_set_fmt(
+ struct xfs_da_args *args)
{
+ struct xfs_buf *leaf_bp = NULL;
struct xfs_inode *dp = args->dp;
int error, error2 = 0;
@@ -258,29 +253,29 @@ xfs_attr_set_shortform(
args->trans = NULL;
return error ? error : error2;
}
+
/*
* It won't fit in the shortform, transform to a leaf block. GROT:
* another possible req'mt for a double-split btree op.
*/
- error = xfs_attr_shortform_to_leaf(args, leaf_bp);
+ error = xfs_attr_shortform_to_leaf(args, &leaf_bp);
if (error)
return error;
/*
* Prevent the leaf buffer from being unlocked so that a concurrent AIL
* push cannot grab the half-baked leaf buffer and run into problems
- * with the write verifier. Once we're done rolling the transaction we
- * can release the hold and add the attr to the leaf.
+ * with the write verifier.
*/
- xfs_trans_bhold(args->trans, *leaf_bp);
+ xfs_trans_bhold(args->trans, leaf_bp);
error = xfs_defer_finish(&args->trans);
- xfs_trans_bhold_release(args->trans, *leaf_bp);
+ xfs_trans_bhold_release(args->trans, leaf_bp);
if (error) {
- xfs_trans_brelse(args->trans, *leaf_bp);
+ xfs_trans_brelse(args->trans, leaf_bp);
return error;
}
- return 0;
+ return -EAGAIN;
}
/*
@@ -291,8 +286,7 @@ xfs_attr_set_args(
struct xfs_da_args *args)
{
struct xfs_inode *dp = args->dp;
- struct xfs_buf *leaf_bp = NULL;
- int error = 0;
+ int error;
/*
* If the attribute list is already in leaf format, jump straight to
@@ -301,15 +295,8 @@ xfs_attr_set_args(
* again.
*/
if (xfs_attr_is_shortform(dp)) {
-
- /*
- * If the attr was successfully set in shortform, the
- * transaction is committed and set to NULL. Otherwise, is it
- * converted from shortform to leaf, and the transaction is
- * retained.
- */
- error = xfs_attr_set_shortform(args, &leaf_bp);
- if (error || !args->trans)
+ error = xfs_attr_set_fmt(args);
+ if (error != -EAGAIN)
return error;
}
@@ -344,8 +331,7 @@ xfs_attr_set_args(
return error;
}
- error = xfs_attr_node_addname(args);
- return error;
+ return xfs_attr_node_addname(args);
}
/*
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 04/27] xfsprogs: Separate xfs_attr_node_addname and xfs_attr_node_addname_clear_incomplete
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (2 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 03/27] xfsprogs: Refactor xfs_attr_set_shortform Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 05/27] xfsprogs: Add helper xfs_attr_node_addname_find_attr Allison Henderson
` (22 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 392a7ea8080e3753aa179d4daaa2ad413d0ff441
This patch separate xfs_attr_node_addname into two functions. This will
help to make it easier to hoist parts of xfs_attr_node_addname that need
state management
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
libxfs/xfs_attr.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index b181777..158149a 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -54,6 +54,7 @@ STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp);
STATIC int xfs_attr_node_get(xfs_da_args_t *args);
STATIC int xfs_attr_node_addname(xfs_da_args_t *args);
STATIC int xfs_attr_node_removename(xfs_da_args_t *args);
+STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_da_args *args);
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);
@@ -1073,6 +1074,28 @@ restart:
return error;
}
+ error = xfs_attr_node_addname_clear_incomplete(args);
+ if (error)
+ goto out;
+ retval = 0;
+out:
+ if (state)
+ xfs_da_state_free(state);
+ if (error)
+ return error;
+ return retval;
+}
+
+
+STATIC int
+xfs_attr_node_addname_clear_incomplete(
+ struct xfs_da_args *args)
+{
+ struct xfs_da_state *state = NULL;
+ struct xfs_da_state_blk *blk;
+ int retval = 0;
+ int error = 0;
+
/*
* Re-find the "old" attribute entry after any split ops. The INCOMPLETE
* flag means that we will find the "old" attr, not the "new" one.
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 05/27] xfsprogs: Add helper xfs_attr_node_addname_find_attr
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (3 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 04/27] xfsprogs: Separate xfs_attr_node_addname and xfs_attr_node_addname_clear_incomplete Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 06/27] xfsprogs: Hoist xfs_attr_node_addname Allison Henderson
` (21 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 1aeec98c3f70bff14cc0e65ef898e91c2c554d41
This patch separates the first half of xfs_attr_node_addname into a
helper function xfs_attr_node_addname_find_attr. It also replaces the
restart goto with an EAGAIN return code driven by a loop in the calling
function. This looks odd now, but will clean up nicly once we introduce
the state machine. It will also enable hoisting the last state out of
xfs_attr_node_addname with out having to plumb in a "done" parameter to
know if we need to move to the next state or not.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
---
libxfs/xfs_attr.c | 87 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 54 insertions(+), 33 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 158149a..32a51d5 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -52,7 +52,10 @@ STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp);
* Internal routines when attribute list is more than one block.
*/
STATIC int xfs_attr_node_get(xfs_da_args_t *args);
-STATIC int xfs_attr_node_addname(xfs_da_args_t *args);
+STATIC int xfs_attr_node_addname(struct xfs_da_args *args,
+ struct xfs_da_state *state);
+STATIC int xfs_attr_node_addname_find_attr(struct xfs_da_args *args,
+ struct xfs_da_state **state);
STATIC int xfs_attr_node_removename(xfs_da_args_t *args);
STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_da_args *args);
STATIC int xfs_attr_node_hasname(xfs_da_args_t *args,
@@ -287,6 +290,7 @@ xfs_attr_set_args(
struct xfs_da_args *args)
{
struct xfs_inode *dp = args->dp;
+ struct xfs_da_state *state;
int error;
/*
@@ -332,7 +336,14 @@ xfs_attr_set_args(
return error;
}
- return xfs_attr_node_addname(args);
+ do {
+ error = xfs_attr_node_addname_find_attr(args, &state);
+ if (error)
+ return error;
+ error = xfs_attr_node_addname(args, state);
+ } while (error == -EAGAIN);
+
+ return error;
}
/*
@@ -896,48 +907,26 @@ xfs_attr_node_hasname(
* External routines when attribute list size > geo->blksize
*========================================================================*/
-/*
- * Add a name to a Btree-format attribute list.
- *
- * This will involve walking down the Btree, and may involve splitting
- * leaf nodes and even splitting intermediate nodes up to and including
- * the root node (a special case of an intermediate node).
- *
- * "Remote" attribute values confuse the issue and atomic rename operations
- * add a whole extra layer of confusion on top of that.
- */
STATIC int
-xfs_attr_node_addname(
- struct xfs_da_args *args)
+xfs_attr_node_addname_find_attr(
+ struct xfs_da_args *args,
+ struct xfs_da_state **state)
{
- struct xfs_da_state *state;
- struct xfs_da_state_blk *blk;
- struct xfs_inode *dp;
- int retval, error;
-
- trace_xfs_attr_node_addname(args);
+ int retval;
/*
- * Fill in bucket of arguments/results/context to carry around.
- */
- dp = args->dp;
-restart:
- /*
* Search to see if name already exists, and get back a pointer
* to where it should go.
*/
- error = 0;
- retval = xfs_attr_node_hasname(args, &state);
+ retval = xfs_attr_node_hasname(args, state);
if (retval != -ENOATTR && retval != -EEXIST)
- goto out;
+ goto error;
- blk = &state->path.blk[ state->path.active-1 ];
- ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
- goto out;
+ goto error;
if (retval == -EEXIST) {
if (args->attr_flags & XATTR_CREATE)
- goto out;
+ goto error;
trace_xfs_attr_node_replace(args);
@@ -955,6 +944,38 @@ restart:
args->rmtvaluelen = 0;
}
+ return 0;
+error:
+ if (*state)
+ xfs_da_state_free(*state);
+ return retval;
+}
+
+/*
+ * Add a name to a Btree-format attribute list.
+ *
+ * This will involve walking down the Btree, and may involve splitting
+ * leaf nodes and even splitting intermediate nodes up to and including
+ * the root node (a special case of an intermediate node).
+ *
+ * "Remote" attribute values confuse the issue and atomic rename operations
+ * add a whole extra layer of confusion on top of that.
+ */
+STATIC int
+xfs_attr_node_addname(
+ struct xfs_da_args *args,
+ struct xfs_da_state *state)
+{
+ struct xfs_da_state_blk *blk;
+ struct xfs_inode *dp;
+ int retval, error;
+
+ trace_xfs_attr_node_addname(args);
+
+ dp = args->dp;
+ blk = &state->path.blk[state->path.active-1];
+ ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
+
retval = xfs_attr3_leaf_add(blk->bp, state->args);
if (retval == -ENOSPC) {
if (state->path.active == 1) {
@@ -980,7 +1001,7 @@ restart:
if (error)
goto out;
- goto restart;
+ return -EAGAIN;
}
/*
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 06/27] xfsprogs: Hoist xfs_attr_node_addname
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (4 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 05/27] xfsprogs: Add helper xfs_attr_node_addname_find_attr Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 07/27] xfsprogs: Hoist xfs_attr_leaf_addname Allison Henderson
` (20 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 8e7db2a154cf8889319a4d6e5f987998ed21fa1b
This patch hoists the later half of xfs_attr_node_addname into
the calling function. We do this because it is this area that
will need the most state management, and we want to keep such
code in the same scope as much as possible
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
---
libxfs/xfs_attr.c | 159 ++++++++++++++++++++++++++----------------------------
1 file changed, 75 insertions(+), 84 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 32a51d5..16e919d 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -52,6 +52,7 @@ STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp);
* Internal routines when attribute list is more than one block.
*/
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_addname(struct xfs_da_args *args,
struct xfs_da_state *state);
STATIC int xfs_attr_node_addname_find_attr(struct xfs_da_args *args,
@@ -290,8 +291,8 @@ xfs_attr_set_args(
struct xfs_da_args *args)
{
struct xfs_inode *dp = args->dp;
- struct xfs_da_state *state;
- int error;
+ struct xfs_da_state *state = NULL;
+ int error = 0;
/*
* If the attribute list is already in leaf format, jump straight to
@@ -342,7 +343,75 @@ xfs_attr_set_args(
return error;
error = xfs_attr_node_addname(args, state);
} while (error == -EAGAIN);
+ if (error)
+ return error;
+ /*
+ * Commit the leaf addition or btree split and start the next
+ * trans in the chain.
+ */
+ error = xfs_trans_roll_inode(&args->trans, dp);
+ if (error)
+ goto out;
+
+ /*
+ * If there was an out-of-line value, allocate the blocks we
+ * identified for its storage and copy the value. This is done
+ * after we create the attribute so that we don't overflow the
+ * maximum size of a transaction and/or hit a deadlock.
+ */
+ if (args->rmtblkno > 0) {
+ error = xfs_attr_rmtval_set(args);
+ if (error)
+ return error;
+ }
+
+ if (!(args->op_flags & XFS_DA_OP_RENAME)) {
+ /*
+ * Added a "remote" value, just clear the incomplete flag.
+ */
+ if (args->rmtblkno > 0)
+ error = xfs_attr3_leaf_clearflag(args);
+ goto out;
+ }
+
+ /*
+ * 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.
+ */
+ error = xfs_attr3_leaf_flipflags(args);
+ if (error)
+ goto out;
+ /*
+ * Commit the flag value change and start the next trans in series
+ */
+ error = xfs_trans_roll_inode(&args->trans, args->dp);
+ if (error)
+ goto out;
+
+ /*
+ * Dismantle the "old" attribute/value pair by removing a "remote" value
+ * (if it exists).
+ */
+ xfs_attr_restore_rmt_blk(args);
+
+ if (args->rmtblkno) {
+ error = xfs_attr_rmtval_invalidate(args);
+ if (error)
+ return error;
+
+ error = xfs_attr_rmtval_remove(args);
+ if (error)
+ return error;
+ }
+
+ error = xfs_attr_node_addname_clear_incomplete(args);
+out:
return error;
}
@@ -968,7 +1037,7 @@ xfs_attr_node_addname(
{
struct xfs_da_state_blk *blk;
struct xfs_inode *dp;
- int retval, error;
+ int error;
trace_xfs_attr_node_addname(args);
@@ -976,8 +1045,8 @@ xfs_attr_node_addname(
blk = &state->path.blk[state->path.active-1];
ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
- retval = xfs_attr3_leaf_add(blk->bp, state->args);
- if (retval == -ENOSPC) {
+ error = xfs_attr3_leaf_add(blk->bp, state->args);
+ if (error == -ENOSPC) {
if (state->path.active == 1) {
/*
* Its really a single leaf node, but it had
@@ -1023,88 +1092,10 @@ xfs_attr_node_addname(
xfs_da3_fixhashpath(state, &state->path);
}
- /*
- * Kill the state structure, we're done with it and need to
- * allow the buffers to come back later.
- */
- xfs_da_state_free(state);
- state = NULL;
-
- /*
- * Commit the leaf addition or btree split and start the next
- * trans in the chain.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- goto out;
-
- /*
- * If there was an out-of-line value, allocate the blocks we
- * identified for its storage and copy the value. This is done
- * after we create the attribute so that we don't overflow the
- * maximum size of a transaction and/or hit a deadlock.
- */
- if (args->rmtblkno > 0) {
- error = xfs_attr_rmtval_set(args);
- if (error)
- return error;
- }
-
- if (!(args->op_flags & XFS_DA_OP_RENAME)) {
- /*
- * Added a "remote" value, just clear the incomplete flag.
- */
- if (args->rmtblkno > 0)
- error = xfs_attr3_leaf_clearflag(args);
- retval = error;
- goto out;
- }
-
- /*
- * 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.
- */
- error = xfs_attr3_leaf_flipflags(args);
- if (error)
- goto out;
- /*
- * Commit the flag value change and start the next trans in series
- */
- error = xfs_trans_roll_inode(&args->trans, args->dp);
- if (error)
- goto out;
-
- /*
- * Dismantle the "old" attribute/value pair by removing a "remote" value
- * (if it exists).
- */
- xfs_attr_restore_rmt_blk(args);
-
- if (args->rmtblkno) {
- error = xfs_attr_rmtval_invalidate(args);
- if (error)
- return error;
-
- error = xfs_attr_rmtval_remove(args);
- if (error)
- return error;
- }
-
- error = xfs_attr_node_addname_clear_incomplete(args);
- if (error)
- goto out;
- retval = 0;
out:
if (state)
xfs_da_state_free(state);
- if (error)
- return error;
- return retval;
+ return error;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 07/27] xfsprogs: Hoist xfs_attr_leaf_addname
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (5 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 06/27] xfsprogs: Hoist xfs_attr_node_addname Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 08/27] xfsprogs: Hoist node transaction handling Allison Henderson
` (19 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 45adc55591f5d91b9b6c7752fa4253bf3de33886
This patch hoists xfs_attr_leaf_addname into the calling function. The
goal being to get all the code that will require state management into
the same scope. This isn't particularly aesthetic right away, but it is a
preliminary step to merging in the state machine code.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
include/xfs_trace.h | 1 -
libxfs/xfs_attr.c | 209 ++++++++++++++++++++++++----------------------------
2 files changed, 96 insertions(+), 114 deletions(-)
diff --git a/include/xfs_trace.h b/include/xfs_trace.h
index a100263..fa4d38d 100644
--- a/include/xfs_trace.h
+++ b/include/xfs_trace.h
@@ -148,7 +148,6 @@
#define trace_xfs_attr_leaf_flipflags(a) ((void) 0)
#define trace_xfs_attr_sf_addname(a) ((void) 0)
-#define trace_xfs_attr_leaf_addname(a) ((void) 0)
#define trace_xfs_attr_leaf_replace(a) ((void) 0)
#define trace_xfs_attr_leaf_removename(a) ((void) 0)
#define trace_xfs_attr_leaf_get(a) ((void) 0)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 16e919d..9dc518a 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -44,9 +44,9 @@ STATIC int xfs_attr_shortform_addname(xfs_da_args_t *args);
* Internal routines when attribute list is one block.
*/
STATIC int xfs_attr_leaf_get(xfs_da_args_t *args);
-STATIC int xfs_attr_leaf_addname(xfs_da_args_t *args);
STATIC int xfs_attr_leaf_removename(xfs_da_args_t *args);
STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp);
+STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args, struct xfs_buf *bp);
/*
* Internal routines when attribute list is more than one block.
@@ -291,8 +291,9 @@ xfs_attr_set_args(
struct xfs_da_args *args)
{
struct xfs_inode *dp = args->dp;
+ struct xfs_buf *bp = NULL;
struct xfs_da_state *state = NULL;
- int error = 0;
+ int forkoff, error = 0;
/*
* If the attribute list is already in leaf format, jump straight to
@@ -307,10 +308,101 @@ xfs_attr_set_args(
}
if (xfs_attr_is_leaf(dp)) {
- error = xfs_attr_leaf_addname(args);
- if (error != -ENOSPC)
+ error = xfs_attr_leaf_try_add(args, bp);
+ if (error == -ENOSPC)
+ goto node;
+ else if (error)
+ return error;
+
+ /*
+ * Commit the transaction that added the attr name so that
+ * later routines can manage their own transactions.
+ */
+ error = xfs_trans_roll_inode(&args->trans, dp);
+ if (error)
+ return error;
+
+ /*
+ * If there was an out-of-line value, allocate the blocks we
+ * identified for its storage and copy the value. This is done
+ * after we create the attribute so that we don't overflow the
+ * maximum size of a transaction and/or hit a deadlock.
+ */
+ if (args->rmtblkno > 0) {
+ error = xfs_attr_rmtval_set(args);
+ if (error)
+ return error;
+ }
+
+ if (!(args->op_flags & XFS_DA_OP_RENAME)) {
+ /*
+ * Added a "remote" value, just clear the incomplete
+ *flag.
+ */
+ if (args->rmtblkno > 0)
+ error = xfs_attr3_leaf_clearflag(args);
+
+ return error;
+ }
+
+ /*
+ * 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.
+ */
+
+ error = xfs_attr3_leaf_flipflags(args);
+ if (error)
+ return error;
+ /*
+ * Commit the flag value change and start the next trans in
+ * series.
+ */
+ error = xfs_trans_roll_inode(&args->trans, args->dp);
+ if (error)
+ return error;
+
+ /*
+ * Dismantle the "old" attribute/value pair by removing a
+ * "remote" value (if it exists).
+ */
+ xfs_attr_restore_rmt_blk(args);
+
+ if (args->rmtblkno) {
+ error = xfs_attr_rmtval_invalidate(args);
+ if (error)
+ return error;
+
+ error = xfs_attr_rmtval_remove(args);
+ if (error)
+ return error;
+ }
+
+ /*
+ * Read in the block containing the "old" attr, then remove the
+ * "old" attr from that block (neat, huh!)
+ */
+ error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno,
+ &bp);
+ if (error)
return error;
+ xfs_attr3_leaf_remove(bp, args);
+
+ /*
+ * If the result is small enough, shrink it all into the inode.
+ */
+ 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;
+node:
/*
* Promote the attribute list to the Btree format.
*/
@@ -737,115 +829,6 @@ out_brelse:
return retval;
}
-
-/*
- * Add a name to the leaf attribute list structure
- *
- * This leaf block cannot have a "remote" value, we only call this routine
- * if bmap_one_block() says there is only one block (ie: no remote blks).
- */
-STATIC int
-xfs_attr_leaf_addname(
- struct xfs_da_args *args)
-{
- int error, forkoff;
- struct xfs_buf *bp = NULL;
- struct xfs_inode *dp = args->dp;
-
- trace_xfs_attr_leaf_addname(args);
-
- error = xfs_attr_leaf_try_add(args, bp);
- if (error)
- return error;
-
- /*
- * Commit the transaction that added the attr name so that
- * later routines can manage their own transactions.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- return error;
-
- /*
- * If there was an out-of-line value, allocate the blocks we
- * identified for its storage and copy the value. This is done
- * after we create the attribute so that we don't overflow the
- * maximum size of a transaction and/or hit a deadlock.
- */
- if (args->rmtblkno > 0) {
- error = xfs_attr_rmtval_set(args);
- if (error)
- return error;
- }
-
- if (!(args->op_flags & XFS_DA_OP_RENAME)) {
- /*
- * Added a "remote" value, just clear the incomplete flag.
- */
- if (args->rmtblkno > 0)
- error = xfs_attr3_leaf_clearflag(args);
-
- return error;
- }
-
- /*
- * 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.
- */
-
- error = xfs_attr3_leaf_flipflags(args);
- if (error)
- return error;
- /*
- * Commit the flag value change and start the next trans in series.
- */
- error = xfs_trans_roll_inode(&args->trans, args->dp);
- if (error)
- return error;
-
- /*
- * Dismantle the "old" attribute/value pair by removing a "remote" value
- * (if it exists).
- */
- xfs_attr_restore_rmt_blk(args);
-
- if (args->rmtblkno) {
- error = xfs_attr_rmtval_invalidate(args);
- if (error)
- return error;
-
- error = xfs_attr_rmtval_remove(args);
- if (error)
- return error;
- }
-
- /*
- * Read in the block containing the "old" attr, then remove the "old"
- * attr from that block (neat, huh!)
- */
- error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno,
- &bp);
- if (error)
- return error;
-
- xfs_attr3_leaf_remove(bp, args);
-
- /*
- * If the result is small enough, shrink it all into the inode.
- */
- 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;
-}
-
/*
* Return EEXIST if attr is found, or ENOATTR if not
*/
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 08/27] xfsprogs: Hoist node transaction handling
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (6 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 07/27] xfsprogs: Hoist xfs_attr_leaf_addname Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 09/27] xfsprogs: Add delay ready attr remove routines Allison Henderson
` (18 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: cb9e8e1fb89d84c77c8035b2391359f9cbf209e6
This patch basically hoists the node transaction handling around the
leaf code we just hoisted. This will helps setup this area for the
state machine since the goto is easily replaced with a state since it
ends with a transaction roll.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
---
libxfs/xfs_attr.c | 55 +++++++++++++++++++++++++++++--------------------------
1 file changed, 29 insertions(+), 26 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 9dc518a..118ec0b4 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -309,10 +309,36 @@ xfs_attr_set_args(
if (xfs_attr_is_leaf(dp)) {
error = xfs_attr_leaf_try_add(args, bp);
- if (error == -ENOSPC)
+ if (error == -ENOSPC) {
+ /*
+ * Promote the attribute list to the Btree format.
+ */
+ error = xfs_attr3_leaf_to_node(args);
+ if (error)
+ return error;
+
+ /*
+ * Finish any deferred work items and roll the transaction once
+ * more. The goal here is to call node_addname with the inode
+ * and transaction in the same state (inode locked and joined,
+ * transaction clean) no matter how we got to this step.
+ */
+ error = xfs_defer_finish(&args->trans);
+ if (error)
+ return error;
+
+ /*
+ * Commit the current trans (including the inode) and
+ * start a new one.
+ */
+ error = xfs_trans_roll_inode(&args->trans, dp);
+ if (error)
+ return error;
+
goto node;
- else if (error)
+ } else if (error) {
return error;
+ }
/*
* Commit the transaction that added the attr name so that
@@ -402,32 +428,9 @@ xfs_attr_set_args(
/* bp is gone due to xfs_da_shrink_inode */
return error;
+ }
node:
- /*
- * Promote the attribute list to the Btree format.
- */
- error = xfs_attr3_leaf_to_node(args);
- if (error)
- return error;
-
- /*
- * Finish any deferred work items and roll the transaction once
- * more. The goal here is to call node_addname with the inode
- * and transaction in the same state (inode locked and joined,
- * transaction clean) no matter how we got to this step.
- */
- error = xfs_defer_finish(&args->trans);
- if (error)
- return error;
- /*
- * Commit the current trans (including the inode) and
- * start a new one.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- return error;
- }
do {
error = xfs_attr_node_addname_find_attr(args, &state);
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 09/27] xfsprogs: Add delay ready attr remove routines
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (7 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 08/27] xfsprogs: Hoist node transaction handling Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 10/27] xfsprogs: Add delay ready attr set routines Allison Henderson
` (17 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 19526 bytes --]
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 is merged with
xfs_attr_node_removename become a new function, xfs_attr_remove_iter.
This new version uses a sort of state machine like switch to keep track
of where it was when EAGAIN was returned. A new version of
xfs_attr_remove_args consists of a simple loop to refresh the
transaction until the operation is completed. A new XFS_DAC_DEFER_FINISH
flag is used to finish the transaction where ever the existing code used
to.
Calls to xfs_attr_rmtval_remove are replaced with the delay ready
version __xfs_attr_rmtval_remove. We will rename
__xfs_attr_rmtval_remove back to xfs_attr_rmtval_remove when we are
done.
xfs_attr_rmtval_remove itself is still in use by the set routines (used
during a rename). For reasons of preserving existing function, we
modify xfs_attr_rmtval_remove to call xfs_defer_finish when the flag is
set. Similar to how xfs_attr_remove_args does here. Once we transition
the set routines to be delay ready, xfs_attr_rmtval_remove is no longer
used and will be removed.
This patch also adds a new struct xfs_delattr_context, which we will use
to keep track of the current state of an attribute operation. The new
xfs_delattr_state enum is used to track various operations that are in
progress so that we know not to repeat them, and resume where we left
off before EAGAIN was returned to cycle out the transaction. Other
members take the place of local variables that need to retain their
values across multiple function calls. See xfs_attr.h for a more
detailed diagram of the states.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
include/libxfs.h | 1 +
libxfs/xfs_attr.c | 223 ++++++++++++++++++++++++++++++++++-------------
libxfs/xfs_attr.h | 131 ++++++++++++++++++++++++++++
libxfs/xfs_attr_leaf.c | 2 +-
libxfs/xfs_attr_remote.c | 53 ++++++-----
libxfs/xfs_attr_remote.h | 2 +-
6 files changed, 327 insertions(+), 85 deletions(-)
diff --git a/include/libxfs.h b/include/libxfs.h
index bc07655..02d97c1 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -168,6 +168,7 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
#include "xfs_ialloc.h"
#include "xfs_attr_leaf.h"
+#include "xfs_attr.h"
#include "xfs_attr_remote.h"
#include "xfs_trans_space.h"
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 118ec0b4..94da860 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -57,7 +57,6 @@ STATIC int xfs_attr_node_addname(struct xfs_da_args *args,
struct xfs_da_state *state);
STATIC int xfs_attr_node_addname_find_attr(struct xfs_da_args *args,
struct xfs_da_state **state);
-STATIC int xfs_attr_node_removename(xfs_da_args_t *args);
STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_da_args *args);
STATIC int xfs_attr_node_hasname(xfs_da_args_t *args,
struct xfs_da_state **state);
@@ -241,6 +240,31 @@ xfs_attr_is_shortform(
ip->i_afp->if_nextents == 0);
}
+/*
+ * Checks to see if a delayed attribute transaction should be rolled. If so,
+ * transaction is finished or rolled as needed.
+ */
+int
+xfs_attr_trans_roll(
+ struct xfs_delattr_context *dac)
+{
+ struct xfs_da_args *args = dac->da_args;
+ int error;
+
+ if (dac->flags & XFS_DAC_DEFER_FINISH) {
+ /*
+ * The caller wants us to finish all the deferred ops so that we
+ * avoid pinning the log tail with a large number of deferred
+ * ops.
+ */
+ dac->flags &= ~XFS_DAC_DEFER_FINISH;
+ error = xfs_defer_finish(&args->trans);
+ } else
+ error = xfs_trans_roll_inode(&args->trans, args->dp);
+
+ return error;
+}
+
STATIC int
xfs_attr_set_fmt(
struct xfs_da_args *args)
@@ -544,16 +568,25 @@ xfs_has_attr(
*/
int
xfs_attr_remove_args(
- struct xfs_da_args *args)
+ struct xfs_da_args *args)
{
- if (!xfs_inode_hasattr(args->dp))
- return -ENOATTR;
+ int error;
+ struct xfs_delattr_context dac = {
+ .da_args = args,
+ };
- if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
- return xfs_attr_shortform_remove(args);
- if (xfs_attr_is_leaf(args->dp))
- return xfs_attr_leaf_removename(args);
- return xfs_attr_node_removename(args);
+ do {
+ error = xfs_attr_remove_iter(&dac);
+ if (error != -EAGAIN)
+ break;
+
+ error = xfs_attr_trans_roll(&dac);
+ if (error)
+ return error;
+
+ } while (true);
+
+ return error;
}
/*
@@ -1197,14 +1230,16 @@ xfs_attr_leaf_mark_incomplete(
*/
STATIC
int xfs_attr_node_removename_setup(
- struct xfs_da_args *args,
- struct xfs_da_state **state)
+ struct xfs_delattr_context *dac)
{
- int error;
+ struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_state **state = &dac->da_state;
+ int error;
error = xfs_attr_node_hasname(args, state);
if (error != -EEXIST)
return error;
+ error = 0;
ASSERT((*state)->path.blk[(*state)->path.active - 1].bp != NULL);
ASSERT((*state)->path.blk[(*state)->path.active - 1].magic ==
@@ -1213,12 +1248,15 @@ int xfs_attr_node_removename_setup(
if (args->rmtblkno > 0) {
error = xfs_attr_leaf_mark_incomplete(args, *state);
if (error)
- return error;
+ goto out;
- return xfs_attr_rmtval_invalidate(args);
+ error = xfs_attr_rmtval_invalidate(args);
}
+out:
+ if (error)
+ xfs_da_state_free(*state);
- return 0;
+ return error;
}
STATIC int
@@ -1241,70 +1279,133 @@ xfs_attr_node_remove_name(
}
/*
- * Remove a name from a B-tree attribute list.
+ * 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.
*/
-STATIC int
-xfs_attr_node_removename(
- struct xfs_da_args *args)
+int
+xfs_attr_remove_iter(
+ struct xfs_delattr_context *dac)
{
- struct xfs_da_state *state;
- int retval, error;
- struct xfs_inode *dp = args->dp;
+ struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_state *state = dac->da_state;
+ int retval, error;
+ struct xfs_inode *dp = args->dp;
trace_xfs_attr_node_removename(args);
- error = xfs_attr_node_removename_setup(args, &state);
- if (error)
- goto out;
+ switch (dac->dela_state) {
+ case XFS_DAS_UNINIT:
+ if (!xfs_inode_hasattr(dp))
+ return -ENOATTR;
- /*
- * 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) {
- error = xfs_attr_rmtval_remove(args);
- if (error)
- goto out;
+ /*
+ * 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_shortform_remove(args);
+
+ if (xfs_attr_is_leaf(dp))
+ return xfs_attr_leaf_removename(args);
/*
- * Refill the state structure with buffers, the prior calls
- * released our buffers.
+ * Node format may require transaction rolls. Set up the
+ * state context and fall into the state machine.
*/
- error = xfs_attr_refillstate(state);
- if (error)
- goto out;
- }
- retval = xfs_attr_node_remove_name(args, state);
+ if (!dac->da_state) {
+ error = xfs_attr_node_removename_setup(dac);
+ if (error)
+ return error;
+ state = dac->da_state;
+ }
+
+ /* fallthrough */
+ case XFS_DAS_RMTBLK:
+ dac->dela_state = XFS_DAS_RMTBLK;
- /*
- * Check to see if the tree needs to be collapsed.
- */
- if (retval && (state->path.active > 1)) {
- 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.
+ * 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.
*/
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- goto out;
- }
+ if (args->rmtblkno > 0) {
+ /*
+ * May return -EAGAIN. Roll and repeat until all remote
+ * blocks are removed.
+ */
+ error = __xfs_attr_rmtval_remove(dac);
+ if (error == -EAGAIN)
+ return error;
+ else if (error)
+ goto out;
- /*
- * If the result is small enough, push it all into the inode.
- */
- if (xfs_attr_is_leaf(dp))
- error = xfs_attr_node_shrink(args, state);
+ /*
+ * 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;
+ dac->dela_state = XFS_DAS_RM_NAME;
+ dac->flags |= XFS_DAC_DEFER_FINISH;
+ 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 (dac->dela_state == XFS_DAS_RM_NAME) {
+ error = xfs_attr_refillstate(state);
+ if (error)
+ goto out;
+ }
+ retval = xfs_attr_node_remove_name(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;
+
+ dac->flags |= XFS_DAC_DEFER_FINISH;
+ dac->dela_state = XFS_DAS_RM_SHRINK;
+ 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_node_shrink(args, state);
+ ASSERT(error != -EAGAIN);
+ break;
+ default:
+ ASSERT(0);
+ error = -EINVAL;
+ goto out;
+ }
out:
if (state)
xfs_da_state_free(state);
diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h
index 2b1f619..1267ea8 100644
--- a/libxfs/xfs_attr.h
+++ b/libxfs/xfs_attr.h
@@ -74,6 +74,133 @@ struct xfs_attr_list_context {
};
+/*
+ * ========================================================================
+ * Structure used to pass context around among the delayed routines.
+ * ========================================================================
+ */
+
+/*
+ * Below is a state machine diagram for attr remove operations. The XFS_DAS_*
+ * states indicate places where the function would return -EAGAIN, and then
+ * immediately resume from after being called by the calling function. States
+ * marked as a "subroutine state" indicate that they belong to a subroutine, and
+ * so the calling function needs to pass them back to that subroutine to allow
+ * it to finish where it left off. But they otherwise do not have a role in the
+ * calling function other than just passing through.
+ *
+ * xfs_attr_remove_iter()
+ * │
+ * v
+ * have attr to remove? ──n──> done
+ * │
+ * y
+ * │
+ * v
+ * are we short form? ──y──> xfs_attr_shortform_remove ──> done
+ * │
+ * n
+ * │
+ * V
+ * are we leaf form? ──y──> xfs_attr_leaf_removename ──> done
+ * │
+ * n
+ * │
+ * V
+ * ┌── need to setup state?
+ * │ │
+ * n y
+ * │ │
+ * │ v
+ * │ find attr and get state
+ * │ attr has remote blks? ──n─┐
+ * │ │ v
+ * │ │ find and invalidate
+ * │ y the remote blocks.
+ * │ │ mark attr incomplete
+ * │ ├────────────────┘
+ * └──────────┤
+ * │
+ * v
+ * Have remote blks to remove? ───y─────┐
+ * │ ^ remove the blks
+ * │ │ │
+ * │ │ v
+ * │ XFS_DAS_RMTBLK <─n── done?
+ * │ re-enter with │
+ * │ one less blk to y
+ * │ remove │
+ * │ V
+ * │ refill the state
+ * n │
+ * │ v
+ * │ XFS_DAS_RM_NAME
+ * │ │
+ * ├─────────────────────────┘
+ * │
+ * v
+ * remove leaf and
+ * update hash with
+ * xfs_attr_node_remove_cleanup
+ * │
+ * v
+ * need to
+ * shrink tree? ─n─┐
+ * │ │
+ * y │
+ * │ │
+ * v │
+ * join leaf │
+ * │ │
+ * v │
+ * XFS_DAS_RM_SHRINK │
+ * │ │
+ * v │
+ * do the shrink │
+ * │ │
+ * v │
+ * free state <──┘
+ * │
+ * v
+ * done
+ *
+ */
+
+/*
+ * Enum values for xfs_delattr_context.da_state
+ *
+ * These values are used by delayed attribute operations to keep track of where
+ * they were before they returned -EAGAIN. A return code of -EAGAIN signals the
+ * calling function to roll the transaction, and then call the subroutine to
+ * finish the operation. The enum is then used by the subroutine to jump back
+ * to where it was and resume executing where it left off.
+ */
+enum xfs_delattr_state {
+ XFS_DAS_UNINIT = 0, /* No state has been set yet */
+ XFS_DAS_RMTBLK, /* Removing remote blks */
+ XFS_DAS_RM_NAME, /* Remove attr name */
+ XFS_DAS_RM_SHRINK, /* We are shrinking the tree */
+};
+
+/*
+ * Defines for xfs_delattr_context.flags
+ */
+#define XFS_DAC_DEFER_FINISH 0x01 /* finish the transaction */
+
+/*
+ * Context used for keeping track of delayed attribute operations
+ */
+struct xfs_delattr_context {
+ struct xfs_da_args *da_args;
+
+ /* Used in xfs_attr_node_removename to roll through removing blocks */
+ struct xfs_da_state *da_state;
+
+ /* Used to keep track of current state of delayed operation */
+ unsigned int flags;
+ enum xfs_delattr_state dela_state;
+};
+
/*========================================================================
* Function prototypes for the kernel.
*========================================================================*/
@@ -92,6 +219,10 @@ int xfs_attr_set(struct xfs_da_args *args);
int xfs_attr_set_args(struct xfs_da_args *args);
int xfs_has_attr(struct xfs_da_args *args);
int xfs_attr_remove_args(struct xfs_da_args *args);
+int xfs_attr_remove_iter(struct xfs_delattr_context *dac);
+int xfs_attr_trans_roll(struct xfs_delattr_context *dac);
bool xfs_attr_namecheck(const void *name, size_t length);
+void xfs_delattr_context_init(struct xfs_delattr_context *dac,
+ struct xfs_da_args *args);
#endif /* __XFS_ATTR_H__ */
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index e13e83e..08600ea 100644
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -19,8 +19,8 @@
#include "xfs_bmap_btree.h"
#include "xfs_bmap.h"
#include "xfs_attr_sf.h"
-#include "xfs_attr_remote.h"
#include "xfs_attr.h"
+#include "xfs_attr_remote.h"
#include "xfs_attr_leaf.h"
#include "xfs_trace.h"
#include "xfs_dir2.h"
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index 3807cd3..e41bbb2 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -673,10 +673,12 @@ xfs_attr_rmtval_invalidate(
*/
int
xfs_attr_rmtval_remove(
- struct xfs_da_args *args)
+ struct xfs_da_args *args)
{
- int error;
- int retval;
+ int error;
+ struct xfs_delattr_context dac = {
+ .da_args = args,
+ };
trace_xfs_attr_rmtval_remove(args);
@@ -684,31 +686,30 @@ xfs_attr_rmtval_remove(
* Keep de-allocating extents until the remote-value region is gone.
*/
do {
- retval = __xfs_attr_rmtval_remove(args);
- if (retval && retval != -EAGAIN)
- return retval;
+ error = __xfs_attr_rmtval_remove(&dac);
+ if (error && error != -EAGAIN)
+ break;
- /*
- * Close out trans and start the next one in the chain.
- */
- error = xfs_trans_roll_inode(&args->trans, args->dp);
+ error = xfs_attr_trans_roll(&dac);
if (error)
return error;
- } while (retval == -EAGAIN);
+ } while (true);
- return 0;
+ return error;
}
/*
* Remove the value associated with an attribute by deleting the out-of-line
- * buffer that it is stored on. Returns EAGAIN for the caller to refresh the
- * transaction and re-call the function
+ * buffer that it is stored on. Returns -EAGAIN for the caller to refresh the
+ * transaction and re-call the function. Callers should keep calling this
+ * routine until it returns something other than -EAGAIN.
*/
int
__xfs_attr_rmtval_remove(
- struct xfs_da_args *args)
+ struct xfs_delattr_context *dac)
{
- int error, done;
+ struct xfs_da_args *args = dac->da_args;
+ int error, done;
/*
* Unmap value blocks for this attr.
@@ -718,12 +719,20 @@ __xfs_attr_rmtval_remove(
if (error)
return error;
- error = xfs_defer_finish(&args->trans);
- if (error)
- return error;
-
- if (!done)
+ /*
+ * We don't need an explicit state here to pick up where we left off. We
+ * can figure it out using the !done return code. The actual value of
+ * attr->xattri_dela_state may be some value reminiscent of the calling
+ * function, but it's value is irrelevant with in the context of this
+ * function. Once we are done here, the next state is set as needed by
+ * the parent
+ */
+ if (!done) {
+ dac->flags |= XFS_DAC_DEFER_FINISH;
return -EAGAIN;
+ }
- return error;
+ args->rmtblkno = 0;
+ args->rmtblkcnt = 0;
+ return 0;
}
diff --git a/libxfs/xfs_attr_remote.h b/libxfs/xfs_attr_remote.h
index 9eee615..002fd30 100644
--- a/libxfs/xfs_attr_remote.h
+++ b/libxfs/xfs_attr_remote.h
@@ -14,5 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args);
int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map,
xfs_buf_flags_t incore_flags);
int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
-int __xfs_attr_rmtval_remove(struct xfs_da_args *args);
+int __xfs_attr_rmtval_remove(struct xfs_delattr_context *dac);
#endif /* __XFS_ATTR_REMOTE_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 10/27] xfsprogs: Add delay ready attr set routines
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (8 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 09/27] xfsprogs: Add delay ready attr remove routines Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 11/27] xfsprogs: Remove xfs_attr_rmtval_set Allison Henderson
` (16 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 37110 bytes --]
Source kernel commit: 45e46c998cd75749f6975ffdeeb9a59768e4fcd3
This patch modifies the attr set 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_set_args has become xfs_attr_set_iter, which uses a
state machine like switch to keep track of where it was when EAGAIN was
returned. See xfs_attr.h for a more detailed diagram of the states.
Two new helper functions have been added: xfs_attr_rmtval_find_space and
xfs_attr_rmtval_set_blk. They provide a subset of logic similar to
xfs_attr_rmtval_set, but they store the current block in the delay attr
context to allow the caller to roll the transaction between allocations.
This helps to simplify and consolidate code used by
xfs_attr_leaf_addname and xfs_attr_node_addname. xfs_attr_set_args has
now become a simple loop to refresh the transaction until the operation
is completed. Lastly, xfs_attr_rmtval_remove is no longer used, and is
removed.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
include/xfs_trace.h | 1 -
libxfs/xfs_attr.c | 450 ++++++++++++++++++++++++++++-------------------
libxfs/xfs_attr.h | 274 ++++++++++++++++++++++++++++-
libxfs/xfs_attr_remote.c | 100 +++++++----
libxfs/xfs_attr_remote.h | 5 +-
5 files changed, 610 insertions(+), 220 deletions(-)
diff --git a/include/xfs_trace.h b/include/xfs_trace.h
index fa4d38d..a847b50 100644
--- a/include/xfs_trace.h
+++ b/include/xfs_trace.h
@@ -159,7 +159,6 @@
#define trace_xfs_attr_node_get(a) ((void) 0)
#define trace_xfs_attr_rmtval_get(a) ((void) 0)
#define trace_xfs_attr_rmtval_set(a) ((void) 0)
-#define trace_xfs_attr_rmtval_remove(a) ((void) 0)
#define trace_xfs_bmap_pre_update(a,b,c,d) ((void) 0)
#define trace_xfs_bmap_post_update(a,b,c,d) ((void) 0)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 94da860..347f854 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -53,15 +53,16 @@ STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args, struct xfs_buf *bp);
*/
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_addname(struct xfs_da_args *args,
- struct xfs_da_state *state);
-STATIC int xfs_attr_node_addname_find_attr(struct xfs_da_args *args,
- struct xfs_da_state **state);
-STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_da_args *args);
+STATIC int xfs_attr_node_addname(struct xfs_delattr_context *dac);
+STATIC int xfs_attr_node_addname_find_attr(struct xfs_delattr_context *dac);
+STATIC int xfs_attr_node_addname_clear_incomplete(
+ struct xfs_delattr_context *dac);
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_set_iter(struct xfs_delattr_context *dac,
+ struct xfs_buf **leaf_bp);
int
xfs_inode_hasattr(
@@ -244,7 +245,7 @@ xfs_attr_is_shortform(
* Checks to see if a delayed attribute transaction should be rolled. If so,
* transaction is finished or rolled as needed.
*/
-int
+STATIC int
xfs_attr_trans_roll(
struct xfs_delattr_context *dac)
{
@@ -265,29 +266,58 @@ xfs_attr_trans_roll(
return error;
}
+/*
+ * Set the attribute specified in @args.
+ */
+int
+xfs_attr_set_args(
+ struct xfs_da_args *args)
+{
+ struct xfs_buf *leaf_bp = NULL;
+ int error = 0;
+ struct xfs_delattr_context dac = {
+ .da_args = args,
+ };
+
+ do {
+ error = xfs_attr_set_iter(&dac, &leaf_bp);
+ if (error != -EAGAIN)
+ break;
+
+ error = xfs_attr_trans_roll(&dac);
+ if (error) {
+ if (leaf_bp)
+ xfs_trans_brelse(args->trans, leaf_bp);
+ return error;
+ }
+ } while (true);
+
+ return error;
+}
+
STATIC int
xfs_attr_set_fmt(
- struct xfs_da_args *args)
+ struct xfs_delattr_context *dac,
+ struct xfs_buf **leaf_bp)
{
- struct xfs_buf *leaf_bp = NULL;
- struct xfs_inode *dp = args->dp;
- int error, error2 = 0;
+ struct xfs_da_args *args = dac->da_args;
+ struct xfs_inode *dp = args->dp;
+ int error = 0;
/*
* Try to add the attr to the attribute list in the inode.
*/
error = xfs_attr_try_sf_addname(dp, args);
- if (error != -ENOSPC) {
- error2 = xfs_trans_commit(args->trans);
- args->trans = NULL;
- return error ? error : error2;
- }
+
+ /* Should only be 0, -EEXIST or -ENOSPC */
+ if (error != -ENOSPC)
+ return error;
/*
* It won't fit in the shortform, transform to a leaf block. GROT:
* another possible req'mt for a double-split btree op.
*/
- error = xfs_attr_shortform_to_leaf(args, &leaf_bp);
+ error = xfs_attr_shortform_to_leaf(args, leaf_bp);
if (error)
return error;
@@ -296,102 +326,130 @@ xfs_attr_set_fmt(
* push cannot grab the half-baked leaf buffer and run into problems
* with the write verifier.
*/
- xfs_trans_bhold(args->trans, leaf_bp);
- error = xfs_defer_finish(&args->trans);
- xfs_trans_bhold_release(args->trans, leaf_bp);
- if (error) {
- xfs_trans_brelse(args->trans, leaf_bp);
- return error;
- }
+ xfs_trans_bhold(args->trans, *leaf_bp);
+ /*
+ * We're still in XFS_DAS_UNINIT state here. We've converted
+ * the attr fork to leaf format and will restart with the leaf
+ * add.
+ */
+ dac->flags |= XFS_DAC_DEFER_FINISH;
return -EAGAIN;
}
/*
* Set the attribute specified in @args.
+ * 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.
*/
int
-xfs_attr_set_args(
- struct xfs_da_args *args)
+xfs_attr_set_iter(
+ struct xfs_delattr_context *dac,
+ struct xfs_buf **leaf_bp)
{
- struct xfs_inode *dp = args->dp;
- struct xfs_buf *bp = NULL;
- struct xfs_da_state *state = NULL;
- int forkoff, error = 0;
+ struct xfs_da_args *args = dac->da_args;
+ struct xfs_inode *dp = args->dp;
+ struct xfs_buf *bp = NULL;
+ int forkoff, error = 0;
- /*
- * If the attribute list is already in leaf format, jump straight to
- * leaf handling. Otherwise, try to add the attribute to the shortform
- * list; if there's no room then convert the list to leaf format and try
- * again.
- */
- if (xfs_attr_is_shortform(dp)) {
- error = xfs_attr_set_fmt(args);
- if (error != -EAGAIN)
- return error;
- }
+ /* State machine switch */
+ switch (dac->dela_state) {
+ case XFS_DAS_UNINIT:
+ /*
+ * If the fork is shortform, attempt to add the attr. If there
+ * is no space, this converts to leaf format and returns
+ * -EAGAIN with the leaf buffer held across the roll. The caller
+ * will deal with a transaction roll error, but otherwise
+ * release the hold once we return with a clean transaction.
+ */
+ if (xfs_attr_is_shortform(dp))
+ return xfs_attr_set_fmt(dac, leaf_bp);
+ if (*leaf_bp != NULL) {
+ xfs_trans_bhold_release(args->trans, *leaf_bp);
+ *leaf_bp = NULL;
+ }
- if (xfs_attr_is_leaf(dp)) {
- error = xfs_attr_leaf_try_add(args, bp);
- if (error == -ENOSPC) {
- /*
- * Promote the attribute list to the Btree format.
- */
- error = xfs_attr3_leaf_to_node(args);
- if (error)
+ if (xfs_attr_is_leaf(dp)) {
+ error = xfs_attr_leaf_try_add(args, *leaf_bp);
+ if (error == -ENOSPC) {
+ error = xfs_attr3_leaf_to_node(args);
+ if (error)
+ return error;
+
+ /*
+ * Finish any deferred work items and roll the
+ * transaction once more. The goal here is to
+ * call node_addname with the inode and
+ * transaction in the same state (inode locked
+ * and joined, transaction clean) no matter how
+ * we got to this step.
+ *
+ * At this point, we are still in
+ * XFS_DAS_UNINIT, but when we come back, we'll
+ * be a node, so we'll fall down into the node
+ * handling code below
+ */
+ dac->flags |= XFS_DAC_DEFER_FINISH;
+ return -EAGAIN;
+ } else if (error) {
return error;
+ }
- /*
- * Finish any deferred work items and roll the transaction once
- * more. The goal here is to call node_addname with the inode
- * and transaction in the same state (inode locked and joined,
- * transaction clean) no matter how we got to this step.
- */
- error = xfs_defer_finish(&args->trans);
+ dac->dela_state = XFS_DAS_FOUND_LBLK;
+ } else {
+ error = xfs_attr_node_addname_find_attr(dac);
if (error)
return error;
- /*
- * Commit the current trans (including the inode) and
- * start a new one.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
+ error = xfs_attr_node_addname(dac);
if (error)
return error;
- goto node;
- } else if (error) {
- return error;
+ dac->dela_state = XFS_DAS_FOUND_NBLK;
}
-
- /*
- * Commit the transaction that added the attr name so that
- * later routines can manage their own transactions.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- return error;
-
+ return -EAGAIN;
+ case XFS_DAS_FOUND_LBLK:
/*
* If there was an out-of-line value, allocate the blocks we
* identified for its storage and copy the value. This is done
* after we create the attribute so that we don't overflow the
* maximum size of a transaction and/or hit a deadlock.
*/
- if (args->rmtblkno > 0) {
- error = xfs_attr_rmtval_set(args);
+
+ /* Open coded xfs_attr_rmtval_set without trans handling */
+ if ((dac->flags & XFS_DAC_LEAF_ADDNAME_INIT) == 0) {
+ dac->flags |= XFS_DAC_LEAF_ADDNAME_INIT;
+ if (args->rmtblkno > 0) {
+ error = xfs_attr_rmtval_find_space(dac);
+ if (error)
+ return error;
+ }
+ }
+
+ /*
+ * Repeat allocating remote blocks for the attr value until
+ * blkcnt drops to zero.
+ */
+ if (dac->blkcnt > 0) {
+ error = xfs_attr_rmtval_set_blk(dac);
if (error)
return error;
+ return -EAGAIN;
}
+ error = xfs_attr_rmtval_set_value(args);
+ if (error)
+ return error;
+
+ /*
+ * If this is not a rename, clear the incomplete flag and we're
+ * done.
+ */
if (!(args->op_flags & XFS_DA_OP_RENAME)) {
- /*
- * Added a "remote" value, just clear the incomplete
- *flag.
- */
if (args->rmtblkno > 0)
error = xfs_attr3_leaf_clearflag(args);
-
return error;
}
@@ -404,7 +462,6 @@ xfs_attr_set_args(
* In a separate transaction, set the incomplete flag on the
* "old" attr and clear the incomplete flag on the "new" attr.
*/
-
error = xfs_attr3_leaf_flipflags(args);
if (error)
return error;
@@ -412,29 +469,37 @@ xfs_attr_set_args(
* Commit the flag value change and start the next trans in
* series.
*/
- error = xfs_trans_roll_inode(&args->trans, args->dp);
- if (error)
- return error;
-
+ dac->dela_state = XFS_DAS_FLIP_LFLAG;
+ return -EAGAIN;
+ case XFS_DAS_FLIP_LFLAG:
/*
* Dismantle the "old" attribute/value pair by removing a
* "remote" value (if it exists).
*/
xfs_attr_restore_rmt_blk(args);
+ error = xfs_attr_rmtval_invalidate(args);
+ if (error)
+ return error;
+ /* fallthrough */
+ case XFS_DAS_RM_LBLK:
+ /* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
+ dac->dela_state = XFS_DAS_RM_LBLK;
if (args->rmtblkno) {
- error = xfs_attr_rmtval_invalidate(args);
+ error = __xfs_attr_rmtval_remove(dac);
if (error)
return error;
- error = xfs_attr_rmtval_remove(args);
- if (error)
- return error;
+ dac->dela_state = XFS_DAS_RD_LEAF;
+ return -EAGAIN;
}
+ /* fallthrough */
+ case XFS_DAS_RD_LEAF:
/*
- * Read in the block containing the "old" attr, then remove the
- * "old" attr from that block (neat, huh!)
+ * 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);
@@ -443,97 +508,116 @@ xfs_attr_set_args(
xfs_attr3_leaf_remove(bp, args);
- /*
- * If the result is small enough, shrink it all into the inode.
- */
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;
- }
-node:
+ case XFS_DAS_FOUND_NBLK:
+ /*
+ * Find space for remote blocks and fall into the allocation
+ * state.
+ */
+ if (args->rmtblkno > 0) {
+ error = xfs_attr_rmtval_find_space(dac);
+ if (error)
+ return error;
+ }
- do {
- error = xfs_attr_node_addname_find_attr(args, &state);
- if (error)
- return error;
- error = xfs_attr_node_addname(args, state);
- } while (error == -EAGAIN);
- if (error)
- return error;
+ /* fallthrough */
+ case XFS_DAS_ALLOC_NODE:
+ /*
+ * If there was an out-of-line value, allocate the blocks we
+ * identified for its storage and copy the value. This is done
+ * after we create the attribute so that we don't overflow the
+ * maximum size of a transaction and/or hit a deadlock.
+ */
+ dac->dela_state = XFS_DAS_ALLOC_NODE;
+ if (args->rmtblkno > 0) {
+ if (dac->blkcnt > 0) {
+ error = xfs_attr_rmtval_set_blk(dac);
+ if (error)
+ return error;
+ return -EAGAIN;
+ }
+
+ error = xfs_attr_rmtval_set_value(args);
+ if (error)
+ return error;
+ }
- /*
- * Commit the leaf addition or btree split and start the next
- * trans in the chain.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- goto out;
+ /*
+ * If this was not a rename, clear the incomplete flag and we're
+ * done.
+ */
+ if (!(args->op_flags & XFS_DA_OP_RENAME)) {
+ if (args->rmtblkno > 0)
+ error = xfs_attr3_leaf_clearflag(args);
+ goto out;
+ }
- /*
- * If there was an out-of-line value, allocate the blocks we
- * identified for its storage and copy the value. This is done
- * after we create the attribute so that we don't overflow the
- * maximum size of a transaction and/or hit a deadlock.
- */
- if (args->rmtblkno > 0) {
- error = xfs_attr_rmtval_set(args);
+ /*
+ * 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.
+ */
+ error = xfs_attr3_leaf_flipflags(args);
if (error)
- return error;
- }
-
- if (!(args->op_flags & XFS_DA_OP_RENAME)) {
+ goto out;
/*
- * Added a "remote" value, just clear the incomplete flag.
+ * Commit the flag value change and start the next trans in
+ * series
*/
- if (args->rmtblkno > 0)
- error = xfs_attr3_leaf_clearflag(args);
- goto out;
- }
-
- /*
- * 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.
- */
- error = xfs_attr3_leaf_flipflags(args);
- if (error)
- goto out;
- /*
- * Commit the flag value change and start the next trans in series
- */
- error = xfs_trans_roll_inode(&args->trans, args->dp);
- if (error)
- goto out;
+ dac->dela_state = XFS_DAS_FLIP_NFLAG;
+ return -EAGAIN;
- /*
- * Dismantle the "old" attribute/value pair by removing a "remote" value
- * (if it exists).
- */
- xfs_attr_restore_rmt_blk(args);
+ case XFS_DAS_FLIP_NFLAG:
+ /*
+ * Dismantle the "old" attribute/value pair by removing a
+ * "remote" value (if it exists).
+ */
+ xfs_attr_restore_rmt_blk(args);
- if (args->rmtblkno) {
error = xfs_attr_rmtval_invalidate(args);
if (error)
return error;
- error = xfs_attr_rmtval_remove(args);
- if (error)
- return error;
- }
+ /* fallthrough */
+ case XFS_DAS_RM_NBLK:
+ /* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
+ dac->dela_state = XFS_DAS_RM_NBLK;
+ if (args->rmtblkno) {
+ error = __xfs_attr_rmtval_remove(dac);
+ if (error)
+ return error;
+
+ dac->dela_state = XFS_DAS_CLR_FLAG;
+ return -EAGAIN;
+ }
- error = xfs_attr_node_addname_clear_incomplete(args);
+ /* fallthrough */
+ 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(dac);
+ break;
+ default:
+ ASSERT(dac->dela_state != XFS_DAS_RM_SHRINK);
+ break;
+ }
out:
return error;
}
+
/*
* Return EEXIST if attr is found, or ENOATTR if not
*/
@@ -997,18 +1081,18 @@ xfs_attr_node_hasname(
STATIC int
xfs_attr_node_addname_find_attr(
- struct xfs_da_args *args,
- struct xfs_da_state **state)
+ struct xfs_delattr_context *dac)
{
- int retval;
+ struct xfs_da_args *args = dac->da_args;
+ int retval;
/*
* Search to see if name already exists, and get back a pointer
* to where it should go.
*/
- retval = xfs_attr_node_hasname(args, state);
+ retval = xfs_attr_node_hasname(args, &dac->da_state);
if (retval != -ENOATTR && retval != -EEXIST)
- goto error;
+ return retval;
if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
goto error;
@@ -1034,8 +1118,8 @@ xfs_attr_node_addname_find_attr(
return 0;
error:
- if (*state)
- xfs_da_state_free(*state);
+ if (dac->da_state)
+ xfs_da_state_free(dac->da_state);
return retval;
}
@@ -1048,19 +1132,23 @@ error:
*
* "Remote" attribute values confuse the issue and atomic rename operations
* add a whole extra layer of confusion on top of that.
+ *
+ * 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_addname(
- struct xfs_da_args *args,
- struct xfs_da_state *state)
+ struct xfs_delattr_context *dac)
{
- struct xfs_da_state_blk *blk;
- struct xfs_inode *dp;
- int error;
+ struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_state *state = dac->da_state;
+ struct xfs_da_state_blk *blk;
+ int error;
trace_xfs_attr_node_addname(args);
- dp = args->dp;
blk = &state->path.blk[state->path.active-1];
ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
@@ -1077,18 +1165,15 @@ xfs_attr_node_addname(
error = xfs_attr3_leaf_to_node(args);
if (error)
goto out;
- error = xfs_defer_finish(&args->trans);
- if (error)
- goto out;
/*
- * Commit the node conversion and start the next
- * trans in the chain.
+ * Now that we have converted the leaf to a node, we can
+ * roll the transaction, and try xfs_attr3_leaf_add
+ * again on re-entry. No need to set dela_state to do
+ * this. dela_state is still unset by this function at
+ * this point.
*/
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- goto out;
-
+ dac->flags |= XFS_DAC_DEFER_FINISH;
return -EAGAIN;
}
@@ -1101,9 +1186,7 @@ xfs_attr_node_addname(
error = xfs_da3_split(state);
if (error)
goto out;
- error = xfs_defer_finish(&args->trans);
- if (error)
- goto out;
+ dac->flags |= XFS_DAC_DEFER_FINISH;
} else {
/*
* Addition succeeded, update Btree hashvals.
@@ -1120,8 +1203,9 @@ out:
STATIC int
xfs_attr_node_addname_clear_incomplete(
- struct xfs_da_args *args)
+ struct xfs_delattr_context *dac)
{
+ struct xfs_da_args *args = dac->da_args;
struct xfs_da_state *state = NULL;
struct xfs_da_state_blk *blk;
int retval = 0;
diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h
index 1267ea8..8de5d1d 100644
--- a/libxfs/xfs_attr.h
+++ b/libxfs/xfs_attr.h
@@ -164,6 +164,264 @@ struct xfs_attr_list_context {
* v
* done
*
+ *
+ * Below is a state machine diagram for attr set operations.
+ *
+ * It seems the challenge with understanding this system comes from trying to
+ * absorb the state machine all at once, when really one should only be looking
+ * at it with in the context of a single function. Once a state sensitive
+ * function is called, the idea is that it "takes ownership" of the
+ * state machine. It isn't concerned with the states that may have belonged to
+ * it's calling parent. Only the states relevant to itself or any other
+ * subroutines there in. Once a calling function hands off the state machine to
+ * a subroutine, it needs to respect the simple rule that it doesn't "own" the
+ * state machine anymore, and it's the responsibility of that calling function
+ * to propagate the -EAGAIN back up the call stack. Upon reentry, it is
+ * committed to re-calling that subroutine until it returns something other than
+ * -EAGAIN. Once that subroutine signals completion (by returning anything other
+ * than -EAGAIN), the calling function can resume using the state machine.
+ *
+ * xfs_attr_set_iter()
+ * │
+ * v
+ * ┌─y─ has an attr fork?
+ * │ |
+ * │ n
+ * │ |
+ * │ V
+ * │ add a fork
+ * │ │
+ * └──────────┤
+ * │
+ * V
+ * ┌─── is shortform?
+ * │ │
+ * │ y
+ * │ │
+ * │ V
+ * │ xfs_attr_set_fmt
+ * │ |
+ * │ V
+ * │ xfs_attr_try_sf_addname
+ * │ │
+ * │ V
+ * │ had enough ──y──> done
+ * │ space?
+ * n │
+ * │ n
+ * │ │
+ * │ V
+ * │ transform to leaf
+ * │ │
+ * │ V
+ * │ hold the leaf buffer
+ * │ │
+ * │ V
+ * │ return -EAGAIN
+ * │ Re-enter in
+ * │ leaf form
+ * │
+ * └─> release leaf buffer
+ * if needed
+ * │
+ * V
+ * ┌───n── fork has
+ * │ only 1 blk?
+ * │ │
+ * │ y
+ * │ │
+ * │ v
+ * │ xfs_attr_leaf_try_add()
+ * │ │
+ * │ v
+ * │ had enough ──────────────y─────────────┐
+ * │ space? │
+ * │ │ │
+ * │ n │
+ * │ │ │
+ * │ v │
+ * │ return -EAGAIN │
+ * │ re-enter in │
+ * │ node form │
+ * │ │ │
+ * └──────────┤ │
+ * │ │
+ * V │
+ * xfs_attr_node_addname_find_attr │
+ * determines if this │
+ * is create or rename │
+ * find space to store attr │
+ * │ │
+ * v │
+ * xfs_attr_node_addname │
+ * │ │
+ * v │
+ * fits in a node leaf? ────n─────┐ │
+ * │ ^ v │
+ * │ │ single leaf node? │
+ * │ │ │ │ │
+ * y │ y n │
+ * │ │ │ │ │
+ * v │ v v │
+ * update │ grow the leaf split if │
+ * hashvals └── return -EAGAIN needed │
+ * │ retry leaf add │ │
+ * │ on reentry │ │
+ * ├────────────────────────────┘ │
+ * │ │
+ * v │
+ * need to alloc │
+ * ┌─y── or flip flag? │
+ * │ │ │
+ * │ n │
+ * │ │ │
+ * │ v │
+ * │ done │
+ * │ │
+ * │ │
+ * │ XFS_DAS_FOUND_LBLK <────────────────┘
+ * │ │
+ * │ V
+ * │ xfs_attr_leaf_addname()
+ * │ │
+ * │ v
+ * │ ┌──first time through?
+ * │ │ │
+ * │ │ y
+ * │ │ │
+ * │ n v
+ * │ │ if we have rmt blks
+ * │ │ find space for them
+ * │ │ │
+ * │ └──────────┤
+ * │ │
+ * │ v
+ * │ still have
+ * │ ┌─n─ blks to alloc? <──┐
+ * │ │ │ │
+ * │ │ y │
+ * │ │ │ │
+ * │ │ v │
+ * │ │ alloc one blk │
+ * │ │ return -EAGAIN ──┘
+ * │ │ re-enter with one
+ * │ │ less blk to alloc
+ * │ │
+ * │ │
+ * │ └───> set the rmt
+ * │ value
+ * │ │
+ * │ v
+ * │ was this
+ * │ a rename? ──n─┐
+ * │ │ │
+ * │ y │
+ * │ │ │
+ * │ v │
+ * │ flip incomplete │
+ * │ flag │
+ * │ │ │
+ * │ v │
+ * │ XFS_DAS_FLIP_LFLAG │
+ * │ │ │
+ * │ v │
+ * │ need to remove │
+ * │ old bks? ──n──┤
+ * │ │ │
+ * │ y │
+ * │ │ │
+ * │ V │
+ * │ remove │
+ * │ ┌───> old blks │
+ * │ │ │ │
+ * │ XFS_DAS_RM_LBLK │ │
+ * │ ^ │ │
+ * │ │ v │
+ * │ └──y── more to │
+ * │ remove? │
+ * │ │ │
+ * │ n │
+ * │ │ │
+ * │ v │
+ * │ XFS_DAS_RD_LEAF │
+ * │ │ │
+ * │ v │
+ * │ remove leaf │
+ * │ │ │
+ * │ v │
+ * │ shrink to sf │
+ * │ if needed │
+ * │ │ │
+ * │ v │
+ * │ done <──────┘
+ * │
+ * └──────> XFS_DAS_FOUND_NBLK
+ * │
+ * v
+ * ┌─────n── need to
+ * │ alloc blks?
+ * │ │
+ * │ y
+ * │ │
+ * │ v
+ * │ find space
+ * │ │
+ * │ v
+ * │ ┌─>XFS_DAS_ALLOC_NODE
+ * │ │ │
+ * │ │ v
+ * │ │ alloc blk
+ * │ │ │
+ * │ │ v
+ * │ └──y── need to alloc
+ * │ more blocks?
+ * │ │
+ * │ n
+ * │ │
+ * │ v
+ * │ set the rmt value
+ * │ │
+ * │ v
+ * │ was this
+ * └────────> a rename? ──n─┐
+ * │ │
+ * y │
+ * │ │
+ * v │
+ * flip incomplete │
+ * flag │
+ * │ │
+ * v │
+ * XFS_DAS_FLIP_NFLAG │
+ * │ │
+ * v │
+ * need to │
+ * remove blks? ─n──┤
+ * │ │
+ * y │
+ * │ │
+ * v │
+ * remove │
+ * ┌────────> old blks │
+ * │ │ │
+ * XFS_DAS_RM_NBLK │ │
+ * ^ │ │
+ * │ v │
+ * └──────y── more to │
+ * remove │
+ * │ │
+ * n │
+ * │ │
+ * v │
+ * XFS_DAS_CLR_FLAG │
+ * │ │
+ * v │
+ * clear flags │
+ * │ │
+ * ├──────────┘
+ * │
+ * v
+ * done
*/
/*
@@ -180,12 +438,22 @@ enum xfs_delattr_state {
XFS_DAS_RMTBLK, /* Removing remote blks */
XFS_DAS_RM_NAME, /* Remove attr name */
XFS_DAS_RM_SHRINK, /* We are shrinking the tree */
+ XFS_DAS_FOUND_LBLK, /* We found leaf blk for attr */
+ XFS_DAS_FOUND_NBLK, /* We found node blk for attr */
+ XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */
+ XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */
+ XFS_DAS_RD_LEAF, /* Read in the new leaf */
+ XFS_DAS_ALLOC_NODE, /* We are allocating node blocks */
+ XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */
+ XFS_DAS_RM_NBLK, /* A rename is removing node blocks */
+ XFS_DAS_CLR_FLAG, /* Clear incomplete flag */
};
/*
* Defines for xfs_delattr_context.flags
*/
#define XFS_DAC_DEFER_FINISH 0x01 /* finish the transaction */
+#define XFS_DAC_LEAF_ADDNAME_INIT 0x02 /* xfs_attr_leaf_addname init*/
/*
* Context used for keeping track of delayed attribute operations
@@ -193,6 +461,11 @@ enum xfs_delattr_state {
struct xfs_delattr_context {
struct xfs_da_args *da_args;
+ /* Used in xfs_attr_rmtval_set_blk to roll through allocating blocks */
+ struct xfs_bmbt_irec map;
+ xfs_dablk_t lblkno;
+ int blkcnt;
+
/* Used in xfs_attr_node_removename to roll through removing blocks */
struct xfs_da_state *da_state;
@@ -220,7 +493,6 @@ int xfs_attr_set_args(struct xfs_da_args *args);
int xfs_has_attr(struct xfs_da_args *args);
int xfs_attr_remove_args(struct xfs_da_args *args);
int xfs_attr_remove_iter(struct xfs_delattr_context *dac);
-int xfs_attr_trans_roll(struct xfs_delattr_context *dac);
bool xfs_attr_namecheck(const void *name, size_t length);
void xfs_delattr_context_init(struct xfs_delattr_context *dac,
struct xfs_da_args *args);
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index e41bbb2..5a0699e 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -438,9 +438,9 @@ xfs_attr_rmtval_get(
/*
* Find a "hole" in the attribute address space large enough for us to drop the
- * new attribute's value into
+ * new attributes value into
*/
-STATIC int
+int
xfs_attr_rmt_find_hole(
struct xfs_da_args *args)
{
@@ -467,7 +467,7 @@ xfs_attr_rmt_find_hole(
return 0;
}
-STATIC int
+int
xfs_attr_rmtval_set_value(
struct xfs_da_args *args)
{
@@ -627,6 +627,69 @@ xfs_attr_rmtval_set(
}
/*
+ * Find a hole for the attr and store it in the delayed attr context. This
+ * initializes the context to roll through allocating an attr extent for a
+ * delayed attr operation
+ */
+int
+xfs_attr_rmtval_find_space(
+ struct xfs_delattr_context *dac)
+{
+ struct xfs_da_args *args = dac->da_args;
+ struct xfs_bmbt_irec *map = &dac->map;
+ int error;
+
+ dac->lblkno = 0;
+ dac->blkcnt = 0;
+ args->rmtblkcnt = 0;
+ args->rmtblkno = 0;
+ memset(map, 0, sizeof(struct xfs_bmbt_irec));
+
+ error = xfs_attr_rmt_find_hole(args);
+ if (error)
+ return error;
+
+ dac->blkcnt = args->rmtblkcnt;
+ dac->lblkno = args->rmtblkno;
+
+ return 0;
+}
+
+/*
+ * Write one block of the value associated with an attribute into the
+ * out-of-line buffer that we have defined for it. This is similar to a subset
+ * of xfs_attr_rmtval_set, but records the current block to the delayed attr
+ * context, and leaves transaction handling to the caller.
+ */
+int
+xfs_attr_rmtval_set_blk(
+ struct xfs_delattr_context *dac)
+{
+ struct xfs_da_args *args = dac->da_args;
+ struct xfs_inode *dp = args->dp;
+ struct xfs_bmbt_irec *map = &dac->map;
+ int nmap;
+ int error;
+
+ nmap = 1;
+ error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)dac->lblkno,
+ dac->blkcnt, XFS_BMAPI_ATTRFORK, args->total,
+ map, &nmap);
+ if (error)
+ return error;
+
+ ASSERT(nmap == 1);
+ ASSERT((map->br_startblock != DELAYSTARTBLOCK) &&
+ (map->br_startblock != HOLESTARTBLOCK));
+
+ /* roll attribute extent map forwards */
+ dac->lblkno += map->br_blockcount;
+ dac->blkcnt -= map->br_blockcount;
+
+ return 0;
+}
+
+/*
* Remove the value associated with an attribute by deleting the
* out-of-line buffer that it is stored on.
*/
@@ -668,37 +731,6 @@ xfs_attr_rmtval_invalidate(
}
/*
- * Remove the value associated with an attribute by deleting the
- * out-of-line buffer that it is stored on.
- */
-int
-xfs_attr_rmtval_remove(
- struct xfs_da_args *args)
-{
- int error;
- struct xfs_delattr_context dac = {
- .da_args = args,
- };
-
- trace_xfs_attr_rmtval_remove(args);
-
- /*
- * Keep de-allocating extents until the remote-value region is gone.
- */
- do {
- error = __xfs_attr_rmtval_remove(&dac);
- if (error && error != -EAGAIN)
- break;
-
- error = xfs_attr_trans_roll(&dac);
- if (error)
- return error;
- } while (true);
-
- return error;
-}
-
-/*
* Remove the value associated with an attribute by deleting the out-of-line
* buffer that it is stored on. Returns -EAGAIN for the caller to refresh the
* transaction and re-call the function. Callers should keep calling this
diff --git a/libxfs/xfs_attr_remote.h b/libxfs/xfs_attr_remote.h
index 002fd30..8ad68d5 100644
--- a/libxfs/xfs_attr_remote.h
+++ b/libxfs/xfs_attr_remote.h
@@ -10,9 +10,12 @@ int xfs_attr3_rmt_blocks(struct xfs_mount *mp, int attrlen);
int xfs_attr_rmtval_get(struct xfs_da_args *args);
int xfs_attr_rmtval_set(struct xfs_da_args *args);
-int xfs_attr_rmtval_remove(struct xfs_da_args *args);
int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map,
xfs_buf_flags_t incore_flags);
int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
int __xfs_attr_rmtval_remove(struct xfs_delattr_context *dac);
+int xfs_attr_rmt_find_hole(struct xfs_da_args *args);
+int xfs_attr_rmtval_set_value(struct xfs_da_args *args);
+int xfs_attr_rmtval_set_blk(struct xfs_delattr_context *dac);
+int xfs_attr_rmtval_find_space(struct xfs_delattr_context *dac);
#endif /* __XFS_ATTR_REMOTE_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 11/27] xfsprogs: Remove xfs_attr_rmtval_set
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (9 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 10/27] xfsprogs: Add delay ready attr set routines Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 12/27] xfsprogs: Clean up xfs_attr_node_addname_clear_incomplete Allison Henderson
` (15 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
This function is no longer used, so it is safe to remove
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
---
libxfs/xfs_attr_remote.c | 66 ------------------------------------------------
libxfs/xfs_attr_remote.h | 1 -
2 files changed, 67 deletions(-)
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index 5a0699e..d474ad7 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -561,72 +561,6 @@ xfs_attr_rmtval_stale(
}
/*
- * Write the value associated with an attribute into the out-of-line buffer
- * that we have defined for it.
- */
-int
-xfs_attr_rmtval_set(
- struct xfs_da_args *args)
-{
- struct xfs_inode *dp = args->dp;
- struct xfs_bmbt_irec map;
- xfs_dablk_t lblkno;
- int blkcnt;
- int nmap;
- int error;
-
- trace_xfs_attr_rmtval_set(args);
-
- error = xfs_attr_rmt_find_hole(args);
- if (error)
- return error;
-
- blkcnt = args->rmtblkcnt;
- lblkno = (xfs_dablk_t)args->rmtblkno;
- /*
- * Roll through the "value", allocating blocks on disk as required.
- */
- while (blkcnt > 0) {
- /*
- * Allocate a single extent, up to the size of the value.
- *
- * Note that we have to consider this a data allocation as we
- * write the remote attribute without logging the contents.
- * Hence we must ensure that we aren't using blocks that are on
- * the busy list so that we don't overwrite blocks which have
- * recently been freed but their transactions are not yet
- * committed to disk. If we overwrite the contents of a busy
- * extent and then crash then the block may not contain the
- * correct metadata after log recovery occurs.
- */
- nmap = 1;
- error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)lblkno,
- blkcnt, XFS_BMAPI_ATTRFORK, args->total, &map,
- &nmap);
- if (error)
- return error;
- error = xfs_defer_finish(&args->trans);
- if (error)
- return error;
-
- ASSERT(nmap == 1);
- ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
- (map.br_startblock != HOLESTARTBLOCK));
- lblkno += map.br_blockcount;
- blkcnt -= map.br_blockcount;
-
- /*
- * Start the next trans in the chain.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- return error;
- }
-
- return xfs_attr_rmtval_set_value(args);
-}
-
-/*
* Find a hole for the attr and store it in the delayed attr context. This
* initializes the context to roll through allocating an attr extent for a
* delayed attr operation
diff --git a/libxfs/xfs_attr_remote.h b/libxfs/xfs_attr_remote.h
index 8ad68d5..61b85b9 100644
--- a/libxfs/xfs_attr_remote.h
+++ b/libxfs/xfs_attr_remote.h
@@ -9,7 +9,6 @@
int xfs_attr3_rmt_blocks(struct xfs_mount *mp, int attrlen);
int xfs_attr_rmtval_get(struct xfs_da_args *args);
-int xfs_attr_rmtval_set(struct xfs_da_args *args);
int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map,
xfs_buf_flags_t incore_flags);
int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 12/27] xfsprogs: Clean up xfs_attr_node_addname_clear_incomplete
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (10 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 11/27] xfsprogs: Remove xfs_attr_rmtval_set Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 13/27] xfsprogs: Fix default ASSERT in xfs_attr_set_iter Allison Henderson
` (14 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 0c0dea93623d000994ee29f86abd836b258b5c80
We can use the helper function xfs_attr_node_remove_name to reduce
duplicate code in this function
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
libxfs/xfs_attr.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 347f854..edc19de 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -63,6 +63,8 @@ STATIC int xfs_attr_fillstate(xfs_da_state_t *state);
STATIC int xfs_attr_refillstate(xfs_da_state_t *state);
STATIC int xfs_attr_set_iter(struct xfs_delattr_context *dac,
struct xfs_buf **leaf_bp);
+STATIC int xfs_attr_node_remove_name(struct xfs_da_args *args,
+ struct xfs_da_state *state);
int
xfs_inode_hasattr(
@@ -1207,7 +1209,6 @@ xfs_attr_node_addname_clear_incomplete(
{
struct xfs_da_args *args = dac->da_args;
struct xfs_da_state *state = NULL;
- struct xfs_da_state_blk *blk;
int retval = 0;
int error = 0;
@@ -1222,13 +1223,7 @@ xfs_attr_node_addname_clear_incomplete(
if (error)
goto out;
- /*
- * 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);
- error = xfs_attr3_leaf_remove(blk->bp, args);
- xfs_da3_fixhashpath(state, &state->path);
+ error = xfs_attr_node_remove_name(args, state);
/*
* Check to see if the tree needs to be collapsed.
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 13/27] xfsprogs: Fix default ASSERT in xfs_attr_set_iter
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (11 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 12/27] xfsprogs: Clean up xfs_attr_node_addname_clear_incomplete Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 14/27] xfsprogs: Make attr name schemes consistent Allison Henderson
` (13 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
This ASSERT checks for the state value of RM_SHRINK in the set path
which should never happen. Change to ASSERT(0);
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
libxfs/xfs_attr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index edc19de..cbac761 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -612,7 +612,7 @@ xfs_attr_set_iter(
error = xfs_attr_node_addname_clear_incomplete(dac);
break;
default:
- ASSERT(dac->dela_state != XFS_DAS_RM_SHRINK);
+ ASSERT(0);
break;
}
out:
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 14/27] xfsprogs: Make attr name schemes consistent
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (12 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 13/27] xfsprogs: Fix default ASSERT in xfs_attr_set_iter Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 15/27] xfs: allow setting and clearing of log incompat feature flags Allison Henderson
` (12 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: c910d5b5ad8de13953197637b74f3e5c25960410
This patch renames the following functions to make the nameing scheme more consistent:
xfs_attr_shortform_remove -> xfs_attr_sf_removename
xfs_attr_node_remove_name -> xfs_attr_node_removename
xfs_attr_set_fmt -> xfs_attr_sf_addname
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
libxfs/xfs_attr.c | 18 +++++++++---------
libxfs/xfs_attr_leaf.c | 2 +-
libxfs/xfs_attr_leaf.h | 2 +-
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index cbac761..8f6f175 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -63,8 +63,8 @@ STATIC int xfs_attr_fillstate(xfs_da_state_t *state);
STATIC int xfs_attr_refillstate(xfs_da_state_t *state);
STATIC int xfs_attr_set_iter(struct xfs_delattr_context *dac,
struct xfs_buf **leaf_bp);
-STATIC int xfs_attr_node_remove_name(struct xfs_da_args *args,
- struct xfs_da_state *state);
+STATIC int xfs_attr_node_removename(struct xfs_da_args *args,
+ struct xfs_da_state *state);
int
xfs_inode_hasattr(
@@ -298,7 +298,7 @@ xfs_attr_set_args(
}
STATIC int
-xfs_attr_set_fmt(
+xfs_attr_sf_addname(
struct xfs_delattr_context *dac,
struct xfs_buf **leaf_bp)
{
@@ -367,7 +367,7 @@ xfs_attr_set_iter(
* release the hold once we return with a clean transaction.
*/
if (xfs_attr_is_shortform(dp))
- return xfs_attr_set_fmt(dac, leaf_bp);
+ return xfs_attr_sf_addname(dac, leaf_bp);
if (*leaf_bp != NULL) {
xfs_trans_bhold_release(args->trans, *leaf_bp);
*leaf_bp = NULL;
@@ -840,7 +840,7 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
if (retval == -EEXIST) {
if (args->attr_flags & XATTR_CREATE)
return retval;
- retval = xfs_attr_shortform_remove(args);
+ retval = xfs_attr_sf_removename(args);
if (retval)
return retval;
/*
@@ -1223,7 +1223,7 @@ xfs_attr_node_addname_clear_incomplete(
if (error)
goto out;
- error = xfs_attr_node_remove_name(args, state);
+ error = xfs_attr_node_removename(args, state);
/*
* Check to see if the tree needs to be collapsed.
@@ -1339,7 +1339,7 @@ out:
}
STATIC int
-xfs_attr_node_remove_name(
+xfs_attr_node_removename(
struct xfs_da_args *args,
struct xfs_da_state *state)
{
@@ -1390,7 +1390,7 @@ xfs_attr_remove_iter(
* thus state transitions. Call the right helper and return.
*/
if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
- return xfs_attr_shortform_remove(args);
+ return xfs_attr_sf_removename(args);
if (xfs_attr_is_leaf(dp))
return xfs_attr_leaf_removename(args);
@@ -1453,7 +1453,7 @@ xfs_attr_remove_iter(
goto out;
}
- retval = xfs_attr_node_remove_name(args, state);
+ retval = xfs_attr_node_removename(args, state);
/*
* Check to see if the tree needs to be collapsed. If so, roll
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index 08600ea..e23fc3d 100644
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -770,7 +770,7 @@ xfs_attr_fork_remove(
* Remove an attribute from the shortform attribute list structure.
*/
int
-xfs_attr_shortform_remove(
+xfs_attr_sf_removename(
struct xfs_da_args *args)
{
struct xfs_attr_shortform *sf;
diff --git a/libxfs/xfs_attr_leaf.h b/libxfs/xfs_attr_leaf.h
index 9b1c59f..efa757f 100644
--- a/libxfs/xfs_attr_leaf.h
+++ b/libxfs/xfs_attr_leaf.h
@@ -51,7 +51,7 @@ int xfs_attr_shortform_lookup(struct xfs_da_args *args);
int xfs_attr_shortform_getvalue(struct xfs_da_args *args);
int xfs_attr_shortform_to_leaf(struct xfs_da_args *args,
struct xfs_buf **leaf_bp);
-int xfs_attr_shortform_remove(struct xfs_da_args *args);
+int xfs_attr_sf_removename(struct xfs_da_args *args);
int xfs_attr_sf_findname(struct xfs_da_args *args,
struct xfs_attr_sf_entry **sfep,
unsigned int *basep);
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 15/27] xfs: allow setting and clearing of log incompat feature flags
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (13 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 14/27] xfsprogs: Make attr name schemes consistent Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 16/27] xfsprogs: Return from xfs_attr_set_iter if there are no more rmtblks to process Allison Henderson
` (11 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
From: "Darrick J. Wong" <djwong@kernel.org>
Source kernel commit: a9642049d91105c676526817b527a1a62940888f
Log incompat feature flags in the superblock exist for one purpose: to
protect the contents of a dirty log from replay on a kernel that isn't
prepared to handle those dirty contents. This means that they can be
cleared if (a) we know the log is clean and (b) we know that there
aren't any other threads in the system that might be setting or relying
upon a log incompat flag.
Therefore, clear the log incompat flags when we've finished recovering
the log, when we're unmounting cleanly, remounting read-only, or
freezing; and provide a function so that subsequent patches can start
using this.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
libxfs/xfs_format.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/libxfs/xfs_format.h b/libxfs/xfs_format.h
index 37570cf..5d8a129 100644
--- a/libxfs/xfs_format.h
+++ b/libxfs/xfs_format.h
@@ -495,6 +495,21 @@ xfs_sb_has_incompat_log_feature(
return (sbp->sb_features_log_incompat & feature) != 0;
}
+static inline void
+xfs_sb_remove_incompat_log_features(
+ struct xfs_sb *sbp)
+{
+ sbp->sb_features_log_incompat &= ~XFS_SB_FEAT_INCOMPAT_LOG_ALL;
+}
+
+static inline void
+xfs_sb_add_incompat_log_features(
+ struct xfs_sb *sbp,
+ unsigned int features)
+{
+ sbp->sb_features_log_incompat |= features;
+}
+
/*
* V5 superblock specific feature checks
*/
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 16/27] xfsprogs: Return from xfs_attr_set_iter if there are no more rmtblks to process
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (14 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 15/27] xfs: allow setting and clearing of log incompat feature flags Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 17/27] xfsprogs: Add state machine tracepoints Allison Henderson
` (10 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: df21062113850c5aaeaa38b5194ee4c64767fb7a
During an attr rename operation, blocks are saved for later removal
as rmtblkno2. The rmtblkno is used in the case of needing to alloc
more blocks if not enough were available. However, in the case
that neither rmtblkno or rmtblkno2 are set, we can return as soon
as xfs_attr_node_addname completes, rather than rolling the transaction
with an -EAGAIN return. This extra loop does not hurt anything right
now, but it will be a problem later when we get into log items because
we end up with an empty log transaction. So, add a simple check to
cut out the unneeded iteration.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
libxfs/xfs_attr.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 8f6f175..84b88f9 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -409,6 +409,13 @@ xfs_attr_set_iter(
if (error)
return error;
+ /*
+ * If addname was successful, and we dont need to alloc
+ * or remove anymore blks, we're done.
+ */
+ if (!args->rmtblkno && !args->rmtblkno2)
+ return 0;
+
dac->dela_state = XFS_DAS_FOUND_NBLK;
}
return -EAGAIN;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 17/27] xfsprogs: Add state machine tracepoints
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (15 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 16/27] xfsprogs: Return from xfs_attr_set_iter if there are no more rmtblks to process Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 18/27] xfsprogs: Rename __xfs_attr_rmtval_remove Allison Henderson
` (9 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: fbc8d6bb875915e0afd8ff6cd4364b368a6f894f
This is a quick patch to add a new tracepoint: xfs_das_state_return. We
use this to track when ever a new state is set or -EAGAIN is returned
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
include/xfs_trace.h | 6 ++++++
libxfs/xfs_attr.c | 28 ++++++++++++++++++++++++++--
libxfs/xfs_attr_remote.c | 1 +
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/include/xfs_trace.h b/include/xfs_trace.h
index a847b50..2169c27 100644
--- a/include/xfs_trace.h
+++ b/include/xfs_trace.h
@@ -312,4 +312,10 @@
#define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
#define trace_xfs_perag_put(a,b,c,d) ((c) = (c))
+#define trace_xfs_attr_sf_addname_return(a,b) ((void) 0)
+#define trace_xfs_attr_set_iter_return(a,b) ((void) 0)
+#define trace_xfs_attr_node_addname_return(a,b) ((void) 0)
+#define trace_xfs_attr_remove_iter_return(a,b) ((void) 0)
+#define trace_xfs_attr_rmtval_remove_return(a,b) ((void) 0)
+
#endif /* __TRACE_H__ */
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 84b88f9..6380cae 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -335,6 +335,7 @@ xfs_attr_sf_addname(
* the attr fork to leaf format and will restart with the leaf
* add.
*/
+ trace_xfs_attr_sf_addname_return(XFS_DAS_UNINIT, args->dp);
dac->flags |= XFS_DAC_DEFER_FINISH;
return -EAGAIN;
}
@@ -394,6 +395,8 @@ xfs_attr_set_iter(
* handling code below
*/
dac->flags |= XFS_DAC_DEFER_FINISH;
+ trace_xfs_attr_set_iter_return(
+ dac->dela_state, args->dp);
return -EAGAIN;
} else if (error) {
return error;
@@ -418,6 +421,7 @@ xfs_attr_set_iter(
dac->dela_state = XFS_DAS_FOUND_NBLK;
}
+ trace_xfs_attr_set_iter_return(dac->dela_state, args->dp);
return -EAGAIN;
case XFS_DAS_FOUND_LBLK:
/*
@@ -445,6 +449,8 @@ xfs_attr_set_iter(
error = xfs_attr_rmtval_set_blk(dac);
if (error)
return error;
+ trace_xfs_attr_set_iter_return(dac->dela_state,
+ args->dp);
return -EAGAIN;
}
@@ -479,6 +485,7 @@ xfs_attr_set_iter(
* series.
*/
dac->dela_state = XFS_DAS_FLIP_LFLAG;
+ trace_xfs_attr_set_iter_return(dac->dela_state, args->dp);
return -EAGAIN;
case XFS_DAS_FLIP_LFLAG:
/*
@@ -496,6 +503,9 @@ xfs_attr_set_iter(
dac->dela_state = XFS_DAS_RM_LBLK;
if (args->rmtblkno) {
error = __xfs_attr_rmtval_remove(dac);
+ if (error == -EAGAIN)
+ trace_xfs_attr_set_iter_return(
+ dac->dela_state, args->dp);
if (error)
return error;
@@ -549,6 +559,8 @@ xfs_attr_set_iter(
error = xfs_attr_rmtval_set_blk(dac);
if (error)
return error;
+ trace_xfs_attr_set_iter_return(
+ dac->dela_state, args->dp);
return -EAGAIN;
}
@@ -584,6 +596,7 @@ xfs_attr_set_iter(
* series
*/
dac->dela_state = XFS_DAS_FLIP_NFLAG;
+ trace_xfs_attr_set_iter_return(dac->dela_state, args->dp);
return -EAGAIN;
case XFS_DAS_FLIP_NFLAG:
@@ -603,6 +616,10 @@ xfs_attr_set_iter(
dac->dela_state = XFS_DAS_RM_NBLK;
if (args->rmtblkno) {
error = __xfs_attr_rmtval_remove(dac);
+ if (error == -EAGAIN)
+ trace_xfs_attr_set_iter_return(
+ dac->dela_state, args->dp);
+
if (error)
return error;
@@ -1183,6 +1200,8 @@ xfs_attr_node_addname(
* this point.
*/
dac->flags |= XFS_DAC_DEFER_FINISH;
+ trace_xfs_attr_node_addname_return(
+ dac->dela_state, args->dp);
return -EAGAIN;
}
@@ -1429,10 +1448,13 @@ xfs_attr_remove_iter(
* blocks are removed.
*/
error = __xfs_attr_rmtval_remove(dac);
- if (error == -EAGAIN)
+ if (error == -EAGAIN) {
+ trace_xfs_attr_remove_iter_return(
+ dac->dela_state, args->dp);
return error;
- else if (error)
+ } else if (error) {
goto out;
+ }
/*
* Refill the state structure with buffers (the prior
@@ -1473,6 +1495,8 @@ xfs_attr_remove_iter(
dac->flags |= XFS_DAC_DEFER_FINISH;
dac->dela_state = XFS_DAS_RM_SHRINK;
+ trace_xfs_attr_remove_iter_return(
+ dac->dela_state, args->dp);
return -EAGAIN;
}
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index d474ad7..137e569 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -695,6 +695,7 @@ __xfs_attr_rmtval_remove(
*/
if (!done) {
dac->flags |= XFS_DAC_DEFER_FINISH;
+ trace_xfs_attr_rmtval_remove_return(dac->dela_state, args->dp);
return -EAGAIN;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 18/27] xfsprogs: Rename __xfs_attr_rmtval_remove
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (16 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 17/27] xfsprogs: Add state machine tracepoints Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 19/27] xfsprogs: Set up infrastructure for deferred attribute operations Allison Henderson
` (8 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Now that xfs_attr_rmtval_remove is gone, rename __xfs_attr_rmtval_remove
to xfs_attr_rmtval_remove
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
libxfs/xfs_attr.c | 6 +++---
libxfs/xfs_attr_remote.c | 2 +-
libxfs/xfs_attr_remote.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 6380cae..f2db0d5 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -502,7 +502,7 @@ xfs_attr_set_iter(
/* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
dac->dela_state = XFS_DAS_RM_LBLK;
if (args->rmtblkno) {
- error = __xfs_attr_rmtval_remove(dac);
+ error = xfs_attr_rmtval_remove(dac);
if (error == -EAGAIN)
trace_xfs_attr_set_iter_return(
dac->dela_state, args->dp);
@@ -615,7 +615,7 @@ xfs_attr_set_iter(
/* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
dac->dela_state = XFS_DAS_RM_NBLK;
if (args->rmtblkno) {
- error = __xfs_attr_rmtval_remove(dac);
+ error = xfs_attr_rmtval_remove(dac);
if (error == -EAGAIN)
trace_xfs_attr_set_iter_return(
dac->dela_state, args->dp);
@@ -1447,7 +1447,7 @@ xfs_attr_remove_iter(
* May return -EAGAIN. Roll and repeat until all remote
* blocks are removed.
*/
- error = __xfs_attr_rmtval_remove(dac);
+ error = xfs_attr_rmtval_remove(dac);
if (error == -EAGAIN) {
trace_xfs_attr_remove_iter_return(
dac->dela_state, args->dp);
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index 137e569..b781e44 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -671,7 +671,7 @@ xfs_attr_rmtval_invalidate(
* routine until it returns something other than -EAGAIN.
*/
int
-__xfs_attr_rmtval_remove(
+xfs_attr_rmtval_remove(
struct xfs_delattr_context *dac)
{
struct xfs_da_args *args = dac->da_args;
diff --git a/libxfs/xfs_attr_remote.h b/libxfs/xfs_attr_remote.h
index 61b85b9..d72eff3 100644
--- a/libxfs/xfs_attr_remote.h
+++ b/libxfs/xfs_attr_remote.h
@@ -12,7 +12,7 @@ int xfs_attr_rmtval_get(struct xfs_da_args *args);
int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map,
xfs_buf_flags_t incore_flags);
int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
-int __xfs_attr_rmtval_remove(struct xfs_delattr_context *dac);
+int xfs_attr_rmtval_remove(struct xfs_delattr_context *dac);
int xfs_attr_rmt_find_hole(struct xfs_da_args *args);
int xfs_attr_rmtval_set_value(struct xfs_da_args *args);
int xfs_attr_rmtval_set_blk(struct xfs_delattr_context *dac);
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 19/27] xfsprogs: Set up infrastructure for deferred attribute operations
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (17 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 18/27] xfsprogs: Rename __xfs_attr_rmtval_remove Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 20/27] xfsprogs: Implement attr logging and replay Allison Henderson
` (7 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Currently attributes are modified directly across one or more
transactions. But they are not logged or replayed in the event of an
error. The goal of delayed attributes is to enable logging and replaying
of attribute operations using the existing delayed operations
infrastructure. This will later enable the attributes to become part of
larger multi part operations that also must first be recorded to the
log. This is mostly of interest in the scheme of parent pointers which
would need to maintain an attribute containing parent inode information
any time an inode is moved, created, or removed. Parent pointers would
then be of interest to any feature that would need to quickly derive an
inode path from the mount point. Online scrub, nfs lookups and fs grow
or shrink operations are all features that could take advantage of this.
This patch adds two new log item types for setting or removing
attributes as deferred operations. The xfs_attri_log_item will log an
intent to set or remove an attribute. The corresponding
xfs_attrd_log_item holds a reference to the xfs_attri_log_item and is
freed once the transaction is done. Both log items use a generic
xfs_attr_log_format structure that contains the attribute name, value,
flags, inode, and an op_flag that indicates if the operations is a set
or remove.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
libxfs/defer_item.c | 3 +++
libxfs/libxfs_priv.h | 1 +
libxfs/xfs_attr.c | 4 +---
libxfs/xfs_attr.h | 31 +++++++++++++++++++++++++++++++
libxfs/xfs_defer.h | 1 +
libxfs/xfs_log_format.h | 43 +++++++++++++++++++++++++++++++++++++++++--
6 files changed, 78 insertions(+), 5 deletions(-)
diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
index b18182e..a1f0d7e 100644
--- a/libxfs/defer_item.c
+++ b/libxfs/defer_item.c
@@ -20,6 +20,9 @@
#include "xfs_refcount.h"
#include "xfs_bmap.h"
#include "xfs_inode.h"
+#include "xfs_da_format.h"
+#include "xfs_da_btree.h"
+#include "xfs_attr.h"
/* Dummy defer item ops, since we don't do logging. */
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 7181a858..0272ef2 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -518,6 +518,7 @@ static inline int retzero(void) { return 0; }
#define xfs_quota_reserve_blkres(i,b) (0)
#define xfs_qm_dqattach(i) (0)
+#define xfs_qm_dqattach_locked(i,b) (0)
#define uuid_copy(s,d) platform_uuid_copy((s),(d))
#define uuid_equal(s,d) (platform_uuid_compare((s),(d)) == 0)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index f2db0d5..70665d9 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -61,8 +61,6 @@ 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_set_iter(struct xfs_delattr_context *dac,
- struct xfs_buf **leaf_bp);
STATIC int xfs_attr_node_removename(struct xfs_da_args *args,
struct xfs_da_state *state);
@@ -166,7 +164,7 @@ xfs_attr_get(
/*
* Calculate how many blocks we need for the new attribute,
*/
-STATIC int
+int
xfs_attr_calc_size(
struct xfs_da_args *args,
int *local)
diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h
index 8de5d1d..463b2be 100644
--- a/libxfs/xfs_attr.h
+++ b/libxfs/xfs_attr.h
@@ -28,6 +28,11 @@ struct xfs_attr_list_context;
*/
#define ATTR_MAX_VALUELEN (64*1024) /* max length of a value */
+static inline bool xfs_hasdelattr(struct xfs_mount *mp)
+{
+ return false;
+}
+
/*
* Kernel-internal version of the attrlist cursor.
*/
@@ -454,6 +459,7 @@ enum xfs_delattr_state {
*/
#define XFS_DAC_DEFER_FINISH 0x01 /* finish the transaction */
#define XFS_DAC_LEAF_ADDNAME_INIT 0x02 /* xfs_attr_leaf_addname init*/
+#define XFS_DAC_DELAYED_OP_INIT 0x04 /* delayed operations init*/
/*
* Context used for keeping track of delayed attribute operations
@@ -461,6 +467,11 @@ enum xfs_delattr_state {
struct xfs_delattr_context {
struct xfs_da_args *da_args;
+ /*
+ * Used by xfs_attr_set to hold a leaf buffer across a transaction roll
+ */
+ struct xfs_buf *leaf_bp;
+
/* Used in xfs_attr_rmtval_set_blk to roll through allocating blocks */
struct xfs_bmbt_irec map;
xfs_dablk_t lblkno;
@@ -474,6 +485,23 @@ struct xfs_delattr_context {
enum xfs_delattr_state dela_state;
};
+/*
+ * List of attrs to commit later.
+ */
+struct xfs_attr_item {
+ struct xfs_delattr_context xattri_dac;
+
+ /*
+ * Indicates if the attr operation is a set or a remove
+ * XFS_ATTR_OP_FLAGS_{SET,REMOVE}
+ */
+ unsigned int xattri_op_flags;
+
+ /* used to log this item to an intent */
+ struct list_head xattri_list;
+};
+
+
/*========================================================================
* Function prototypes for the kernel.
*========================================================================*/
@@ -490,11 +518,14 @@ int xfs_attr_get_ilocked(struct xfs_da_args *args);
int xfs_attr_get(struct xfs_da_args *args);
int xfs_attr_set(struct xfs_da_args *args);
int xfs_attr_set_args(struct xfs_da_args *args);
+int xfs_attr_set_iter(struct xfs_delattr_context *dac,
+ struct xfs_buf **leaf_bp);
int xfs_has_attr(struct xfs_da_args *args);
int xfs_attr_remove_args(struct xfs_da_args *args);
int xfs_attr_remove_iter(struct xfs_delattr_context *dac);
bool xfs_attr_namecheck(const void *name, size_t length);
void xfs_delattr_context_init(struct xfs_delattr_context *dac,
struct xfs_da_args *args);
+int xfs_attr_calc_size(struct xfs_da_args *args, int *local);
#endif /* __XFS_ATTR_H__ */
diff --git a/libxfs/xfs_defer.h b/libxfs/xfs_defer.h
index 05472f7..7566f61 100644
--- a/libxfs/xfs_defer.h
+++ b/libxfs/xfs_defer.h
@@ -63,6 +63,7 @@ extern const struct xfs_defer_op_type xfs_refcount_update_defer_type;
extern const struct xfs_defer_op_type xfs_rmap_update_defer_type;
extern const struct xfs_defer_op_type xfs_extent_free_defer_type;
extern const struct xfs_defer_op_type xfs_agfl_free_defer_type;
+extern const struct xfs_defer_op_type xfs_attr_defer_type;
/*
* This structure enables a dfops user to detach the chain of deferred
diff --git a/libxfs/xfs_log_format.h b/libxfs/xfs_log_format.h
index 3e15ea2..66ea559 100644
--- a/libxfs/xfs_log_format.h
+++ b/libxfs/xfs_log_format.h
@@ -117,7 +117,11 @@ struct xfs_unmount_log_format {
#define XLOG_REG_TYPE_CUD_FORMAT 24
#define XLOG_REG_TYPE_BUI_FORMAT 25
#define XLOG_REG_TYPE_BUD_FORMAT 26
-#define XLOG_REG_TYPE_MAX 26
+#define XLOG_REG_TYPE_ATTRI_FORMAT 27
+#define XLOG_REG_TYPE_ATTRD_FORMAT 28
+#define XLOG_REG_TYPE_ATTR_NAME 29
+#define XLOG_REG_TYPE_ATTR_VALUE 30
+#define XLOG_REG_TYPE_MAX 30
/*
* Flags to log operation header
@@ -240,6 +244,8 @@ typedef struct xfs_trans_header {
#define XFS_LI_CUD 0x1243
#define XFS_LI_BUI 0x1244 /* bmbt update intent */
#define XFS_LI_BUD 0x1245
+#define XFS_LI_ATTRI 0x1246 /* attr set/remove intent*/
+#define XFS_LI_ATTRD 0x1247 /* attr set/remove done */
#define XFS_LI_TYPE_DESC \
{ XFS_LI_EFI, "XFS_LI_EFI" }, \
@@ -255,7 +261,9 @@ typedef struct xfs_trans_header {
{ XFS_LI_CUI, "XFS_LI_CUI" }, \
{ XFS_LI_CUD, "XFS_LI_CUD" }, \
{ XFS_LI_BUI, "XFS_LI_BUI" }, \
- { XFS_LI_BUD, "XFS_LI_BUD" }
+ { XFS_LI_BUD, "XFS_LI_BUD" }, \
+ { XFS_LI_ATTRI, "XFS_LI_ATTRI" }, \
+ { XFS_LI_ATTRD, "XFS_LI_ATTRD" }
/*
* Inode Log Item Format definitions.
@@ -863,4 +871,35 @@ struct xfs_icreate_log {
__be32 icl_gen; /* inode generation number to use */
};
+/*
+ * Flags for deferred attribute operations.
+ * Upper bits are flags, lower byte is type code
+ */
+#define XFS_ATTR_OP_FLAGS_SET 1 /* Set the attribute */
+#define XFS_ATTR_OP_FLAGS_REMOVE 2 /* Remove the attribute */
+#define XFS_ATTR_OP_FLAGS_TYPE_MASK 0xFF /* Flags type mask */
+
+/*
+ * This is the structure used to lay out an attr log item in the
+ * log.
+ */
+struct xfs_attri_log_format {
+ uint16_t alfi_type; /* attri log item type */
+ uint16_t alfi_size; /* size of this item */
+ uint32_t __pad; /* pad to 64 bit aligned */
+ uint64_t alfi_id; /* attri identifier */
+ uint64_t alfi_ino; /* the inode for this attr operation */
+ uint32_t alfi_op_flags; /* marks the op as a set or remove */
+ uint32_t alfi_name_len; /* attr name length */
+ uint32_t alfi_value_len; /* attr value length */
+ uint32_t alfi_attr_flags;/* attr flags */
+};
+
+struct xfs_attrd_log_format {
+ uint16_t alfd_type; /* attrd log item type */
+ uint16_t alfd_size; /* size of this item */
+ uint32_t __pad; /* pad to 64 bit aligned */
+ uint64_t alfd_alf_id; /* id of corresponding attri */
+};
+
#endif /* __XFS_LOG_FORMAT_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 20/27] xfsprogs: Implement attr logging and replay
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (18 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 19/27] xfsprogs: Set up infrastructure for deferred attribute operations Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 21/27] RFC xfsprogs: Skip flip flags for delayed attrs Allison Henderson
` (6 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
This patch adds the needed routines to create, log and recover logged
extended attribute intents.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
libxfs/defer_item.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++
libxfs/xfs_defer.c | 1 +
libxfs/xfs_defer.h | 1 +
libxfs/xfs_format.h | 10 +++-
4 files changed, 148 insertions(+), 1 deletion(-)
diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
index a1f0d7e..f144ed6 100644
--- a/libxfs/defer_item.c
+++ b/libxfs/defer_item.c
@@ -116,6 +116,143 @@ const struct xfs_defer_op_type xfs_extent_free_defer_type = {
};
/*
+ * Performs one step of an attribute update intent and marks the attrd item
+ * dirty.. An attr operation may be a set or a remove. Note that the
+ * transaction is marked dirty regardless of whether the operation succeeds or
+ * fails to support the ATTRI/ATTRD lifecycle rules.
+ */
+STATIC int
+xfs_trans_attr_finish_update(
+ struct xfs_delattr_context *dac,
+ struct xfs_buf **leaf_bp,
+ uint32_t op_flags)
+{
+ struct xfs_da_args *args = dac->da_args;
+ unsigned int op = op_flags &
+ XFS_ATTR_OP_FLAGS_TYPE_MASK;
+ int error;
+
+ error = xfs_qm_dqattach_locked(args->dp, 0);
+ if (error)
+ return error;
+
+ switch (op) {
+ case XFS_ATTR_OP_FLAGS_SET:
+ args->op_flags |= XFS_DA_OP_ADDNAME;
+ error = xfs_attr_set_iter(dac, leaf_bp);
+ break;
+ case XFS_ATTR_OP_FLAGS_REMOVE:
+ ASSERT(XFS_IFORK_Q(args->dp));
+ error = xfs_attr_remove_iter(dac);
+ break;
+ default:
+ error = -EFSCORRUPTED;
+ break;
+ }
+
+ /*
+ * Mark the transaction dirty, even on error. This ensures the
+ * transaction is aborted, which:
+ *
+ * 1.) releases the ATTRI and frees the ATTRD
+ * 2.) shuts down the filesystem
+ */
+ args->trans->t_flags |= XFS_TRANS_DIRTY;
+
+ return error;
+}
+
+/* Get an ATTRI. */
+static struct xfs_log_item *
+xfs_attr_create_intent(
+ struct xfs_trans *tp,
+ struct list_head *items,
+ unsigned int count,
+ bool sort)
+{
+ return NULL;
+}
+
+/* Abort all pending ATTRs. */
+STATIC void
+xfs_attr_abort_intent(
+ struct xfs_log_item *intent)
+{
+}
+
+/* Get an ATTRD so we can process all the attrs. */
+static struct xfs_log_item *
+xfs_attr_create_done(
+ struct xfs_trans *tp,
+ struct xfs_log_item *intent,
+ unsigned int count)
+{
+ return NULL;
+}
+
+/* Process an attr. */
+STATIC int
+xfs_attr_finish_item(
+ struct xfs_trans *tp,
+ struct xfs_log_item *done,
+ struct list_head *item,
+ struct xfs_btree_cur **state)
+{
+ struct xfs_attr_item *attr;
+ int error;
+ struct xfs_delattr_context *dac;
+
+ attr = container_of(item, struct xfs_attr_item, xattri_list);
+ dac = &attr->xattri_dac;
+ /*
+ * Corner case that can happen during a recovery. Because the first
+ * iteration of a multi part delay op happens in xfs_attri_item_recover
+ * to maintain the order of the log replay items. But the new
+ * transactions do not automatically rejoin during a recovery as they do
+ * in a standard delay op, so we need to catch this here and rejoin the
+ * leaf to the new transaction
+ */
+ if (attr->xattri_dac.leaf_bp &&
+ attr->xattri_dac.leaf_bp->b_transp != tp) {
+ xfs_trans_bjoin(tp, attr->xattri_dac.leaf_bp);
+ xfs_trans_bhold(tp, attr->xattri_dac.leaf_bp);
+ }
+
+ /*
+ * Always reset trans after EAGAIN cycle
+ * since the transaction is new
+ */
+ dac->da_args->trans = tp;
+
+ error = xfs_trans_attr_finish_update(dac, &dac->leaf_bp,
+ attr->xattri_op_flags);
+ if (error != -EAGAIN)
+ kmem_free(attr);
+
+ return error;
+}
+
+/* Cancel an attr */
+STATIC void
+xfs_attr_cancel_item(
+ struct list_head *item)
+{
+ struct xfs_attr_item *attr;
+
+ attr = container_of(item, struct xfs_attr_item, xattri_list);
+ kmem_free(attr);
+}
+
+const struct xfs_defer_op_type xfs_attr_defer_type = {
+ .max_items = 1,
+ .create_intent = xfs_attr_create_intent,
+ .abort_intent = xfs_attr_abort_intent,
+ .create_done = xfs_attr_create_done,
+ .finish_item = xfs_attr_finish_item,
+ .cancel_item = xfs_attr_cancel_item,
+};
+
+/*
* AGFL blocks are accounted differently in the reserve pools and are not
* inserted into the busy extent list.
*/
diff --git a/libxfs/xfs_defer.c b/libxfs/xfs_defer.c
index 1fdf6c7..06df7ea 100644
--- a/libxfs/xfs_defer.c
+++ b/libxfs/xfs_defer.c
@@ -174,6 +174,7 @@ static const struct xfs_defer_op_type *defer_op_types[] = {
[XFS_DEFER_OPS_TYPE_RMAP] = &xfs_rmap_update_defer_type,
[XFS_DEFER_OPS_TYPE_FREE] = &xfs_extent_free_defer_type,
[XFS_DEFER_OPS_TYPE_AGFL_FREE] = &xfs_agfl_free_defer_type,
+ [XFS_DEFER_OPS_TYPE_ATTR] = &xfs_attr_defer_type,
};
static void
diff --git a/libxfs/xfs_defer.h b/libxfs/xfs_defer.h
index 7566f61..58cf4e2 100644
--- a/libxfs/xfs_defer.h
+++ b/libxfs/xfs_defer.h
@@ -19,6 +19,7 @@ enum xfs_defer_ops_type {
XFS_DEFER_OPS_TYPE_RMAP,
XFS_DEFER_OPS_TYPE_FREE,
XFS_DEFER_OPS_TYPE_AGFL_FREE,
+ XFS_DEFER_OPS_TYPE_ATTR,
XFS_DEFER_OPS_TYPE_MAX,
};
diff --git a/libxfs/xfs_format.h b/libxfs/xfs_format.h
index 5d8a129..f062766 100644
--- a/libxfs/xfs_format.h
+++ b/libxfs/xfs_format.h
@@ -485,7 +485,9 @@ xfs_sb_has_incompat_feature(
return (sbp->sb_features_incompat & feature) != 0;
}
-#define XFS_SB_FEAT_INCOMPAT_LOG_ALL 0
+#define XFS_SB_FEAT_INCOMPAT_LOG_DELATTR (1 << 0) /* Delayed Attributes */
+#define XFS_SB_FEAT_INCOMPAT_LOG_ALL \
+ (XFS_SB_FEAT_INCOMPAT_LOG_DELATTR)
#define XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN ~XFS_SB_FEAT_INCOMPAT_LOG_ALL
static inline bool
xfs_sb_has_incompat_log_feature(
@@ -590,6 +592,12 @@ static inline bool xfs_sb_version_hasbigtime(struct xfs_sb *sbp)
(sbp->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_BIGTIME);
}
+static inline bool xfs_sb_version_hasdelattr(struct xfs_sb *sbp)
+{
+ return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+ (sbp->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_LOG_DELATTR);
+}
+
/*
* Inode btree block counter. We record the number of inobt and finobt blocks
* in the AGI header so that we can skip the finobt walk at mount time when
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 21/27] RFC xfsprogs: Skip flip flags for delayed attrs
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (19 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 20/27] xfsprogs: Implement attr logging and replay Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:18 ` [PATCH v22 22/27] xfsprogs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred Allison Henderson
` (5 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 2b1c81a8c3f453ba16b6db8dae256723bf53c051
This is a clean up patch that skips the flip flag logic for delayed attr
renames. Since the log replay keeps the inode locked, we do not need to
worry about race windows with attr lookups. So we can skip over
flipping the flag and the extra transaction roll for it
RFC: In the last review, folks asked for some performance analysis, so I
did a few perf captures with and with out this patch. What I found was
that there wasnt very much difference at all between having the patch or
not having it. Of the time we do spend in the affected code, the
percentage is small. Most of the time we spend about %0.03 of the time
in this function, with or with out the patch. Occasionally we get a
0.02%, though not often. So I think this starts to challenge needing
this patch at all. This patch was requested some number of reviews ago,
be perhaps in light of the findings, it may no longer be of interest.
0.03% 0.00% fsstress [xfs] [k] xfs_attr_set_iter
Keep it or drop it?
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
libxfs/xfs_attr.c | 51 +++++++++++++++++++++++++++++---------------------
libxfs/xfs_attr_leaf.c | 3 ++-
2 files changed, 32 insertions(+), 22 deletions(-)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 70665d9..9967719 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -354,6 +354,7 @@ 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 */
switch (dac->dela_state) {
@@ -475,16 +476,21 @@ xfs_attr_set_iter(
* In a separate transaction, set the incomplete flag on the
* "old" attr and clear the incomplete flag on the "new" attr.
*/
- error = xfs_attr3_leaf_flipflags(args);
- if (error)
- return error;
- /*
- * Commit the flag value change and start the next trans in
- * series.
- */
- dac->dela_state = XFS_DAS_FLIP_LFLAG;
- trace_xfs_attr_set_iter_return(dac->dela_state, args->dp);
- return -EAGAIN;
+ if (!xfs_hasdelattr(mp)) {
+ error = xfs_attr3_leaf_flipflags(args);
+ if (error)
+ return error;
+ /*
+ * Commit the flag value change and start the next trans
+ * in series.
+ */
+ dac->dela_state = XFS_DAS_FLIP_LFLAG;
+ trace_xfs_attr_set_iter_return(dac->dela_state,
+ args->dp);
+ return -EAGAIN;
+ }
+
+ /* fallthrough */
case XFS_DAS_FLIP_LFLAG:
/*
* Dismantle the "old" attribute/value pair by removing a
@@ -586,17 +592,21 @@ xfs_attr_set_iter(
* In a separate transaction, set the incomplete flag on the
* "old" attr and clear the incomplete flag on the "new" attr.
*/
- error = xfs_attr3_leaf_flipflags(args);
- if (error)
- goto out;
- /*
- * Commit the flag value change and start the next trans in
- * series
- */
- dac->dela_state = XFS_DAS_FLIP_NFLAG;
- trace_xfs_attr_set_iter_return(dac->dela_state, args->dp);
- return -EAGAIN;
+ if (!xfs_hasdelattr(mp)) {
+ error = xfs_attr3_leaf_flipflags(args);
+ if (error)
+ goto out;
+ /*
+ * Commit the flag value change and start the next trans
+ * in series
+ */
+ dac->dela_state = XFS_DAS_FLIP_NFLAG;
+ trace_xfs_attr_set_iter_return(dac->dela_state,
+ args->dp);
+ return -EAGAIN;
+ }
+ /* fallthrough */
case XFS_DAS_FLIP_NFLAG:
/*
* Dismantle the "old" attribute/value pair by removing a
@@ -1240,7 +1250,6 @@ xfs_attr_node_addname_clear_incomplete(
* Re-find the "old" attribute entry after any split ops. The INCOMPLETE
* flag means that we will find the "old" attr, not the "new" one.
*/
- args->attr_filter |= XFS_ATTR_INCOMPLETE;
state = xfs_da_state_alloc(args);
state->inleaf = 0;
error = xfs_da3_node_lookup_int(state, &retval);
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index e23fc3d..056f4eb 100644
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -1478,7 +1478,8 @@ xfs_attr3_leaf_add_work(
if (tmp)
entry->flags |= XFS_ATTR_LOCAL;
if (args->op_flags & XFS_DA_OP_RENAME) {
- entry->flags |= XFS_ATTR_INCOMPLETE;
+ if (!xfs_hasdelattr(mp))
+ entry->flags |= XFS_ATTR_INCOMPLETE;
if ((args->blkno2 == args->blkno) &&
(args->index2 <= args->index)) {
args->index2++;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 22/27] xfsprogs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (20 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 21/27] RFC xfsprogs: Skip flip flags for delayed attrs Allison Henderson
@ 2021-07-27 6:18 ` Allison Henderson
2021-07-27 6:19 ` [PATCH v22 23/27] xfsprogs: Remove unused xfs_attr_*_args Allison Henderson
` (4 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:18 UTC (permalink / raw)
To: linux-xfs
From: Allison Collins <allison.henderson@oracle.com>
These routines set up and queue a new deferred attribute operations.
These functions are meant to be called by any routine needing to
initiate a deferred attribute operation as opposed to the existing
inline operations. New helper function xfs_attr_item_init also added.
Finally enable delayed attributes in xfs_attr_set and xfs_attr_remove.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
libxfs/libxfs_priv.h | 4 ++++
libxfs/xfs_attr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++---
libxfs/xfs_attr.h | 2 ++
3 files changed, 71 insertions(+), 3 deletions(-)
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 0272ef2..0d4ec11 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -614,9 +614,13 @@ typedef int (*xfs_rtalloc_query_range_fn)(
int libxfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
xfs_off_t count_fsb);
+/* xfs_log.c */
+struct xlog;
bool xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
void xfs_log_item_init(struct xfs_mount *, struct xfs_log_item *, int);
+int xfs_attr_use_log_assist(struct xfs_mount *mp);
+void xlog_drop_incompat_feat(struct xlog *log);
#define xfs_log_in_recovery(mp) (false)
/* xfs_icache.c */
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 9967719..260ae8f 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -778,13 +778,19 @@ xfs_attr_set(
rmt_blks = xfs_attr3_rmt_blocks(mp, XFS_XATTR_SIZE_MAX);
}
+ if (xfs_hasdelattr(mp)) {
+ error = xfs_attr_use_log_assist(mp);
+ if (error)
+ return error;
+ }
+
/*
* Root fork attributes can use reserved data blocks for this
* operation if necessary
*/
error = xfs_trans_alloc_inode(dp, &tres, total, 0, rsvd, &args->trans);
if (error)
- return error;
+ goto drop_incompat;
if (args->value || xfs_inode_hasattr(dp)) {
error = xfs_iext_count_may_overflow(dp, XFS_ATTR_FORK,
@@ -802,9 +808,10 @@ xfs_attr_set(
if (error != -ENOATTR && error != -EEXIST)
goto out_trans_cancel;
- error = xfs_attr_set_args(args);
+ error = xfs_attr_set_deferred(args);
if (error)
goto out_trans_cancel;
+
/* shortform attribute has already been committed */
if (!args->trans)
goto out_unlock;
@@ -813,7 +820,7 @@ xfs_attr_set(
if (error != -EEXIST)
goto out_trans_cancel;
- error = xfs_attr_remove_args(args);
+ error = xfs_attr_remove_deferred(args);
if (error)
goto out_trans_cancel;
}
@@ -835,6 +842,9 @@ xfs_attr_set(
error = xfs_trans_commit(args->trans);
out_unlock:
xfs_iunlock(dp, XFS_ILOCK_EXCL);
+drop_incompat:
+ if (xfs_hasdelattr(mp))
+ xlog_drop_incompat_feat(mp->m_log);
return error;
out_trans_cancel:
@@ -843,6 +853,58 @@ out_trans_cancel:
goto out_unlock;
}
+STATIC int
+xfs_attr_item_init(
+ struct xfs_da_args *args,
+ unsigned int op_flags, /* op flag (set or remove) */
+ struct xfs_attr_item **attr) /* new xfs_attr_item */
+{
+
+ struct xfs_attr_item *new;
+
+ new = kmem_zalloc(sizeof(struct xfs_attr_item), KM_NOFS);
+ new->xattri_op_flags = op_flags;
+ new->xattri_dac.da_args = args;
+
+ *attr = new;
+ return 0;
+}
+
+/* Sets an attribute for an inode as a deferred operation */
+int
+xfs_attr_set_deferred(
+ struct xfs_da_args *args)
+{
+ struct xfs_attr_item *new;
+ int error = 0;
+
+ error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_SET, &new);
+ if (error)
+ return error;
+
+ xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new->xattri_list);
+
+ return 0;
+}
+
+/* Removes an attribute for an inode as a deferred operation */
+int
+xfs_attr_remove_deferred(
+ struct xfs_da_args *args)
+{
+
+ struct xfs_attr_item *new;
+ int error;
+
+ error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_REMOVE, &new);
+ if (error)
+ return error;
+
+ xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new->xattri_list);
+
+ return 0;
+}
+
/*========================================================================
* External routines when attribute list is inside the inode
*========================================================================*/
diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h
index 463b2be..72b0ea5 100644
--- a/libxfs/xfs_attr.h
+++ b/libxfs/xfs_attr.h
@@ -527,5 +527,7 @@ bool xfs_attr_namecheck(const void *name, size_t length);
void xfs_delattr_context_init(struct xfs_delattr_context *dac,
struct xfs_da_args *args);
int xfs_attr_calc_size(struct xfs_da_args *args, int *local);
+int xfs_attr_set_deferred(struct xfs_da_args *args);
+int xfs_attr_remove_deferred(struct xfs_da_args *args);
#endif /* __XFS_ATTR_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 23/27] xfsprogs: Remove unused xfs_attr_*_args
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (21 preceding siblings ...)
2021-07-27 6:18 ` [PATCH v22 22/27] xfsprogs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred Allison Henderson
@ 2021-07-27 6:19 ` Allison Henderson
2021-07-27 6:19 ` [PATCH v22 24/27] xfsprogs: Add delayed attributes error tag Allison Henderson
` (3 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:19 UTC (permalink / raw)
To: linux-xfs
Remove xfs_attr_set_args, xfs_attr_remove_args, and xfs_attr_trans_roll.
These high level loops are now driven by the delayed operations code,
and can be removed.
Additionally collapse in the leaf_bp parameter of xfs_attr_set_iter
since we only have one caller that passes dac->leaf_bp
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
libxfs/defer_item.c | 6 +--
libxfs/xfs_attr.c | 96 +++---------------------------------------------
libxfs/xfs_attr.h | 10 ++---
libxfs/xfs_attr_remote.c | 1 -
4 files changed, 10 insertions(+), 103 deletions(-)
diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
index f144ed6..41cf921 100644
--- a/libxfs/defer_item.c
+++ b/libxfs/defer_item.c
@@ -124,7 +124,6 @@ const struct xfs_defer_op_type xfs_extent_free_defer_type = {
STATIC int
xfs_trans_attr_finish_update(
struct xfs_delattr_context *dac,
- struct xfs_buf **leaf_bp,
uint32_t op_flags)
{
struct xfs_da_args *args = dac->da_args;
@@ -139,7 +138,7 @@ xfs_trans_attr_finish_update(
switch (op) {
case XFS_ATTR_OP_FLAGS_SET:
args->op_flags |= XFS_DA_OP_ADDNAME;
- error = xfs_attr_set_iter(dac, leaf_bp);
+ error = xfs_attr_set_iter(dac);
break;
case XFS_ATTR_OP_FLAGS_REMOVE:
ASSERT(XFS_IFORK_Q(args->dp));
@@ -224,8 +223,7 @@ xfs_attr_finish_item(
*/
dac->da_args->trans = tp;
- error = xfs_trans_attr_finish_update(dac, &dac->leaf_bp,
- attr->xattri_op_flags);
+ error = xfs_trans_attr_finish_update(dac, attr->xattri_op_flags);
if (error != -EAGAIN)
kmem_free(attr);
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 260ae8f..36ef8e7 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -241,67 +241,13 @@ xfs_attr_is_shortform(
ip->i_afp->if_nextents == 0);
}
-/*
- * Checks to see if a delayed attribute transaction should be rolled. If so,
- * transaction is finished or rolled as needed.
- */
-STATIC int
-xfs_attr_trans_roll(
- struct xfs_delattr_context *dac)
-{
- struct xfs_da_args *args = dac->da_args;
- int error;
-
- if (dac->flags & XFS_DAC_DEFER_FINISH) {
- /*
- * The caller wants us to finish all the deferred ops so that we
- * avoid pinning the log tail with a large number of deferred
- * ops.
- */
- dac->flags &= ~XFS_DAC_DEFER_FINISH;
- error = xfs_defer_finish(&args->trans);
- } else
- error = xfs_trans_roll_inode(&args->trans, args->dp);
-
- return error;
-}
-
-/*
- * Set the attribute specified in @args.
- */
-int
-xfs_attr_set_args(
- struct xfs_da_args *args)
-{
- struct xfs_buf *leaf_bp = NULL;
- int error = 0;
- struct xfs_delattr_context dac = {
- .da_args = args,
- };
-
- do {
- error = xfs_attr_set_iter(&dac, &leaf_bp);
- if (error != -EAGAIN)
- break;
-
- error = xfs_attr_trans_roll(&dac);
- if (error) {
- if (leaf_bp)
- xfs_trans_brelse(args->trans, leaf_bp);
- return error;
- }
- } while (true);
-
- return error;
-}
-
STATIC int
xfs_attr_sf_addname(
- struct xfs_delattr_context *dac,
- struct xfs_buf **leaf_bp)
+ struct xfs_delattr_context *dac)
{
struct xfs_da_args *args = dac->da_args;
struct xfs_inode *dp = args->dp;
+ struct xfs_buf **leaf_bp = &dac->leaf_bp;
int error = 0;
/*
@@ -334,7 +280,6 @@ xfs_attr_sf_addname(
* add.
*/
trace_xfs_attr_sf_addname_return(XFS_DAS_UNINIT, args->dp);
- dac->flags |= XFS_DAC_DEFER_FINISH;
return -EAGAIN;
}
@@ -347,10 +292,10 @@ xfs_attr_sf_addname(
*/
int
xfs_attr_set_iter(
- struct xfs_delattr_context *dac,
- struct xfs_buf **leaf_bp)
+ struct xfs_delattr_context *dac)
{
struct xfs_da_args *args = dac->da_args;
+ struct xfs_buf **leaf_bp = &dac->leaf_bp;
struct xfs_inode *dp = args->dp;
struct xfs_buf *bp = NULL;
int forkoff, error = 0;
@@ -367,7 +312,7 @@ xfs_attr_set_iter(
* release the hold once we return with a clean transaction.
*/
if (xfs_attr_is_shortform(dp))
- return xfs_attr_sf_addname(dac, leaf_bp);
+ return xfs_attr_sf_addname(dac);
if (*leaf_bp != NULL) {
xfs_trans_bhold_release(args->trans, *leaf_bp);
*leaf_bp = NULL;
@@ -393,7 +338,6 @@ xfs_attr_set_iter(
* be a node, so we'll fall down into the node
* handling code below
*/
- dac->flags |= XFS_DAC_DEFER_FINISH;
trace_xfs_attr_set_iter_return(
dac->dela_state, args->dp);
return -EAGAIN;
@@ -682,32 +626,6 @@ xfs_has_attr(
}
/*
- * Remove the attribute specified in @args.
- */
-int
-xfs_attr_remove_args(
- struct xfs_da_args *args)
-{
- int error;
- struct xfs_delattr_context dac = {
- .da_args = args,
- };
-
- do {
- error = xfs_attr_remove_iter(&dac);
- if (error != -EAGAIN)
- break;
-
- error = xfs_attr_trans_roll(&dac);
- if (error)
- return error;
-
- } while (true);
-
- return error;
-}
-
-/*
* Note: If args->value is NULL the attribute will be removed, just like the
* Linux ->setattr API.
*/
@@ -1269,7 +1187,6 @@ xfs_attr_node_addname(
* this. dela_state is still unset by this function at
* this point.
*/
- dac->flags |= XFS_DAC_DEFER_FINISH;
trace_xfs_attr_node_addname_return(
dac->dela_state, args->dp);
return -EAGAIN;
@@ -1284,7 +1201,6 @@ xfs_attr_node_addname(
error = xfs_da3_split(state);
if (error)
goto out;
- dac->flags |= XFS_DAC_DEFER_FINISH;
} else {
/*
* Addition succeeded, update Btree hashvals.
@@ -1535,7 +1451,6 @@ xfs_attr_remove_iter(
if (error)
goto out;
dac->dela_state = XFS_DAS_RM_NAME;
- dac->flags |= XFS_DAC_DEFER_FINISH;
return -EAGAIN;
}
@@ -1562,7 +1477,6 @@ xfs_attr_remove_iter(
if (error)
goto out;
- dac->flags |= XFS_DAC_DEFER_FINISH;
dac->dela_state = XFS_DAS_RM_SHRINK;
trace_xfs_attr_remove_iter_return(
dac->dela_state, args->dp);
diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h
index 72b0ea5..c0c92bd3 100644
--- a/libxfs/xfs_attr.h
+++ b/libxfs/xfs_attr.h
@@ -457,9 +457,8 @@ enum xfs_delattr_state {
/*
* Defines for xfs_delattr_context.flags
*/
-#define XFS_DAC_DEFER_FINISH 0x01 /* finish the transaction */
-#define XFS_DAC_LEAF_ADDNAME_INIT 0x02 /* xfs_attr_leaf_addname init*/
-#define XFS_DAC_DELAYED_OP_INIT 0x04 /* delayed operations init*/
+#define XFS_DAC_LEAF_ADDNAME_INIT 0x01 /* xfs_attr_leaf_addname init*/
+#define XFS_DAC_DELAYED_OP_INIT 0x02 /* delayed operations init*/
/*
* Context used for keeping track of delayed attribute operations
@@ -517,11 +516,8 @@ bool xfs_attr_is_leaf(struct xfs_inode *ip);
int xfs_attr_get_ilocked(struct xfs_da_args *args);
int xfs_attr_get(struct xfs_da_args *args);
int xfs_attr_set(struct xfs_da_args *args);
-int xfs_attr_set_args(struct xfs_da_args *args);
-int xfs_attr_set_iter(struct xfs_delattr_context *dac,
- struct xfs_buf **leaf_bp);
+int xfs_attr_set_iter(struct xfs_delattr_context *dac);
int xfs_has_attr(struct xfs_da_args *args);
-int xfs_attr_remove_args(struct xfs_da_args *args);
int xfs_attr_remove_iter(struct xfs_delattr_context *dac);
bool xfs_attr_namecheck(const void *name, size_t length);
void xfs_delattr_context_init(struct xfs_delattr_context *dac,
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index b781e44..42943b3 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -694,7 +694,6 @@ xfs_attr_rmtval_remove(
* the parent
*/
if (!done) {
- dac->flags |= XFS_DAC_DEFER_FINISH;
trace_xfs_attr_rmtval_remove_return(dac->dela_state, args->dp);
return -EAGAIN;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 24/27] xfsprogs: Add delayed attributes error tag
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (22 preceding siblings ...)
2021-07-27 6:19 ` [PATCH v22 23/27] xfsprogs: Remove unused xfs_attr_*_args Allison Henderson
@ 2021-07-27 6:19 ` Allison Henderson
2021-07-27 6:19 ` [PATCH v22 25/27] xfsprogs: Merge xfs_delattr_context into xfs_attr_item Allison Henderson
` (2 subsequent siblings)
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:19 UTC (permalink / raw)
To: linux-xfs
This patch adds an error tag that we can use to test delayed attribute
recovery and replay
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
io/inject.c | 1 +
libxfs/defer_item.c | 6 ++++++
libxfs/xfs_errortag.h | 4 +++-
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/io/inject.c b/io/inject.c
index b8b0977..53286a3 100644
--- a/io/inject.c
+++ b/io/inject.c
@@ -58,6 +58,7 @@ error_tag(char *name)
{ XFS_ERRTAG_REDUCE_MAX_IEXTENTS, "reduce_max_iextents" },
{ XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT, "bmap_alloc_minlen_extent" },
{ XFS_ERRTAG_AG_RESV_FAIL, "ag_resv_fail" },
+ { XFS_ERRTAG_DELAYED_ATTR, "delayed_attr" },
{ XFS_ERRTAG_MAX, NULL }
};
int count;
diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
index 41cf921..6db4380 100644
--- a/libxfs/defer_item.c
+++ b/libxfs/defer_item.c
@@ -135,6 +135,11 @@ xfs_trans_attr_finish_update(
if (error)
return error;
+ if (XFS_TEST_ERROR(false, args->dp->i_mount, XFS_ERRTAG_DELAYED_ATTR)) {
+ error = -EIO;
+ goto out;
+ }
+
switch (op) {
case XFS_ATTR_OP_FLAGS_SET:
args->op_flags |= XFS_DA_OP_ADDNAME;
@@ -149,6 +154,7 @@ xfs_trans_attr_finish_update(
break;
}
+out:
/*
* Mark the transaction dirty, even on error. This ensures the
* transaction is aborted, which:
diff --git a/libxfs/xfs_errortag.h b/libxfs/xfs_errortag.h
index a23a52e..46f359c 100644
--- a/libxfs/xfs_errortag.h
+++ b/libxfs/xfs_errortag.h
@@ -59,7 +59,8 @@
#define XFS_ERRTAG_REDUCE_MAX_IEXTENTS 36
#define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT 37
#define XFS_ERRTAG_AG_RESV_FAIL 38
-#define XFS_ERRTAG_MAX 39
+#define XFS_ERRTAG_DELAYED_ATTR 39
+#define XFS_ERRTAG_MAX 40
/*
* Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -103,5 +104,6 @@
#define XFS_RANDOM_REDUCE_MAX_IEXTENTS 1
#define XFS_RANDOM_BMAP_ALLOC_MINLEN_EXTENT 1
#define XFS_RANDOM_AG_RESV_FAIL 1
+#define XFS_RANDOM_DELAYED_ATTR 1
#endif /* __XFS_ERRORTAG_H_ */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 25/27] xfsprogs: Merge xfs_delattr_context into xfs_attr_item
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (23 preceding siblings ...)
2021-07-27 6:19 ` [PATCH v22 24/27] xfsprogs: Add delayed attributes error tag Allison Henderson
@ 2021-07-27 6:19 ` Allison Henderson
2021-07-27 6:19 ` [PATCH v22 26/27] xfsprogs: Add helper function xfs_attr_leaf_addname Allison Henderson
2021-07-27 6:19 ` [PATCH v22 27/27] xfsprogs: Add log item printing for ATTRI and ATTRD Allison Henderson
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:19 UTC (permalink / raw)
To: linux-xfs
This is a clean up patch that merges xfs_delattr_context into
xfs_attr_item. Now that the refactoring is complete and the delayed
operation infrastructure is in place, we can combine these to eliminate
the extra struct
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
libxfs/defer_item.c | 21 +++----
libxfs/xfs_attr.c | 142 +++++++++++++++++++++++------------------------
libxfs/xfs_attr.h | 40 ++++++-------
libxfs/xfs_attr_remote.c | 36 ++++++------
libxfs/xfs_attr_remote.h | 6 +-
5 files changed, 119 insertions(+), 126 deletions(-)
diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
index 6db4380..a595e8c 100644
--- a/libxfs/defer_item.c
+++ b/libxfs/defer_item.c
@@ -123,10 +123,10 @@ const struct xfs_defer_op_type xfs_extent_free_defer_type = {
*/
STATIC int
xfs_trans_attr_finish_update(
- struct xfs_delattr_context *dac,
+ struct xfs_attr_item *attr,
uint32_t op_flags)
{
- struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_args *args = attr->xattri_da_args;
unsigned int op = op_flags &
XFS_ATTR_OP_FLAGS_TYPE_MASK;
int error;
@@ -143,11 +143,11 @@ xfs_trans_attr_finish_update(
switch (op) {
case XFS_ATTR_OP_FLAGS_SET:
args->op_flags |= XFS_DA_OP_ADDNAME;
- error = xfs_attr_set_iter(dac);
+ error = xfs_attr_set_iter(attr);
break;
case XFS_ATTR_OP_FLAGS_REMOVE:
ASSERT(XFS_IFORK_Q(args->dp));
- error = xfs_attr_remove_iter(dac);
+ error = xfs_attr_remove_iter(attr);
break;
default:
error = -EFSCORRUPTED;
@@ -205,10 +205,8 @@ xfs_attr_finish_item(
{
struct xfs_attr_item *attr;
int error;
- struct xfs_delattr_context *dac;
attr = container_of(item, struct xfs_attr_item, xattri_list);
- dac = &attr->xattri_dac;
/*
* Corner case that can happen during a recovery. Because the first
* iteration of a multi part delay op happens in xfs_attri_item_recover
@@ -217,19 +215,18 @@ xfs_attr_finish_item(
* in a standard delay op, so we need to catch this here and rejoin the
* leaf to the new transaction
*/
- if (attr->xattri_dac.leaf_bp &&
- attr->xattri_dac.leaf_bp->b_transp != tp) {
- xfs_trans_bjoin(tp, attr->xattri_dac.leaf_bp);
- xfs_trans_bhold(tp, attr->xattri_dac.leaf_bp);
+ if (attr->xattri_leaf_bp && attr->xattri_leaf_bp->b_transp != tp) {
+ xfs_trans_bjoin(tp, attr->xattri_leaf_bp);
+ xfs_trans_bhold(tp, attr->xattri_leaf_bp);
}
/*
* Always reset trans after EAGAIN cycle
* since the transaction is new
*/
- dac->da_args->trans = tp;
+ attr->xattri_da_args->trans = tp;
- error = xfs_trans_attr_finish_update(dac, attr->xattri_op_flags);
+ error = xfs_trans_attr_finish_update(attr, attr->xattri_op_flags);
if (error != -EAGAIN)
kmem_free(attr);
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 36ef8e7..f5d4380 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -53,10 +53,9 @@ STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args, struct xfs_buf *bp);
*/
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_addname(struct xfs_delattr_context *dac);
-STATIC int xfs_attr_node_addname_find_attr(struct xfs_delattr_context *dac);
-STATIC int xfs_attr_node_addname_clear_incomplete(
- struct xfs_delattr_context *dac);
+STATIC int xfs_attr_node_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_hasname(xfs_da_args_t *args,
struct xfs_da_state **state);
STATIC int xfs_attr_fillstate(xfs_da_state_t *state);
@@ -243,11 +242,11 @@ xfs_attr_is_shortform(
STATIC int
xfs_attr_sf_addname(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_args *args = attr->xattri_da_args;
struct xfs_inode *dp = args->dp;
- struct xfs_buf **leaf_bp = &dac->leaf_bp;
+ struct xfs_buf **leaf_bp = &attr->xattri_leaf_bp;
int error = 0;
/*
@@ -292,17 +291,17 @@ xfs_attr_sf_addname(
*/
int
xfs_attr_set_iter(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
- struct xfs_buf **leaf_bp = &dac->leaf_bp;
+ struct xfs_da_args *args = attr->xattri_da_args;
+ struct xfs_buf **leaf_bp = &attr->xattri_leaf_bp;
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 */
- switch (dac->dela_state) {
+ switch (attr->xattri_dela_state) {
case XFS_DAS_UNINIT:
/*
* If the fork is shortform, attempt to add the attr. If there
@@ -312,7 +311,7 @@ xfs_attr_set_iter(
* release the hold once we return with a clean transaction.
*/
if (xfs_attr_is_shortform(dp))
- return xfs_attr_sf_addname(dac);
+ return xfs_attr_sf_addname(attr);
if (*leaf_bp != NULL) {
xfs_trans_bhold_release(args->trans, *leaf_bp);
*leaf_bp = NULL;
@@ -339,19 +338,19 @@ xfs_attr_set_iter(
* handling code below
*/
trace_xfs_attr_set_iter_return(
- dac->dela_state, args->dp);
+ attr->xattri_dela_state, args->dp);
return -EAGAIN;
} else if (error) {
return error;
}
- dac->dela_state = XFS_DAS_FOUND_LBLK;
+ attr->xattri_dela_state = XFS_DAS_FOUND_LBLK;
} else {
- error = xfs_attr_node_addname_find_attr(dac);
+ error = xfs_attr_node_addname_find_attr(attr);
if (error)
return error;
- error = xfs_attr_node_addname(dac);
+ error = xfs_attr_node_addname(attr);
if (error)
return error;
@@ -362,9 +361,10 @@ xfs_attr_set_iter(
if (!args->rmtblkno && !args->rmtblkno2)
return 0;
- dac->dela_state = XFS_DAS_FOUND_NBLK;
+ attr->xattri_dela_state = XFS_DAS_FOUND_NBLK;
}
- trace_xfs_attr_set_iter_return(dac->dela_state, args->dp);
+ trace_xfs_attr_set_iter_return(attr->xattri_dela_state,
+ args->dp);
return -EAGAIN;
case XFS_DAS_FOUND_LBLK:
/*
@@ -375,10 +375,10 @@ xfs_attr_set_iter(
*/
/* Open coded xfs_attr_rmtval_set without trans handling */
- if ((dac->flags & XFS_DAC_LEAF_ADDNAME_INIT) == 0) {
- dac->flags |= XFS_DAC_LEAF_ADDNAME_INIT;
+ if ((attr->xattri_flags & XFS_DAC_LEAF_ADDNAME_INIT) == 0) {
+ attr->xattri_flags |= XFS_DAC_LEAF_ADDNAME_INIT;
if (args->rmtblkno > 0) {
- error = xfs_attr_rmtval_find_space(dac);
+ error = xfs_attr_rmtval_find_space(attr);
if (error)
return error;
}
@@ -388,11 +388,11 @@ xfs_attr_set_iter(
* Repeat allocating remote blocks for the attr value until
* blkcnt drops to zero.
*/
- if (dac->blkcnt > 0) {
- error = xfs_attr_rmtval_set_blk(dac);
+ if (attr->xattri_blkcnt > 0) {
+ error = xfs_attr_rmtval_set_blk(attr);
if (error)
return error;
- trace_xfs_attr_set_iter_return(dac->dela_state,
+ trace_xfs_attr_set_iter_return(attr->xattri_dela_state,
args->dp);
return -EAGAIN;
}
@@ -428,8 +428,8 @@ xfs_attr_set_iter(
* Commit the flag value change and start the next trans
* in series.
*/
- dac->dela_state = XFS_DAS_FLIP_LFLAG;
- trace_xfs_attr_set_iter_return(dac->dela_state,
+ attr->xattri_dela_state = XFS_DAS_FLIP_LFLAG;
+ trace_xfs_attr_set_iter_return(attr->xattri_dela_state,
args->dp);
return -EAGAIN;
}
@@ -448,16 +448,16 @@ xfs_attr_set_iter(
/* fallthrough */
case XFS_DAS_RM_LBLK:
/* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
- dac->dela_state = XFS_DAS_RM_LBLK;
+ attr->xattri_dela_state = XFS_DAS_RM_LBLK;
if (args->rmtblkno) {
- error = xfs_attr_rmtval_remove(dac);
+ error = xfs_attr_rmtval_remove(attr);
if (error == -EAGAIN)
trace_xfs_attr_set_iter_return(
- dac->dela_state, args->dp);
+ attr->xattri_dela_state, args->dp);
if (error)
return error;
- dac->dela_state = XFS_DAS_RD_LEAF;
+ attr->xattri_dela_state = XFS_DAS_RD_LEAF;
return -EAGAIN;
}
@@ -488,7 +488,7 @@ xfs_attr_set_iter(
* state.
*/
if (args->rmtblkno > 0) {
- error = xfs_attr_rmtval_find_space(dac);
+ error = xfs_attr_rmtval_find_space(attr);
if (error)
return error;
}
@@ -501,14 +501,14 @@ xfs_attr_set_iter(
* after we create the attribute so that we don't overflow the
* maximum size of a transaction and/or hit a deadlock.
*/
- dac->dela_state = XFS_DAS_ALLOC_NODE;
+ attr->xattri_dela_state = XFS_DAS_ALLOC_NODE;
if (args->rmtblkno > 0) {
- if (dac->blkcnt > 0) {
- error = xfs_attr_rmtval_set_blk(dac);
+ if (attr->xattri_blkcnt > 0) {
+ error = xfs_attr_rmtval_set_blk(attr);
if (error)
return error;
trace_xfs_attr_set_iter_return(
- dac->dela_state, args->dp);
+ attr->xattri_dela_state, args->dp);
return -EAGAIN;
}
@@ -544,8 +544,8 @@ xfs_attr_set_iter(
* Commit the flag value change and start the next trans
* in series
*/
- dac->dela_state = XFS_DAS_FLIP_NFLAG;
- trace_xfs_attr_set_iter_return(dac->dela_state,
+ attr->xattri_dela_state = XFS_DAS_FLIP_NFLAG;
+ trace_xfs_attr_set_iter_return(attr->xattri_dela_state,
args->dp);
return -EAGAIN;
}
@@ -565,17 +565,17 @@ xfs_attr_set_iter(
/* fallthrough */
case XFS_DAS_RM_NBLK:
/* Set state in case xfs_attr_rmtval_remove returns -EAGAIN */
- dac->dela_state = XFS_DAS_RM_NBLK;
+ attr->xattri_dela_state = XFS_DAS_RM_NBLK;
if (args->rmtblkno) {
- error = xfs_attr_rmtval_remove(dac);
+ error = xfs_attr_rmtval_remove(attr);
if (error == -EAGAIN)
trace_xfs_attr_set_iter_return(
- dac->dela_state, args->dp);
+ attr->xattri_dela_state, args->dp);
if (error)
return error;
- dac->dela_state = XFS_DAS_CLR_FLAG;
+ attr->xattri_dela_state = XFS_DAS_CLR_FLAG;
return -EAGAIN;
}
@@ -585,7 +585,7 @@ xfs_attr_set_iter(
* The last state for node format. Look up the old attr and
* remove it.
*/
- error = xfs_attr_node_addname_clear_incomplete(dac);
+ error = xfs_attr_node_addname_clear_incomplete(attr);
break;
default:
ASSERT(0);
@@ -782,7 +782,7 @@ xfs_attr_item_init(
new = kmem_zalloc(sizeof(struct xfs_attr_item), KM_NOFS);
new->xattri_op_flags = op_flags;
- new->xattri_dac.da_args = args;
+ new->xattri_da_args = args;
*attr = new;
return 0;
@@ -1095,16 +1095,16 @@ xfs_attr_node_hasname(
STATIC int
xfs_attr_node_addname_find_attr(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_args *args = attr->xattri_da_args;
int retval;
/*
* Search to see if name already exists, and get back a pointer
* to where it should go.
*/
- retval = xfs_attr_node_hasname(args, &dac->da_state);
+ retval = xfs_attr_node_hasname(args, &attr->xattri_da_state);
if (retval != -ENOATTR && retval != -EEXIST)
return retval;
@@ -1132,8 +1132,8 @@ xfs_attr_node_addname_find_attr(
return 0;
error:
- if (dac->da_state)
- xfs_da_state_free(dac->da_state);
+ if (attr->xattri_da_state)
+ xfs_da_state_free(attr->xattri_da_state);
return retval;
}
@@ -1154,10 +1154,10 @@ error:
*/
STATIC int
xfs_attr_node_addname(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
- struct xfs_da_state *state = dac->da_state;
+ struct xfs_da_args *args = attr->xattri_da_args;
+ struct xfs_da_state *state = attr->xattri_da_state;
struct xfs_da_state_blk *blk;
int error;
@@ -1188,7 +1188,7 @@ xfs_attr_node_addname(
* this point.
*/
trace_xfs_attr_node_addname_return(
- dac->dela_state, args->dp);
+ attr->xattri_dela_state, args->dp);
return -EAGAIN;
}
@@ -1217,9 +1217,9 @@ out:
STATIC int
xfs_attr_node_addname_clear_incomplete(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_args *args = attr->xattri_da_args;
struct xfs_da_state *state = NULL;
int retval = 0;
int error = 0;
@@ -1320,10 +1320,10 @@ xfs_attr_leaf_mark_incomplete(
*/
STATIC
int xfs_attr_node_removename_setup(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
- struct xfs_da_state **state = &dac->da_state;
+ struct xfs_da_args *args = attr->xattri_da_args;
+ struct xfs_da_state **state = &attr->xattri_da_state;
int error;
error = xfs_attr_node_hasname(args, state);
@@ -1382,16 +1382,16 @@ xfs_attr_node_removename(
*/
int
xfs_attr_remove_iter(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
- struct xfs_da_state *state = dac->da_state;
+ struct xfs_da_args *args = attr->xattri_da_args;
+ struct xfs_da_state *state = attr->xattri_da_state;
int retval, error;
struct xfs_inode *dp = args->dp;
trace_xfs_attr_node_removename(args);
- switch (dac->dela_state) {
+ switch (attr->xattri_dela_state) {
case XFS_DAS_UNINIT:
if (!xfs_inode_hasattr(dp))
return -ENOATTR;
@@ -1410,16 +1410,16 @@ xfs_attr_remove_iter(
* Node format may require transaction rolls. Set up the
* state context and fall into the state machine.
*/
- if (!dac->da_state) {
- error = xfs_attr_node_removename_setup(dac);
+ if (!attr->xattri_da_state) {
+ error = xfs_attr_node_removename_setup(attr);
if (error)
return error;
- state = dac->da_state;
+ state = attr->xattri_da_state;
}
/* fallthrough */
case XFS_DAS_RMTBLK:
- dac->dela_state = XFS_DAS_RMTBLK;
+ attr->xattri_dela_state = XFS_DAS_RMTBLK;
/*
* If there is an out-of-line value, de-allocate the blocks.
@@ -1432,10 +1432,10 @@ xfs_attr_remove_iter(
* May return -EAGAIN. Roll and repeat until all remote
* blocks are removed.
*/
- error = xfs_attr_rmtval_remove(dac);
+ error = xfs_attr_rmtval_remove(attr);
if (error == -EAGAIN) {
trace_xfs_attr_remove_iter_return(
- dac->dela_state, args->dp);
+ attr->xattri_dela_state, args->dp);
return error;
} else if (error) {
goto out;
@@ -1450,7 +1450,7 @@ xfs_attr_remove_iter(
error = xfs_attr_refillstate(state);
if (error)
goto out;
- dac->dela_state = XFS_DAS_RM_NAME;
+ attr->xattri_dela_state = XFS_DAS_RM_NAME;
return -EAGAIN;
}
@@ -1460,7 +1460,7 @@ xfs_attr_remove_iter(
* If we came here fresh from a transaction roll, reattach all
* the buffers to the current transaction.
*/
- if (dac->dela_state == XFS_DAS_RM_NAME) {
+ if (attr->xattri_dela_state == XFS_DAS_RM_NAME) {
error = xfs_attr_refillstate(state);
if (error)
goto out;
@@ -1477,9 +1477,9 @@ xfs_attr_remove_iter(
if (error)
goto out;
- dac->dela_state = XFS_DAS_RM_SHRINK;
+ attr->xattri_dela_state = XFS_DAS_RM_SHRINK;
trace_xfs_attr_remove_iter_return(
- dac->dela_state, args->dp);
+ attr->xattri_dela_state, args->dp);
return -EAGAIN;
}
diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h
index c0c92bd3..8a5acde 100644
--- a/libxfs/xfs_attr.h
+++ b/libxfs/xfs_attr.h
@@ -430,7 +430,7 @@ struct xfs_attr_list_context {
*/
/*
- * Enum values for xfs_delattr_context.da_state
+ * Enum values for xfs_attr_item.xattri_da_state
*
* These values are used by delayed attribute operations to keep track of where
* they were before they returned -EAGAIN. A return code of -EAGAIN signals the
@@ -455,7 +455,7 @@ enum xfs_delattr_state {
};
/*
- * Defines for xfs_delattr_context.flags
+ * Defines for xfs_attr_item.xattri_flags
*/
#define XFS_DAC_LEAF_ADDNAME_INIT 0x01 /* xfs_attr_leaf_addname init*/
#define XFS_DAC_DELAYED_OP_INIT 0x02 /* delayed operations init*/
@@ -463,32 +463,25 @@ enum xfs_delattr_state {
/*
* Context used for keeping track of delayed attribute operations
*/
-struct xfs_delattr_context {
- struct xfs_da_args *da_args;
+struct xfs_attr_item {
+ struct xfs_da_args *xattri_da_args;
/*
* Used by xfs_attr_set to hold a leaf buffer across a transaction roll
*/
- struct xfs_buf *leaf_bp;
+ struct xfs_buf *xattri_leaf_bp;
/* Used in xfs_attr_rmtval_set_blk to roll through allocating blocks */
- struct xfs_bmbt_irec map;
- xfs_dablk_t lblkno;
- int blkcnt;
+ struct xfs_bmbt_irec xattri_map;
+ xfs_dablk_t xattri_lblkno;
+ int xattri_blkcnt;
/* Used in xfs_attr_node_removename to roll through removing blocks */
- struct xfs_da_state *da_state;
+ struct xfs_da_state *xattri_da_state;
/* Used to keep track of current state of delayed operation */
- unsigned int flags;
- enum xfs_delattr_state dela_state;
-};
-
-/*
- * List of attrs to commit later.
- */
-struct xfs_attr_item {
- struct xfs_delattr_context xattri_dac;
+ unsigned int xattri_flags;
+ enum xfs_delattr_state xattri_dela_state;
/*
* Indicates if the attr operation is a set or a remove
@@ -496,7 +489,10 @@ struct xfs_attr_item {
*/
unsigned int xattri_op_flags;
- /* used to log this item to an intent */
+ /*
+ * used to log this item to an intent containing a list of attrs to
+ * commit later
+ */
struct list_head xattri_list;
};
@@ -516,12 +512,10 @@ bool xfs_attr_is_leaf(struct xfs_inode *ip);
int xfs_attr_get_ilocked(struct xfs_da_args *args);
int xfs_attr_get(struct xfs_da_args *args);
int xfs_attr_set(struct xfs_da_args *args);
-int xfs_attr_set_iter(struct xfs_delattr_context *dac);
+int xfs_attr_set_iter(struct xfs_attr_item *attr);
int xfs_has_attr(struct xfs_da_args *args);
-int xfs_attr_remove_iter(struct xfs_delattr_context *dac);
+int xfs_attr_remove_iter(struct xfs_attr_item *attr);
bool xfs_attr_namecheck(const void *name, size_t length);
-void xfs_delattr_context_init(struct xfs_delattr_context *dac,
- struct xfs_da_args *args);
int xfs_attr_calc_size(struct xfs_da_args *args, int *local);
int xfs_attr_set_deferred(struct xfs_da_args *args);
int xfs_attr_remove_deferred(struct xfs_da_args *args);
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index 42943b3..72461bd 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -567,14 +567,14 @@ xfs_attr_rmtval_stale(
*/
int
xfs_attr_rmtval_find_space(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
- struct xfs_bmbt_irec *map = &dac->map;
+ struct xfs_da_args *args = attr->xattri_da_args;
+ struct xfs_bmbt_irec *map = &attr->xattri_map;
int error;
- dac->lblkno = 0;
- dac->blkcnt = 0;
+ attr->xattri_lblkno = 0;
+ attr->xattri_blkcnt = 0;
args->rmtblkcnt = 0;
args->rmtblkno = 0;
memset(map, 0, sizeof(struct xfs_bmbt_irec));
@@ -583,8 +583,8 @@ xfs_attr_rmtval_find_space(
if (error)
return error;
- dac->blkcnt = args->rmtblkcnt;
- dac->lblkno = args->rmtblkno;
+ attr->xattri_blkcnt = args->rmtblkcnt;
+ attr->xattri_lblkno = args->rmtblkno;
return 0;
}
@@ -597,17 +597,18 @@ xfs_attr_rmtval_find_space(
*/
int
xfs_attr_rmtval_set_blk(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_args *args = attr->xattri_da_args;
struct xfs_inode *dp = args->dp;
- struct xfs_bmbt_irec *map = &dac->map;
+ struct xfs_bmbt_irec *map = &attr->xattri_map;
int nmap;
int error;
nmap = 1;
- error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)dac->lblkno,
- dac->blkcnt, XFS_BMAPI_ATTRFORK, args->total,
+ error = xfs_bmapi_write(args->trans, dp,
+ (xfs_fileoff_t)attr->xattri_lblkno,
+ attr->xattri_blkcnt, XFS_BMAPI_ATTRFORK, args->total,
map, &nmap);
if (error)
return error;
@@ -617,8 +618,8 @@ xfs_attr_rmtval_set_blk(
(map->br_startblock != HOLESTARTBLOCK));
/* roll attribute extent map forwards */
- dac->lblkno += map->br_blockcount;
- dac->blkcnt -= map->br_blockcount;
+ attr->xattri_lblkno += map->br_blockcount;
+ attr->xattri_blkcnt -= map->br_blockcount;
return 0;
}
@@ -672,9 +673,9 @@ xfs_attr_rmtval_invalidate(
*/
int
xfs_attr_rmtval_remove(
- struct xfs_delattr_context *dac)
+ struct xfs_attr_item *attr)
{
- struct xfs_da_args *args = dac->da_args;
+ struct xfs_da_args *args = attr->xattri_da_args;
int error, done;
/*
@@ -694,7 +695,8 @@ xfs_attr_rmtval_remove(
* the parent
*/
if (!done) {
- trace_xfs_attr_rmtval_remove_return(dac->dela_state, args->dp);
+ trace_xfs_attr_rmtval_remove_return(attr->xattri_dela_state,
+ args->dp);
return -EAGAIN;
}
diff --git a/libxfs/xfs_attr_remote.h b/libxfs/xfs_attr_remote.h
index d72eff3..62b398e 100644
--- a/libxfs/xfs_attr_remote.h
+++ b/libxfs/xfs_attr_remote.h
@@ -12,9 +12,9 @@ int xfs_attr_rmtval_get(struct xfs_da_args *args);
int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map,
xfs_buf_flags_t incore_flags);
int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
-int xfs_attr_rmtval_remove(struct xfs_delattr_context *dac);
+int xfs_attr_rmtval_remove(struct xfs_attr_item *attr);
int xfs_attr_rmt_find_hole(struct xfs_da_args *args);
int xfs_attr_rmtval_set_value(struct xfs_da_args *args);
-int xfs_attr_rmtval_set_blk(struct xfs_delattr_context *dac);
-int xfs_attr_rmtval_find_space(struct xfs_delattr_context *dac);
+int xfs_attr_rmtval_set_blk(struct xfs_attr_item *attr);
+int xfs_attr_rmtval_find_space(struct xfs_attr_item *attr);
#endif /* __XFS_ATTR_REMOTE_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 26/27] xfsprogs: Add helper function xfs_attr_leaf_addname
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (24 preceding siblings ...)
2021-07-27 6:19 ` [PATCH v22 25/27] xfsprogs: Merge xfs_delattr_context into xfs_attr_item Allison Henderson
@ 2021-07-27 6:19 ` Allison Henderson
2021-07-27 6:19 ` [PATCH v22 27/27] xfsprogs: Add log item printing for ATTRI and ATTRD Allison Henderson
26 siblings, 0 replies; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:19 UTC (permalink / raw)
To: linux-xfs
Source kernel commit: 6d9d518192a3f633fce0821601847966280857f7
This patch adds a helper function xfs_attr_leaf_addname. While this
does help to break down xfs_attr_set_iter, it does also hoist out some
of the state management. This patch has been moved to the end of the
clean up series for further discussion.
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
include/xfs_trace.h | 1 +
libxfs/xfs_attr.c | 108 +++++++++++++++++++++++++++++-----------------------
2 files changed, 61 insertions(+), 48 deletions(-)
diff --git a/include/xfs_trace.h b/include/xfs_trace.h
index 2169c27..4557ddd 100644
--- a/include/xfs_trace.h
+++ b/include/xfs_trace.h
@@ -314,6 +314,7 @@
#define trace_xfs_attr_sf_addname_return(a,b) ((void) 0)
#define trace_xfs_attr_set_iter_return(a,b) ((void) 0)
+#define trace_xfs_attr_leaf_addname_return(a,b) ((void) 0)
#define trace_xfs_attr_node_addname_return(a,b) ((void) 0)
#define trace_xfs_attr_remove_iter_return(a,b) ((void) 0)
#define trace_xfs_attr_rmtval_remove_return(a,b) ((void) 0)
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index f5d4380..c84997e 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -282,6 +282,65 @@ xfs_attr_sf_addname(
return -EAGAIN;
}
+STATIC int
+xfs_attr_leaf_addname(
+ struct xfs_attr_item *attr)
+{
+ struct xfs_da_args *args = attr->xattri_da_args;
+ struct xfs_buf *leaf_bp = attr->xattri_leaf_bp;
+ struct xfs_inode *dp = args->dp;
+ int error;
+
+ if (xfs_attr_is_leaf(dp)) {
+ error = xfs_attr_leaf_try_add(args, leaf_bp);
+ if (error == -ENOSPC) {
+ error = xfs_attr3_leaf_to_node(args);
+ if (error)
+ return error;
+
+ /*
+ * Finish any deferred work items and roll the
+ * transaction once more. The goal here is to call
+ * node_addname with the inode and transaction in the
+ * same state (inode locked and joined, transaction
+ * clean) no matter how we got to this step.
+ *
+ * At this point, we are still in XFS_DAS_UNINIT, but
+ * when we come back, we'll be a node, so we'll fall
+ * down into the node handling code below
+ */
+ trace_xfs_attr_set_iter_return(
+ attr->xattri_dela_state, args->dp);
+ return -EAGAIN;
+ }
+
+ if (error)
+ return error;
+
+ attr->xattri_dela_state = XFS_DAS_FOUND_LBLK;
+ } else {
+ error = xfs_attr_node_addname_find_attr(attr);
+ if (error)
+ return error;
+
+ error = xfs_attr_node_addname(attr);
+ if (error)
+ return error;
+
+ /*
+ * If addname was successful, and we dont need to alloc or
+ * remove anymore blks, we're done.
+ */
+ if (!args->rmtblkno && !args->rmtblkno2)
+ return 0;
+
+ attr->xattri_dela_state = XFS_DAS_FOUND_NBLK;
+ }
+
+ trace_xfs_attr_leaf_addname_return(attr->xattri_dela_state, args->dp);
+ return -EAGAIN;
+}
+
/*
* Set the attribute specified in @args.
* This routine is meant to function as a delayed operation, and may return
@@ -317,55 +376,8 @@ xfs_attr_set_iter(
*leaf_bp = NULL;
}
- if (xfs_attr_is_leaf(dp)) {
- error = xfs_attr_leaf_try_add(args, *leaf_bp);
- if (error == -ENOSPC) {
- error = xfs_attr3_leaf_to_node(args);
- if (error)
- return error;
-
- /*
- * Finish any deferred work items and roll the
- * transaction once more. The goal here is to
- * call node_addname with the inode and
- * transaction in the same state (inode locked
- * and joined, transaction clean) no matter how
- * we got to this step.
- *
- * At this point, we are still in
- * XFS_DAS_UNINIT, but when we come back, we'll
- * be a node, so we'll fall down into the node
- * handling code below
- */
- trace_xfs_attr_set_iter_return(
- attr->xattri_dela_state, args->dp);
- return -EAGAIN;
- } else if (error) {
- return error;
- }
-
- attr->xattri_dela_state = XFS_DAS_FOUND_LBLK;
- } else {
- error = xfs_attr_node_addname_find_attr(attr);
- if (error)
- return error;
+ return xfs_attr_leaf_addname(attr);
- error = xfs_attr_node_addname(attr);
- if (error)
- return error;
-
- /*
- * If addname was successful, and we dont need to alloc
- * or remove anymore blks, we're done.
- */
- if (!args->rmtblkno && !args->rmtblkno2)
- return 0;
-
- attr->xattri_dela_state = XFS_DAS_FOUND_NBLK;
- }
- trace_xfs_attr_set_iter_return(attr->xattri_dela_state,
- args->dp);
- return -EAGAIN;
case XFS_DAS_FOUND_LBLK:
/*
* If there was an out-of-line value, allocate the blocks we
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v22 27/27] xfsprogs: Add log item printing for ATTRI and ATTRD
2021-07-27 6:18 [PATCH v22 00/27] Delayed Attributes Allison Henderson
` (25 preceding siblings ...)
2021-07-27 6:19 ` [PATCH v22 26/27] xfsprogs: Add helper function xfs_attr_leaf_addname Allison Henderson
@ 2021-07-27 6:19 ` Allison Henderson
2021-07-28 19:41 ` Darrick J. Wong
26 siblings, 1 reply; 29+ messages in thread
From: Allison Henderson @ 2021-07-27 6:19 UTC (permalink / raw)
To: linux-xfs
This patch implements a new set of log printing functions to print the
ATTRI and ATTRD items and vectors in the log. These will be used during
log dump and log recover operations.
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
logprint/log_misc.c | 48 +++++++++++-
logprint/log_print_all.c | 12 +++
logprint/log_redo.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++
logprint/logprint.h | 12 +++
4 files changed, 268 insertions(+), 1 deletion(-)
diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index 35e926a..d8c6038 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -54,11 +54,46 @@ print_stars(void)
"***********************************\n");
} /* print_stars */
+void
+print_hex_dump(char *ptr, int len) {
+ int i = 0;
+
+ for (i = 0; i < len; i++) {
+ if (i % 16 == 0)
+ printf("%08x ", i);
+
+ printf("%02x", ptr[i]);
+
+ if ((i+1)%16 == 0)
+ printf("\n");
+ else if ((i+1)%2 == 0)
+ printf(" ");
+ }
+ printf("\n");
+}
+
+bool
+is_printable(char *ptr, int len) {
+ int i = 0;
+
+ for (i = 0; i < len; i++)
+ if (!isprint(ptr[i]) )
+ return false;
+ return true;
+}
+
+void print_or_dump(char *ptr, int len) {
+ if (is_printable(ptr, len))
+ printf("%.*s\n", len, ptr);
+ else
+ print_hex_dump(ptr, len);
+}
+
/*
* Given a pointer to a data segment, print out the data as if it were
* a log operation header.
*/
-static void
+void
xlog_print_op_header(xlog_op_header_t *op_head,
int i,
char **ptr)
@@ -961,6 +996,17 @@ xlog_print_record(
be32_to_cpu(op_head->oh_len));
break;
}
+ case XFS_LI_ATTRI: {
+ skip = xlog_print_trans_attri(&ptr,
+ be32_to_cpu(op_head->oh_len),
+ &i);
+ break;
+ }
+ case XFS_LI_ATTRD: {
+ skip = xlog_print_trans_attrd(&ptr,
+ be32_to_cpu(op_head->oh_len));
+ break;
+ }
case XFS_LI_RUI: {
skip = xlog_print_trans_rui(&ptr,
be32_to_cpu(op_head->oh_len),
diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
index c9c453f..89cb649 100644
--- a/logprint/log_print_all.c
+++ b/logprint/log_print_all.c
@@ -404,6 +404,12 @@ xlog_recover_print_logitem(
case XFS_LI_EFI:
xlog_recover_print_efi(item);
break;
+ case XFS_LI_ATTRD:
+ xlog_recover_print_attrd(item);
+ break;
+ case XFS_LI_ATTRI:
+ xlog_recover_print_attri(item);
+ break;
case XFS_LI_RUD:
xlog_recover_print_rud(item);
break;
@@ -456,6 +462,12 @@ xlog_recover_print_item(
case XFS_LI_EFI:
printf("EFI");
break;
+ case XFS_LI_ATTRD:
+ printf("ATTRD");
+ break;
+ case XFS_LI_ATTRI:
+ printf("ATTRI");
+ break;
case XFS_LI_RUD:
printf("RUD");
break;
diff --git a/logprint/log_redo.c b/logprint/log_redo.c
index 297e203..502345d 100644
--- a/logprint/log_redo.c
+++ b/logprint/log_redo.c
@@ -653,3 +653,200 @@ xlog_recover_print_bud(
f = item->ri_buf[0].i_addr;
xlog_print_trans_bud(&f, sizeof(struct xfs_bud_log_format));
}
+
+/* Attr Items */
+
+static int
+xfs_attri_copy_log_format(
+ char *buf,
+ uint len,
+ struct xfs_attri_log_format *dst_attri_fmt)
+{
+ uint dst_len = sizeof(struct xfs_attri_log_format);
+
+ if (len == dst_len) {
+ memcpy((char *)dst_attri_fmt, buf, len);
+ return 0;
+ }
+
+ fprintf(stderr, _("%s: bad size of attri format: %u; expected %u\n"),
+ progname, len, dst_len);
+ return 1;
+}
+
+int
+xlog_print_trans_attri(
+ char **ptr,
+ uint src_len,
+ int *i)
+{
+ struct xfs_attri_log_format *src_f = NULL;
+ xlog_op_header_t *head = NULL;
+ uint dst_len;
+ int error = 0;
+
+ dst_len = sizeof(struct xfs_attri_log_format);
+ if (src_len != dst_len) {
+ fprintf(stderr, _("%s: bad size of attri format: %u; expected %u\n"),
+ progname, src_len, dst_len);
+ return 1;
+ }
+
+ /*
+ * memmove to ensure 8-byte alignment for the long longs in
+ * xfs_attri_log_format_t structure
+ */
+ src_f = malloc(src_len);
+ if (!src_f) {
+ fprintf(stderr, _("%s: xlog_print_trans_attri: malloc failed\n"),
+ progname);
+ exit(1);
+ }
+ memmove((char*)src_f, *ptr, src_len);
+ *ptr += src_len;
+
+ printf(_("ATTRI: #regs: %d name_len: %d, value_len: %d id: 0x%llx\n"),
+ src_f->alfi_size, src_f->alfi_name_len, src_f->alfi_value_len,
+ (unsigned long long)src_f->alfi_id);
+
+ if (src_f->alfi_name_len > 0) {
+ printf(_("\n"));
+ (*i)++;
+ head = (xlog_op_header_t *)*ptr;
+ xlog_print_op_header(head, *i, ptr);
+ error = xlog_print_trans_attri_name(ptr, be32_to_cpu(head->oh_len));
+ if (error)
+ goto error;
+ }
+
+ if (src_f->alfi_value_len > 0) {
+ printf(_("\n"));
+ (*i)++;
+ head = (xlog_op_header_t *)*ptr;
+ xlog_print_op_header(head, *i, ptr);
+ error = xlog_print_trans_attri_value(ptr, be32_to_cpu(head->oh_len),
+ src_f->alfi_value_len);
+ }
+error:
+ free(src_f);
+
+ return error;
+} /* xlog_print_trans_attri */
+
+int
+xlog_print_trans_attri_name(
+ char **ptr,
+ uint src_len)
+{
+ printf(_("ATTRI: name len:%u\n"), src_len);
+ print_or_dump(*ptr, src_len);
+
+ *ptr += src_len;
+
+ return 0;
+} /* xlog_print_trans_attri */
+
+int
+xlog_print_trans_attri_value(
+ char **ptr,
+ uint src_len,
+ int value_len)
+{
+ int len = value_len;
+
+ if (len > MAX_ATTR_VAL_PRINT)
+ len = MAX_ATTR_VAL_PRINT;
+
+ printf(_("ATTRI: value len:%u\n"), value_len);
+ print_or_dump(*ptr, len);
+
+ *ptr += src_len;
+
+ return 0;
+} /* xlog_print_trans_attri_value */
+
+void
+xlog_recover_print_attri(
+ struct xlog_recover_item *item)
+{
+ struct xfs_attri_log_format *f, *src_f = NULL;
+ uint src_len, dst_len;
+
+ int region = 0;
+
+ src_f = (struct xfs_attri_log_format *)item->ri_buf[0].i_addr;
+ src_len = item->ri_buf[region].i_len;
+
+ /*
+ * An xfs_attri_log_format structure contains a attribute name and
+ * variable length value as the last field.
+ */
+ dst_len = sizeof(struct xfs_attri_log_format);
+
+ if ((f = ((struct xfs_attri_log_format *)malloc(dst_len))) == NULL) {
+ fprintf(stderr, _("%s: xlog_recover_print_attri: malloc failed\n"),
+ progname);
+ exit(1);
+ }
+ if (xfs_attri_copy_log_format((char*)src_f, src_len, f))
+ goto out;
+
+ printf(_("ATTRI: #regs: %d name_len: %d, value_len: %d id: 0x%llx\n"),
+ f->alfi_size, f->alfi_name_len, f->alfi_value_len, (unsigned long long)f->alfi_id);
+
+ if (f->alfi_name_len > 0) {
+ region++;
+ printf(_("ATTRI: name len:%u\n"), f->alfi_name_len);
+ print_or_dump((char *)item->ri_buf[region].i_addr,
+ f->alfi_name_len);
+ }
+
+ if (f->alfi_value_len > 0) {
+ int len = f->alfi_value_len;
+
+ if (len > MAX_ATTR_VAL_PRINT)
+ len = MAX_ATTR_VAL_PRINT;
+
+ region++;
+ printf(_("ATTRI: value len:%u\n"), f->alfi_value_len);
+ print_or_dump((char *)item->ri_buf[region].i_addr, len);
+ }
+
+out:
+ free(f);
+
+}
+
+int
+xlog_print_trans_attrd(char **ptr, uint len)
+{
+ struct xfs_attrd_log_format *f;
+ struct xfs_attrd_log_format lbuf;
+ uint core_size = sizeof(struct xfs_attrd_log_format);
+
+ memcpy(&lbuf, *ptr, MIN(core_size, len));
+ f = &lbuf;
+ *ptr += len;
+ if (len >= core_size) {
+ printf(_("ATTRD: #regs: %d id: 0x%llx\n"),
+ f->alfd_size,
+ (unsigned long long)f->alfd_alf_id);
+ return 0;
+ } else {
+ printf(_("ATTRD: Not enough data to decode further\n"));
+ return 1;
+ }
+} /* xlog_print_trans_attrd */
+
+void
+xlog_recover_print_attrd(
+ struct xlog_recover_item *item)
+{
+ struct xfs_attrd_log_format *f;
+
+ f = (struct xfs_attrd_log_format *)item->ri_buf[0].i_addr;
+
+ printf(_(" ATTRD: #regs: %d id: 0x%llx\n"),
+ f->alfd_size,
+ (unsigned long long)f->alfd_alf_id);
+}
diff --git a/logprint/logprint.h b/logprint/logprint.h
index 38a7d3f..b4479c2 100644
--- a/logprint/logprint.h
+++ b/logprint/logprint.h
@@ -29,6 +29,9 @@ extern void xfs_log_print_trans(struct xlog *, int);
extern void print_xlog_record_line(void);
extern void print_xlog_op_line(void);
extern void print_stars(void);
+extern void print_hex_dump(char* ptr, int len);
+extern bool is_printable(char* ptr, int len);
+extern void print_or_dump(char* ptr, int len);
extern struct xfs_inode_log_format *
xfs_inode_item_format_convert(char *, uint, struct xfs_inode_log_format *);
@@ -53,4 +56,13 @@ extern void xlog_recover_print_bui(struct xlog_recover_item *item);
extern int xlog_print_trans_bud(char **ptr, uint len);
extern void xlog_recover_print_bud(struct xlog_recover_item *item);
+#define MAX_ATTR_VAL_PRINT 128
+
+extern int xlog_print_trans_attri(char **ptr, uint src_len, int *i);
+extern int xlog_print_trans_attri_name(char **ptr, uint src_len);
+extern int xlog_print_trans_attri_value(char **ptr, uint src_len, int value_len);
+extern void xlog_recover_print_attri(struct xlog_recover_item *item);
+extern int xlog_print_trans_attrd(char **ptr, uint len);
+extern void xlog_recover_print_attrd(struct xlog_recover_item *item);
+extern void xlog_print_op_header(xlog_op_header_t *op_head, int i, char **ptr);
#endif /* LOGPRINT_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v22 27/27] xfsprogs: Add log item printing for ATTRI and ATTRD
2021-07-27 6:19 ` [PATCH v22 27/27] xfsprogs: Add log item printing for ATTRI and ATTRD Allison Henderson
@ 2021-07-28 19:41 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2021-07-28 19:41 UTC (permalink / raw)
To: Allison Henderson; +Cc: linux-xfs
On Mon, Jul 26, 2021 at 11:19:04PM -0700, Allison Henderson wrote:
> This patch implements a new set of log printing functions to print the
> ATTRI and ATTRD items and vectors in the log. These will be used during
> log dump and log recover operations.
>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
/me thinks this looks ok, though I admit I'm relying on you to have
exercised logprint and made sure that the output looks right.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> logprint/log_misc.c | 48 +++++++++++-
> logprint/log_print_all.c | 12 +++
> logprint/log_redo.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++
> logprint/logprint.h | 12 +++
> 4 files changed, 268 insertions(+), 1 deletion(-)
>
> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> index 35e926a..d8c6038 100644
> --- a/logprint/log_misc.c
> +++ b/logprint/log_misc.c
> @@ -54,11 +54,46 @@ print_stars(void)
> "***********************************\n");
> } /* print_stars */
>
> +void
> +print_hex_dump(char *ptr, int len) {
> + int i = 0;
> +
> + for (i = 0; i < len; i++) {
> + if (i % 16 == 0)
> + printf("%08x ", i);
> +
> + printf("%02x", ptr[i]);
> +
> + if ((i+1)%16 == 0)
> + printf("\n");
> + else if ((i+1)%2 == 0)
> + printf(" ");
> + }
> + printf("\n");
> +}
> +
> +bool
> +is_printable(char *ptr, int len) {
> + int i = 0;
> +
> + for (i = 0; i < len; i++)
> + if (!isprint(ptr[i]) )
> + return false;
> + return true;
> +}
> +
> +void print_or_dump(char *ptr, int len) {
> + if (is_printable(ptr, len))
> + printf("%.*s\n", len, ptr);
> + else
> + print_hex_dump(ptr, len);
> +}
> +
> /*
> * Given a pointer to a data segment, print out the data as if it were
> * a log operation header.
> */
> -static void
> +void
> xlog_print_op_header(xlog_op_header_t *op_head,
> int i,
> char **ptr)
> @@ -961,6 +996,17 @@ xlog_print_record(
> be32_to_cpu(op_head->oh_len));
> break;
> }
> + case XFS_LI_ATTRI: {
> + skip = xlog_print_trans_attri(&ptr,
> + be32_to_cpu(op_head->oh_len),
> + &i);
> + break;
> + }
> + case XFS_LI_ATTRD: {
> + skip = xlog_print_trans_attrd(&ptr,
> + be32_to_cpu(op_head->oh_len));
> + break;
> + }
> case XFS_LI_RUI: {
> skip = xlog_print_trans_rui(&ptr,
> be32_to_cpu(op_head->oh_len),
> diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
> index c9c453f..89cb649 100644
> --- a/logprint/log_print_all.c
> +++ b/logprint/log_print_all.c
> @@ -404,6 +404,12 @@ xlog_recover_print_logitem(
> case XFS_LI_EFI:
> xlog_recover_print_efi(item);
> break;
> + case XFS_LI_ATTRD:
> + xlog_recover_print_attrd(item);
> + break;
> + case XFS_LI_ATTRI:
> + xlog_recover_print_attri(item);
> + break;
> case XFS_LI_RUD:
> xlog_recover_print_rud(item);
> break;
> @@ -456,6 +462,12 @@ xlog_recover_print_item(
> case XFS_LI_EFI:
> printf("EFI");
> break;
> + case XFS_LI_ATTRD:
> + printf("ATTRD");
> + break;
> + case XFS_LI_ATTRI:
> + printf("ATTRI");
> + break;
> case XFS_LI_RUD:
> printf("RUD");
> break;
> diff --git a/logprint/log_redo.c b/logprint/log_redo.c
> index 297e203..502345d 100644
> --- a/logprint/log_redo.c
> +++ b/logprint/log_redo.c
> @@ -653,3 +653,200 @@ xlog_recover_print_bud(
> f = item->ri_buf[0].i_addr;
> xlog_print_trans_bud(&f, sizeof(struct xfs_bud_log_format));
> }
> +
> +/* Attr Items */
> +
> +static int
> +xfs_attri_copy_log_format(
> + char *buf,
> + uint len,
> + struct xfs_attri_log_format *dst_attri_fmt)
> +{
> + uint dst_len = sizeof(struct xfs_attri_log_format);
> +
> + if (len == dst_len) {
> + memcpy((char *)dst_attri_fmt, buf, len);
> + return 0;
> + }
> +
> + fprintf(stderr, _("%s: bad size of attri format: %u; expected %u\n"),
> + progname, len, dst_len);
> + return 1;
> +}
> +
> +int
> +xlog_print_trans_attri(
> + char **ptr,
> + uint src_len,
> + int *i)
> +{
> + struct xfs_attri_log_format *src_f = NULL;
> + xlog_op_header_t *head = NULL;
> + uint dst_len;
> + int error = 0;
> +
> + dst_len = sizeof(struct xfs_attri_log_format);
> + if (src_len != dst_len) {
> + fprintf(stderr, _("%s: bad size of attri format: %u; expected %u\n"),
> + progname, src_len, dst_len);
> + return 1;
> + }
> +
> + /*
> + * memmove to ensure 8-byte alignment for the long longs in
> + * xfs_attri_log_format_t structure
> + */
> + src_f = malloc(src_len);
> + if (!src_f) {
> + fprintf(stderr, _("%s: xlog_print_trans_attri: malloc failed\n"),
> + progname);
> + exit(1);
> + }
> + memmove((char*)src_f, *ptr, src_len);
> + *ptr += src_len;
> +
> + printf(_("ATTRI: #regs: %d name_len: %d, value_len: %d id: 0x%llx\n"),
> + src_f->alfi_size, src_f->alfi_name_len, src_f->alfi_value_len,
> + (unsigned long long)src_f->alfi_id);
> +
> + if (src_f->alfi_name_len > 0) {
> + printf(_("\n"));
> + (*i)++;
> + head = (xlog_op_header_t *)*ptr;
> + xlog_print_op_header(head, *i, ptr);
> + error = xlog_print_trans_attri_name(ptr, be32_to_cpu(head->oh_len));
> + if (error)
> + goto error;
> + }
> +
> + if (src_f->alfi_value_len > 0) {
> + printf(_("\n"));
> + (*i)++;
> + head = (xlog_op_header_t *)*ptr;
> + xlog_print_op_header(head, *i, ptr);
> + error = xlog_print_trans_attri_value(ptr, be32_to_cpu(head->oh_len),
> + src_f->alfi_value_len);
> + }
> +error:
> + free(src_f);
> +
> + return error;
> +} /* xlog_print_trans_attri */
> +
> +int
> +xlog_print_trans_attri_name(
> + char **ptr,
> + uint src_len)
> +{
> + printf(_("ATTRI: name len:%u\n"), src_len);
> + print_or_dump(*ptr, src_len);
> +
> + *ptr += src_len;
> +
> + return 0;
> +} /* xlog_print_trans_attri */
> +
> +int
> +xlog_print_trans_attri_value(
> + char **ptr,
> + uint src_len,
> + int value_len)
> +{
> + int len = value_len;
> +
> + if (len > MAX_ATTR_VAL_PRINT)
> + len = MAX_ATTR_VAL_PRINT;
> +
> + printf(_("ATTRI: value len:%u\n"), value_len);
> + print_or_dump(*ptr, len);
> +
> + *ptr += src_len;
> +
> + return 0;
> +} /* xlog_print_trans_attri_value */
> +
> +void
> +xlog_recover_print_attri(
> + struct xlog_recover_item *item)
> +{
> + struct xfs_attri_log_format *f, *src_f = NULL;
> + uint src_len, dst_len;
> +
> + int region = 0;
> +
> + src_f = (struct xfs_attri_log_format *)item->ri_buf[0].i_addr;
> + src_len = item->ri_buf[region].i_len;
> +
> + /*
> + * An xfs_attri_log_format structure contains a attribute name and
> + * variable length value as the last field.
> + */
> + dst_len = sizeof(struct xfs_attri_log_format);
> +
> + if ((f = ((struct xfs_attri_log_format *)malloc(dst_len))) == NULL) {
> + fprintf(stderr, _("%s: xlog_recover_print_attri: malloc failed\n"),
> + progname);
> + exit(1);
> + }
> + if (xfs_attri_copy_log_format((char*)src_f, src_len, f))
> + goto out;
> +
> + printf(_("ATTRI: #regs: %d name_len: %d, value_len: %d id: 0x%llx\n"),
> + f->alfi_size, f->alfi_name_len, f->alfi_value_len, (unsigned long long)f->alfi_id);
> +
> + if (f->alfi_name_len > 0) {
> + region++;
> + printf(_("ATTRI: name len:%u\n"), f->alfi_name_len);
> + print_or_dump((char *)item->ri_buf[region].i_addr,
> + f->alfi_name_len);
> + }
> +
> + if (f->alfi_value_len > 0) {
> + int len = f->alfi_value_len;
> +
> + if (len > MAX_ATTR_VAL_PRINT)
> + len = MAX_ATTR_VAL_PRINT;
> +
> + region++;
> + printf(_("ATTRI: value len:%u\n"), f->alfi_value_len);
> + print_or_dump((char *)item->ri_buf[region].i_addr, len);
> + }
> +
> +out:
> + free(f);
> +
> +}
> +
> +int
> +xlog_print_trans_attrd(char **ptr, uint len)
> +{
> + struct xfs_attrd_log_format *f;
> + struct xfs_attrd_log_format lbuf;
> + uint core_size = sizeof(struct xfs_attrd_log_format);
> +
> + memcpy(&lbuf, *ptr, MIN(core_size, len));
> + f = &lbuf;
> + *ptr += len;
> + if (len >= core_size) {
> + printf(_("ATTRD: #regs: %d id: 0x%llx\n"),
> + f->alfd_size,
> + (unsigned long long)f->alfd_alf_id);
> + return 0;
> + } else {
> + printf(_("ATTRD: Not enough data to decode further\n"));
> + return 1;
> + }
> +} /* xlog_print_trans_attrd */
> +
> +void
> +xlog_recover_print_attrd(
> + struct xlog_recover_item *item)
> +{
> + struct xfs_attrd_log_format *f;
> +
> + f = (struct xfs_attrd_log_format *)item->ri_buf[0].i_addr;
> +
> + printf(_(" ATTRD: #regs: %d id: 0x%llx\n"),
> + f->alfd_size,
> + (unsigned long long)f->alfd_alf_id);
> +}
> diff --git a/logprint/logprint.h b/logprint/logprint.h
> index 38a7d3f..b4479c2 100644
> --- a/logprint/logprint.h
> +++ b/logprint/logprint.h
> @@ -29,6 +29,9 @@ extern void xfs_log_print_trans(struct xlog *, int);
> extern void print_xlog_record_line(void);
> extern void print_xlog_op_line(void);
> extern void print_stars(void);
> +extern void print_hex_dump(char* ptr, int len);
> +extern bool is_printable(char* ptr, int len);
> +extern void print_or_dump(char* ptr, int len);
>
> extern struct xfs_inode_log_format *
> xfs_inode_item_format_convert(char *, uint, struct xfs_inode_log_format *);
> @@ -53,4 +56,13 @@ extern void xlog_recover_print_bui(struct xlog_recover_item *item);
> extern int xlog_print_trans_bud(char **ptr, uint len);
> extern void xlog_recover_print_bud(struct xlog_recover_item *item);
>
> +#define MAX_ATTR_VAL_PRINT 128
> +
> +extern int xlog_print_trans_attri(char **ptr, uint src_len, int *i);
> +extern int xlog_print_trans_attri_name(char **ptr, uint src_len);
> +extern int xlog_print_trans_attri_value(char **ptr, uint src_len, int value_len);
> +extern void xlog_recover_print_attri(struct xlog_recover_item *item);
> +extern int xlog_print_trans_attrd(char **ptr, uint len);
> +extern void xlog_recover_print_attrd(struct xlog_recover_item *item);
> +extern void xlog_print_op_header(xlog_op_header_t *op_head, int i, char **ptr);
> #endif /* LOGPRINT_H */
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 29+ messages in thread