On 12/20/20 11:45 PM, Chandan Babu R wrote:
On Fri, 18 Dec 2020 00:29:05 -0700, Allison Henderson wrote:
This patch hoists transaction handling in xfs_attr_node_remove to
... "transaction handling in xfs_attr_node_removename"
xfs_attr_node_remove_step. This will help keep transaction handling in
higher level functions instead of buried in subfunctions when we
introduce delay attributes
Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_attr.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index e93d76a..1969b88 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1251,7 +1251,7 @@ xfs_attr_node_remove_step(
struct xfs_da_args *args,
struct xfs_da_state *state)
{
- int retval, error;
+ int error;
struct xfs_inode *dp = args->dp;
The declaration of "dp" variable can be removed since there are no references
to it left after the removal of the following hunk.
Ok, will remove
@@ -1265,25 +1265,6 @@ xfs_attr_node_remove_step(
if (error)
return error;
}
- retval = xfs_attr_node_remove_cleanup(args, state);
-
- /*
- * Check to see if the tree needs to be collapsed.
- */
- if (retval && (state->path.active > 1)) {
- error = xfs_da3_join(state);
- if (error)
- return error;
- error = xfs_defer_finish(&args->trans);
- if (error)
- return error;
- /*
- * Commit the Btree join operation and start a new trans.
- */
- error = xfs_trans_roll_inode(&args->trans, dp);
- if (error)
- return error;
- }
return error;
}
@@ -1299,7 +1280,7 @@ xfs_attr_node_removename(
struct xfs_da_args *args)
{
struct xfs_da_state *state = NULL;
- int error;
+ int retval, error;
struct xfs_inode *dp = args->dp;
trace_xfs_attr_node_removename(args);
@@ -1312,6 +1293,26 @@ xfs_attr_node_removename(
if (error)
goto out;
+ retval = xfs_attr_node_remove_cleanup(args, state);
+
+ /*
+ * Check to see if the tree needs to be collapsed.
+ */
+ if (retval && (state->path.active > 1)) {
+ error = xfs_da3_join(state);
+ if (error)
+ return error;
When a non-zero value is returned by xfs_da3_join(), the code would fail to
free the memory pointed to by "state". Same review comment applies to the two
return statements below.
Ok, these need to be "goto out". Will fix, thx!
Allison
+ error = xfs_defer_finish(&args->trans);
+ if (error)
+ return error;
+ /*
+ * Commit the Btree join operation and start a new trans.
+ */
+ error = xfs_trans_roll_inode(&args->trans, dp);
+ if (error)
+ return error;
+ }
+
/*
* If the result is small enough, push it all into the inode.
*/
