Re: [PATCH] xfs: remove the repeated crc verification in xfs_attr3_rmt_verify

From: kaixuxia <xiakaixu1987@gmail.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org, Kaixu Xia <kaixuxia@tencent.com>
Subject: Re: [PATCH] xfs: remove the repeated crc verification in xfs_attr3_rmt_verify
Date: Thu, 17 Sep 2020 15:03:25 +0800	[thread overview]
Message-ID: <faf768f0-8db1-0188-fbf2-73bc6538178d@gmail.com> (raw)
In-Reply-To: <20200916184545.GI7955@magnolia>

On 2020/9/17 2:45, Darrick J. Wong wrote:
> On Wed, Sep 16, 2020 at 07:19:10PM +0800, xiakaixu1987@gmail.com wrote:
>> From: Kaixu Xia <kaixuxia@tencent.com>
>>
>> We already do the crc verification before calling the xfs_attr3_rmt_verify()
> 
> Nit: The function call you're removing does not itself do crc
> verification; it merely checks that the *crc feature* is set.  This
> commit message needs to make this distinction, because at first I
> thought "Why would you remove crc verification?"
> 
> IOWs...
> 
> "We already check that the crc feature is enabled before calling
> xfs_attr3_rmt_verify(), so remove the redundant feature check in that
> function."

Yes, right! The original commit message is confusing, will update it
in the next version.

Thanks,
Kaixu
> 
> --D
> 
>> function, and just return directly for non-crc buffers, so don't need
>> to do the repeated crc verification in xfs_attr3_rmt_verify().
>>
>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
>> ---
>>  fs/xfs/libxfs/xfs_attr_remote.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
>> index 3f80cede7406..48d8e9caf86f 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.c
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
>> @@ -96,8 +96,6 @@ xfs_attr3_rmt_verify(
>>  {
>>  	struct xfs_attr3_rmt_hdr *rmt = ptr;
>>  
>> -	if (!xfs_sb_version_hascrc(&mp->m_sb))
>> -		return __this_address;
>>  	if (!xfs_verify_magic(bp, rmt->rm_magic))
>>  		return __this_address;
>>  	if (!uuid_equal(&rmt->rm_uuid, &mp->m_sb.sb_meta_uuid))
>> -- 
>> 2.20.0
>>

-- 
kaixuxia