Re: [PATCH v9 17/24] xfs: Add helper function xfs_attr_node_removename_setup

[Allison Collins <allison.henderson@oracle.com>]

On 5/4/20 11:58 AM, Darrick J. Wong wrote:
> On Thu, Apr 30, 2020 at 03:50:09PM -0700, Allison Collins wrote:
>> This patch adds a new helper function xfs_attr_node_removename_setup.
>> This will help modularize xfs_attr_node_removename when we add delay
>> ready attributes later.
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> Reviewed-by: Brian Foster <bfoster@redhat.com>
>> Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>
>> ---
>>   fs/xfs/libxfs/xfs_attr.c | 48 +++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 35 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index feae122..c8226c6 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1184,6 +1184,39 @@ xfs_attr_leaf_mark_incomplete(
>>   }
>>   
>>   /*
>> + * Initial setup for xfs_attr_node_removename.  Make sure the attr is there and
>> + * the blocks are valid.  Any remote blocks will be marked incomplete.
> 
> "Attr keys with remote blocks will be marked incomplete."
Ok, will fix.

> 
>> + */
>> +STATIC
>> +int xfs_attr_node_removename_setup(
>> +	struct xfs_da_args	*args,
>> +	struct xfs_da_state	**state)
>> +{
>> +	int			error;
>> +	struct xfs_da_state_blk	*blk;
>> +
>> +	error = xfs_attr_node_hasname(args, state);
>> +	if (error != -EEXIST)
>> +		return error;
>> +
>> +	blk = &(*state)->path.blk[(*state)->path.active - 1];
>> +	ASSERT(blk->bp != NULL);
>> +	ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
>> +
>> +	if (args->rmtblkno > 0) {
>> +		error = xfs_attr_leaf_mark_incomplete(args, *state);
>> +		if (error)
>> +			return error;
>> +
>> +		error = xfs_attr_rmtval_invalidate(args);
>> +		if (error)
>> +			return error;
> 
> 		return xfs_attr_rmtval_invalidate(args);
> 
> With those two things changed,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Ok, thank you!

Allison
> 
> --D
> 
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>>    * Remove a name from a B-tree attribute list.
>>    *
>>    * This will involve walking down the Btree, and may involve joining
>> @@ -1201,8 +1234,8 @@ xfs_attr_node_removename(
>>   
>>   	trace_xfs_attr_node_removename(args);
>>   
>> -	error = xfs_attr_node_hasname(args, &state);
>> -	if (error != -EEXIST)
>> +	error = xfs_attr_node_removename_setup(args, &state);
>> +	if (error)
>>   		goto out;
>>   
>>   	/*
>> @@ -1210,18 +1243,7 @@ xfs_attr_node_removename(
>>   	 * This is done before we remove the attribute so that we don't
>>   	 * overflow the maximum size of a transaction and/or hit a deadlock.
>>   	 */
>> -	blk = &state->path.blk[ state->path.active-1 ];
>> -	ASSERT(blk->bp != NULL);
>> -	ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
>>   	if (args->rmtblkno > 0) {
>> -		error = xfs_attr_leaf_mark_incomplete(args, state);
>> -		if (error)
>> -			goto out;
>> -
>> -		error = xfs_attr_rmtval_invalidate(args);
>> -		if (error)
>> -			return error;
>> -
>>   		error = xfs_attr_rmtval_remove(args);
>>   		if (error)
>>   			goto out;
>> -- 
>> 2.7.4
>>