[v7,17/19] xfs: Add helper function xfs_attr_leaf_mark_incomplete
diff mbox series

Message ID 20200223020611.1802-18-allison.henderson@oracle.com
State Superseded
Headers show
Series
  • xfs: Delayed Ready Attrs
Related show

Commit Message

Allison Collins Feb. 23, 2020, 2:06 a.m. UTC
This patch helps to 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@oracle.com>
---
 fs/xfs/libxfs/xfs_attr.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

Comments

Amir Goldstein Feb. 23, 2020, 1:47 p.m. UTC | #1
On Sun, Feb 23, 2020 at 4:07 AM Allison Collins
<allison.henderson@oracle.com> wrote:
>
> This patch helps to 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@oracle.com>

Found no surprises here, you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

> ---
>  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 dd935ff..b9728d1 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1416,6 +1416,36 @@ xfs_attr_node_shrink(
>  }
>
>  /*
> + * Mark an attribute entry INCOMPLETE and save pointers to the relevant buffers
> + * for later deletion of the entry.
> + */
> +STATIC int
> +xfs_attr_leaf_mark_incomplete(
> +       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
> +        */
> +       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
> @@ -1473,20 +1503,7 @@ xfs_attr_node_removename(
>         args->dac.da_state = state;
>
>         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_leaf_mark_incomplete(args, state);
>                 if (error)
>                         goto out;
>
> --
> 2.7.4
>
Allison Collins Feb. 23, 2020, 6:43 p.m. UTC | #2
On 2/23/20 6:47 AM, Amir Goldstein wrote:
> On Sun, Feb 23, 2020 at 4:07 AM Allison Collins
> <allison.henderson@oracle.com> wrote:
>>
>> This patch helps to 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@oracle.com>
> 
> Found no surprises here, you may add:
> 
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> Thanks,
> Amir.

Alrighty then.  Thank you!

Allison

> 
>> ---
>>   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 dd935ff..b9728d1 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1416,6 +1416,36 @@ xfs_attr_node_shrink(
>>   }
>>
>>   /*
>> + * Mark an attribute entry INCOMPLETE and save pointers to the relevant buffers
>> + * for later deletion of the entry.
>> + */
>> +STATIC int
>> +xfs_attr_leaf_mark_incomplete(
>> +       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
>> +        */
>> +       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
>> @@ -1473,20 +1503,7 @@ xfs_attr_node_removename(
>>          args->dac.da_state = state;
>>
>>          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_leaf_mark_incomplete(args, state);
>>                  if (error)
>>                          goto out;
>>
>> --
>> 2.7.4
>>
Dave Chinner Feb. 25, 2020, 9:31 a.m. UTC | #3
On Sat, Feb 22, 2020 at 07:06:09PM -0700, Allison Collins wrote:
> This patch helps to 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@oracle.com>

Another candidate for being at the start of this patchset.

> ---
>  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 dd935ff..b9728d1 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1416,6 +1416,36 @@ xfs_attr_node_shrink(
>  }
>  
>  /*
> + * Mark an attribute entry INCOMPLETE and save pointers to the relevant buffers
> + * for later deletion of the entry.
> + */
> +STATIC int
> +xfs_attr_leaf_mark_incomplete(
> +	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.
> +	 */

Reformat to use all 80 columns.

[ Handy vim hints, add this to your .vimrc:

" set the textwidth to 80 characters for C code
 au BufNewFile,BufRead *.c,*.h set tw=80

" set the textwidth to 68 characters for guilt commit messages
 au BufNewFile,BufRead guilt.msg.*,.gitsendemail.*,git.*,*/.git/* set tw=68

" Formatting the current paragraph according to
" the current 'textwidth' with ^J (control-j):
  imap <C-J> <c-o>gqap
   map <C-J> gqap

" highlight textwidth
set cc=+1

]

Cheers,

Dave.
Allison Collins Feb. 26, 2020, 2:17 a.m. UTC | #4
On 2/25/20 2:31 AM, Dave Chinner wrote:
> On Sat, Feb 22, 2020 at 07:06:09PM -0700, Allison Collins wrote:
>> This patch helps to 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@oracle.com>
> 
> Another candidate for being at the start of this patchset.
> 
>> ---
>>   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 dd935ff..b9728d1 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1416,6 +1416,36 @@ xfs_attr_node_shrink(
>>   }
>>   
>>   /*
>> + * Mark an attribute entry INCOMPLETE and save pointers to the relevant buffers
>> + * for later deletion of the entry.
>> + */
>> +STATIC int
>> +xfs_attr_leaf_mark_incomplete(
>> +	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.
>> +	 */
> 
> Reformat to use all 80 columns.
> 
> [ Handy vim hints, add this to your .vimrc:
> 
> " set the textwidth to 80 characters for C code
>   au BufNewFile,BufRead *.c,*.h set tw=80
> 
> " set the textwidth to 68 characters for guilt commit messages
>   au BufNewFile,BufRead guilt.msg.*,.gitsendemail.*,git.*,*/.git/* set tw=68
> 
> " Formatting the current paragraph according to
> " the current 'textwidth' with ^J (control-j):
>    imap <C-J> <c-o>gqap
>     map <C-J> gqap
> 
> " highlight textwidth
> set cc=+1
> 
> ]

Ah, alrighty, I will try these out and update the comment.

Thanks!
Allison
> 
> Cheers,
> 
> Dave.
>
Chandan Rajendra March 4, 2020, 4:37 a.m. UTC | #5
On Sunday, February 23, 2020 7:36 AM Allison Collins wrote: 
> This patch helps to 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.
>

The changes look good to me.

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
>  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 dd935ff..b9728d1 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1416,6 +1416,36 @@ xfs_attr_node_shrink(
>  }
>  
>  /*
> + * Mark an attribute entry INCOMPLETE and save pointers to the relevant buffers
> + * for later deletion of the entry.
> + */
> +STATIC int
> +xfs_attr_leaf_mark_incomplete(
> +	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
> +	 */
> +	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
> @@ -1473,20 +1503,7 @@ xfs_attr_node_removename(
>  	args->dac.da_state = state;
>  
>  	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_leaf_mark_incomplete(args, state);
>  		if (error)
>  			goto out;
>  
>

Patch
diff mbox series

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index dd935ff..b9728d1 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1416,6 +1416,36 @@  xfs_attr_node_shrink(
 }
 
 /*
+ * Mark an attribute entry INCOMPLETE and save pointers to the relevant buffers
+ * for later deletion of the entry.
+ */
+STATIC int
+xfs_attr_leaf_mark_incomplete(
+	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
+	 */
+	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
@@ -1473,20 +1503,7 @@  xfs_attr_node_removename(
 	args->dac.da_state = state;
 
 	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_leaf_mark_incomplete(args, state);
 		if (error)
 			goto out;