Re: [PATCH v13 01/10] xfs: Add helper xfs_attr_node_remove_step

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/27/20 5:15 AM, Brian Foster wrote:
On Thu, Oct 22, 2020 at 11:34:26PM -0700, Allison Henderson wrote:
From: Allison Collins <allison.henderson@xxxxxxxxxx>

This patch adds a new helper function xfs_attr_node_remove_step.  This
will help simplify and modularize the calling function
xfs_attr_node_remove.

Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
---
  fs/xfs/libxfs/xfs_attr.c | 46 ++++++++++++++++++++++++++++++++++------------
  1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index fd8e641..f4d39bf 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
...
@@ -1267,18 +1262,45 @@ xfs_attr_node_removename(
  	if (retval && (state->path.active > 1)) {
  		error = xfs_da3_join(state);
  		if (error)
-			goto out;
+			return error;
  		error = xfs_defer_finish(&args->trans);
  		if (error)
-			goto out;
+			return error;
  		/*
  		 * Commit the Btree join operation and start a new trans.
  		 */
  		error = xfs_trans_roll_inode(&args->trans, dp);
  		if (error)
-			goto out;
+			return error;
  	}
+ return error;
+}
+
+/*
+ * Remove a name from a B-tree attribute list.
+ *
+ * This routine will find the blocks of the name to remove, remove them and
+ * shirnk the tree if needed.
+ */
+STATIC int
+xfs_attr_node_removename(
+	struct xfs_da_args	*args)
+{
+	struct xfs_da_state	*state;

It urks me a little bit that we have to dig down into a couple functions
to grok that state allocation is the first step or otherwise occurs
before we potentially use the error path. Since we already check for
state in the out path, can we just initialize this as *state = NULL
here so the logic is clear? Otherwise the patch LGTM:

Sure, will add.  Thanks!

Allison
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

+	int			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;
+
+	error = xfs_attr_node_remove_step(args, state);
+	if (error)
+		goto out;
+
  	/*
  	 * If the result is small enough, push it all into the inode.
  	 */
--
2.7.4





[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux