Re: [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number

[kaixuxia <xiakaixu1987@gmail.com>]

On 2020/10/20 0:32, Eric Sandeen wrote:
> On 10/19/20 4:47 AM, xiakaixu1987@gmail.com wrote:
>> From: Kaixu Xia <kaixuxia@tencent.com>
>>
>> We use the SECTOR_SHIFT macro to define the sector size shift, so maybe
>> it is more reasonable to use it than the magic number 9.
>>
>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> 
> Hm ...  SECTOR_SHIFT is a block layer #define, really,
> and blkdev_issue_zeroout is a block layer interface I guess.
> 
> We also have our own BBSHIFT in XFS which is used elsewhere, though.
> 
> And FWIW, /many/ other fs/* manipulations still use the "- 9" today when
> converting s_blocksize_bits to sectors.  *shrug* this seems like something
> that should change tree-wide, if it's to be changed at all.
> 

Yeah, I think the magic number 9 is insecure, maybe a patchset is needed to
change them :)

Thanks,
Kaixu

> -Eric
> 
>> ---
>>  fs/xfs/xfs_bmap_util.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
>> index f2a8a0e75e1f..9f02c1824205 100644
>> --- a/fs/xfs/xfs_bmap_util.c
>> +++ b/fs/xfs/xfs_bmap_util.c
>> @@ -63,8 +63,8 @@ xfs_zero_extent(
>>  	sector_t		block = XFS_BB_TO_FSBT(mp, sector);
>>  
>>  	return blkdev_issue_zeroout(target->bt_bdev,
>> -		block << (mp->m_super->s_blocksize_bits - 9),
>> -		count_fsb << (mp->m_super->s_blocksize_bits - 9),
>> +		block << (mp->m_super->s_blocksize_bits - SECTOR_SHIFT),
>> +		count_fsb << (mp->m_super->s_blocksize_bits - SECTOR_SHIFT),
>>  		GFP_NOFS, 0);
>>  }
>>  
>>

-- 
kaixuxia