Re: [PATCH v2 2/2] xfs: clean up attr intent recovery error paths

[Yingjie Gao]

在 2026/6/10 13:12, Darrick J. Wong 写道:
> On Wed, Jun 10, 2026 at 10:20:28AM +0800, Yingjie Gao wrote:
>> xfs_attr_recover_work() still uses a backward goto from out_cancel to
>> out_unlock.
> 
> What's so bad about backwards gotos?
> 
> --D
> 
There is nothing wrong with backward gotos. They are perfectly clear
and correct as-is. My only thought was that since the bmap and exchmaps
recovery helpers next door both use the linear fallthrough style, it
might be nice to align this function for consistency. That said, I do
not feel strongly about it -- if you would prefer to leave the backward
goto in place, I am happy to drop patch 2 and proceed with just patch
1.

Either way, thank you for taking the time to review this.

Best regards,
Yingjie
```
>> Restructure the cleanup labels into the same linear fallthrough style
>> used by the neighboring bmap and exchmaps recovery helpers.  Initialize
>> the local error variable to zero as those helpers do as well, which
>> makes the shared return path easier to follow.
>>
>> No functional change intended.
>>
>> Signed-off-by: Yingjie Gao <gaoyingjie@xxxxxxxxxxxxx>
>> ---
>>  fs/xfs/xfs_attr_item.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
>> index 841838bc1d0f..f8aa9dd80bb9 100644
>> --- a/fs/xfs/xfs_attr_item.c
>> +++ b/fs/xfs/xfs_attr_item.c
>> @@ -739,7 +739,7 @@ xfs_attr_recover_work(
>>  	struct xfs_trans_res		resv;
>>  	struct xfs_attri_log_format	*attrp;
>>  	struct xfs_attri_log_nameval	*nv = attrip->attri_nameval;
>> -	int				error;
>> +	int				error = 0;
>>  	unsigned int			total = 0;
>>  
>>  	/*
>> @@ -789,14 +789,20 @@ xfs_attr_recover_work(
>>  		goto out_cancel;
>>  
>>  	error = xfs_defer_ops_capture_and_commit(tp, capture_list);
>> +	if (error)
>> +		goto out_unlock;
>> +
>> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>> +	xfs_irele(ip);
>> +	return 0;
>> +
>> +out_cancel:
>> +	xfs_trans_cancel(tp);
>>  out_unlock:
>>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>>  out_rele:
>>  	xfs_irele(ip);
>>  	return error;
>> -out_cancel:
>> -	xfs_trans_cancel(tp);
>> -	goto out_unlock;
>>  }
>>  
>>  /* Re-log an intent item to push the log tail forward. */
>> -- 
>> 2.20.1
>>
>>
>