diff mbox series

xfs: use the SECTOR_SHIFT macro instead of the magic number

Message ID 1603100845-12205-2-git-send-email-kaixuxia@tencent.com (mailing list archive)
State New
Headers show
Series xfs: use the SECTOR_SHIFT macro instead of the magic number | expand

Commit Message

kaixuxia Oct. 19, 2020, 9:47 a.m. UTC
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>
---
 fs/xfs/xfs_bmap_util.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Eric Sandeen Oct. 19, 2020, 4:32 p.m. UTC | #1
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.

-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 Oct. 20, 2020, 2:54 a.m. UTC | #2
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);
>>  }
>>  
>>
diff mbox series

Patch

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);
 }