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