On 1/21/20 4:21 PM, Darrick J. Wong wrote:
On Sat, Jan 18, 2020 at 03:50:31PM -0700, Allison Collins wrote:
This patch helps to pre-simplify xfs_attr_node_removename by modularizing
the code around the transactions into helper functions. This will make
the function easier to follow when we introduce delayed attributes.
Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_attr.c | 45 +++++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index e9d22c1..453ea59 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1202,6 +1202,36 @@ xfs_attr_node_addname(
}
/*
+ * Set up the state for unmap and set the incomplete flag
"Mark an attribute entry INCOMPLETE and save pointers to the relevant
buffers for later deletion of the entry." ?
I am fine with that if folks prefer.
+ */
+STATIC int
+xfs_attr_init_unmapstate(
I'm hung up on this name -- it marks the entry incomplete and saves some
breadcrumbs for erasing data later, which I guess is "unmap state"?
What do you think of:
xfs_attr_leaf_mark_incomplete()
I actually struggled a bit with what to call it. I called it an init
because in the immediately proceeding code we fall into a loop of
bunmapi and refilling the state, rolling the transaction in between.
But we only make this initial call once before we start doing that.
Later when we get into the state machine this turns into multiple
iterations of jumping in and out of this region of code. Even though
the while() syntax is gone, it still ends up functioning very much like
a loop.
This helper function isnt so much necessary as much as it is an effort
to cut down on the size of xfs_attr_node_removename which was getting
quite lengthy.
In anycase, xfs_attr_leaf_mark_incomplete is fine with me :-)
+ struct xfs_da_args *args,
+ struct xfs_da_state *state)
+{
+ int error;
+
+ /*
+ * Fill in disk block numbers in the state structure
+ * so that we can get the buffers back after we commit
+ * several transactions in the following calls.
+ */
+ error = xfs_attr_fillstate(state);
+ if (error)
+ return error;
+
+ /*
+ * Mark the attribute as INCOMPLETE
Has this stopped doing the bunmapi call, or was the comment inaccurate?
I'm guessing the second if it's necessary to save state to recover
buffer pointers later...?
Right, I clipped off the comment because we dont do the bunmapi in this
function scope. That happens later in the loop behavior described above.
Hope that helps!
Allison
--D
+ */
+ error = xfs_attr3_leaf_setflag(args);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+
+/*
* Remove a name from a B-tree attribute list.
*
* This will involve walking down the Btree, and may involve joining
@@ -1233,20 +1263,7 @@ xfs_attr_node_removename(
ASSERT(blk->bp != NULL);
ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
if (args->rmtblkno > 0) {
- /*
- * Fill in disk block numbers in the state structure
- * so that we can get the buffers back after we commit
- * several transactions in the following calls.
- */
- error = xfs_attr_fillstate(state);
- if (error)
- goto out;
-
- /*
- * Mark the attribute as INCOMPLETE, then bunmapi() the
- * remote value.
- */
- error = xfs_attr3_leaf_setflag(args);
+ error = xfs_attr_init_unmapstate(args, state);
if (error)
goto out;
--
2.7.4
