diff mbox series

[v15,03/22] xfs: Hoist transaction handling in xfs_attr_node_remove_step

Message ID 20210218165348.4754-4-allison.henderson@oracle.com (mailing list archive)
State New
Headers show
Series xfs: Delayed Attributes | expand

Commit Message

Allison Henderson Feb. 18, 2021, 4:53 p.m. UTC
This patch hoists transaction handling in xfs_attr_node_removename to
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@oracle.com>
---
 fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

Comments

Brian Foster Feb. 24, 2021, 3:04 p.m. UTC | #1
On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote:
> This patch hoists transaction handling in xfs_attr_node_removename to
> 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@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++-----------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 4e6c89d..3cf76e2 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step(
>  	struct xfs_da_args	*args,
>  	struct xfs_da_state	*state)
>  {
> -	int			retval, error;
> -	struct xfs_inode	*dp = args->dp;
> -
> +	int			error = 0;
>  
>  	/*
>  	 * If there is an out-of-line value, de-allocate the blocks.
> @@ -1265,25 +1263,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 +1278,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 +1291,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)
> +			goto out;
> +		error = xfs_defer_finish(&args->trans);
> +		if (error)
> +			goto out;
> +		/*
> +		 * Commit the Btree join operation and start a new trans.
> +		 */
> +		error = xfs_trans_roll_inode(&args->trans, dp);
> +		if (error)
> +			goto out;
> +	}
> +
>  	/*
>  	 * If the result is small enough, push it all into the inode.
>  	 */
> -- 
> 2.7.4
>
Allison Henderson Feb. 25, 2021, 6:18 a.m. UTC | #2
On 2/24/21 8:04 AM, Brian Foster wrote:
> On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote:
>> This patch hoists transaction handling in xfs_attr_node_removename to
>> 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@oracle.com>
>> ---
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
Great, thank you!
Allison
> 
>>   fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++-----------------------
>>   1 file changed, 22 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 4e6c89d..3cf76e2 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step(
>>   	struct xfs_da_args	*args,
>>   	struct xfs_da_state	*state)
>>   {
>> -	int			retval, error;
>> -	struct xfs_inode	*dp = args->dp;
>> -
>> +	int			error = 0;
>>   
>>   	/*
>>   	 * If there is an out-of-line value, de-allocate the blocks.
>> @@ -1265,25 +1263,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 +1278,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 +1291,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)
>> +			goto out;
>> +		error = xfs_defer_finish(&args->trans);
>> +		if (error)
>> +			goto out;
>> +		/*
>> +		 * Commit the Btree join operation and start a new trans.
>> +		 */
>> +		error = xfs_trans_roll_inode(&args->trans, dp);
>> +		if (error)
>> +			goto out;
>> +	}
>> +
>>   	/*
>>   	 * If the result is small enough, push it all into the inode.
>>   	 */
>> -- 
>> 2.7.4
>>
>
Darrick J. Wong Feb. 26, 2021, 3:02 a.m. UTC | #3
On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote:
> This patch hoists transaction handling in xfs_attr_node_removename to
> 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@oracle.com>

Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++-----------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 4e6c89d..3cf76e2 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step(
>  	struct xfs_da_args	*args,
>  	struct xfs_da_state	*state)
>  {
> -	int			retval, error;
> -	struct xfs_inode	*dp = args->dp;
> -
> +	int			error = 0;
>  
>  	/*
>  	 * If there is an out-of-line value, de-allocate the blocks.
> @@ -1265,25 +1263,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 +1278,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 +1291,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)
> +			goto out;
> +		error = xfs_defer_finish(&args->trans);
> +		if (error)
> +			goto out;
> +		/*
> +		 * Commit the Btree join operation and start a new trans.
> +		 */
> +		error = xfs_trans_roll_inode(&args->trans, dp);
> +		if (error)
> +			goto out;
> +	}
> +
>  	/*
>  	 * If the result is small enough, push it all into the inode.
>  	 */
> -- 
> 2.7.4
>
Allison Henderson Feb. 27, 2021, 12:48 a.m. UTC | #4
On 2/25/21 8:02 PM, Darrick J. Wong wrote:
> On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote:
>> This patch hoists transaction handling in xfs_attr_node_removename to
>> 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@oracle.com>
> 
> Looks ok,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Great, thank you!
Allison

> 
> --D
> 
>> ---
>>   fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++-----------------------
>>   1 file changed, 22 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 4e6c89d..3cf76e2 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step(
>>   	struct xfs_da_args	*args,
>>   	struct xfs_da_state	*state)
>>   {
>> -	int			retval, error;
>> -	struct xfs_inode	*dp = args->dp;
>> -
>> +	int			error = 0;
>>   
>>   	/*
>>   	 * If there is an out-of-line value, de-allocate the blocks.
>> @@ -1265,25 +1263,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 +1278,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 +1291,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)
>> +			goto out;
>> +		error = xfs_defer_finish(&args->trans);
>> +		if (error)
>> +			goto out;
>> +		/*
>> +		 * Commit the Btree join operation and start a new trans.
>> +		 */
>> +		error = xfs_trans_roll_inode(&args->trans, dp);
>> +		if (error)
>> +			goto out;
>> +	}
>> +
>>   	/*
>>   	 * If the result is small enough, push it all into the inode.
>>   	 */
>> -- 
>> 2.7.4
>>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 4e6c89d..3cf76e2 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1251,9 +1251,7 @@  xfs_attr_node_remove_step(
 	struct xfs_da_args	*args,
 	struct xfs_da_state	*state)
 {
-	int			retval, error;
-	struct xfs_inode	*dp = args->dp;
-
+	int			error = 0;
 
 	/*
 	 * If there is an out-of-line value, de-allocate the blocks.
@@ -1265,25 +1263,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 +1278,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 +1291,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)
+			goto out;
+		error = xfs_defer_finish(&args->trans);
+		if (error)
+			goto out;
+		/*
+		 * Commit the Btree join operation and start a new trans.
+		 */
+		error = xfs_trans_roll_inode(&args->trans, dp);
+		if (error)
+			goto out;
+	}
+
 	/*
 	 * If the result is small enough, push it all into the inode.
 	 */