* [PATCH] xfs: remove the unused BBMASK macro
@ 2020-10-19 9:47 xiakaixu1987
2020-10-19 9:47 ` [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number xiakaixu1987
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: xiakaixu1987 @ 2020-10-19 9:47 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, Kaixu Xia
From: Kaixu Xia <kaixuxia@tencent.com>
There are no callers of the BBMASK macro, so remove it.
Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
fs/xfs/libxfs/xfs_fs.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 2a2e3cfd94f0..8fd1e20f0d73 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -847,7 +847,6 @@ struct xfs_scrub_metadata {
*/
#define BBSHIFT 9
#define BBSIZE (1<<BBSHIFT)
-#define BBMASK (BBSIZE-1)
#define BTOBB(bytes) (((__u64)(bytes) + BBSIZE - 1) >> BBSHIFT)
#define BTOBBT(bytes) ((__u64)(bytes) >> BBSHIFT)
#define BBTOB(bbs) ((bbs) << BBSHIFT)
--
2.20.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number
2020-10-19 9:47 [PATCH] xfs: remove the unused BBMASK macro xiakaixu1987
@ 2020-10-19 9:47 ` xiakaixu1987
2020-10-19 16:32 ` Eric Sandeen
2020-10-19 13:54 ` [PATCH] xfs: remove the unused BBMASK macro Eric Sandeen
2020-10-27 18:44 ` Christoph Hellwig
2 siblings, 1 reply; 9+ messages in thread
From: xiakaixu1987 @ 2020-10-19 9:47 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, Kaixu Xia
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(-)
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);
}
--
2.20.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: remove the unused BBMASK macro
2020-10-19 9:47 [PATCH] xfs: remove the unused BBMASK macro xiakaixu1987
2020-10-19 9:47 ` [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number xiakaixu1987
@ 2020-10-19 13:54 ` Eric Sandeen
2020-10-19 16:08 ` Darrick J. Wong
2020-10-20 2:59 ` kaixuxia
2020-10-27 18:44 ` Christoph Hellwig
2 siblings, 2 replies; 9+ messages in thread
From: Eric Sandeen @ 2020-10-19 13:54 UTC (permalink / raw)
To: xiakaixu1987, linux-xfs; +Cc: darrick.wong, Kaixu Xia
On 10/19/20 4:47 AM, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
>
> There are no callers of the BBMASK macro, so remove it.
>
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 2a2e3cfd94f0..8fd1e20f0d73 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -847,7 +847,6 @@ struct xfs_scrub_metadata {
> */
> #define BBSHIFT 9
> #define BBSIZE (1<<BBSHIFT)
> -#define BBMASK (BBSIZE-1)
> #define BTOBB(bytes) (((__u64)(bytes) + BBSIZE - 1) >> BBSHIFT)
> #define BTOBBT(bytes) ((__u64)(bytes) >> BBSHIFT)
> #define BBTOB(bbs) ((bbs) << BBSHIFT)
This header is shared with userspace, and the macro is used there,
though only once.
This header is also shipped as part of the "install-dev" fileset, and
defines a public API, though I don't think BBMSK is actually used
in any userspace interface.
-Eric
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: remove the unused BBMASK macro
2020-10-19 13:54 ` [PATCH] xfs: remove the unused BBMASK macro Eric Sandeen
@ 2020-10-19 16:08 ` Darrick J. Wong
2020-10-27 18:46 ` Christoph Hellwig
2020-10-20 2:59 ` kaixuxia
1 sibling, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2020-10-19 16:08 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xiakaixu1987, linux-xfs, Kaixu Xia
On Mon, Oct 19, 2020 at 08:54:28AM -0500, Eric Sandeen wrote:
> On 10/19/20 4:47 AM, xiakaixu1987@gmail.com wrote:
> > From: Kaixu Xia <kaixuxia@tencent.com>
> >
> > There are no callers of the BBMASK macro, so remove it.
> >
> > Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> > ---
> > fs/xfs/libxfs/xfs_fs.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index 2a2e3cfd94f0..8fd1e20f0d73 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -847,7 +847,6 @@ struct xfs_scrub_metadata {
> > */
> > #define BBSHIFT 9
> > #define BBSIZE (1<<BBSHIFT)
> > -#define BBMASK (BBSIZE-1)
> > #define BTOBB(bytes) (((__u64)(bytes) + BBSIZE - 1) >> BBSHIFT)
> > #define BTOBBT(bytes) ((__u64)(bytes) >> BBSHIFT)
> > #define BBTOB(bbs) ((bbs) << BBSHIFT)
>
>
> This header is shared with userspace, and the macro is used there,
> though only once.
>
> This header is also shipped as part of the "install-dev" fileset, and
> defines a public API, though I don't think BBMSK is actually used
> in any userspace interface.
$ grep BBMASK /usr/include/
/usr/include/xfs/xfs_fs.h:868:#define BBMASK (BBSIZE-1)
This ships in a user-visible header file, so it can only be removed by
going through the deprecation process.
--D
>
> -Eric
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number
2020-10-19 9:47 ` [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number xiakaixu1987
@ 2020-10-19 16:32 ` Eric Sandeen
2020-10-20 2:54 ` kaixuxia
0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2020-10-19 16:32 UTC (permalink / raw)
To: xiakaixu1987, linux-xfs; +Cc: darrick.wong, Kaixu Xia
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);
> }
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number
2020-10-19 16:32 ` Eric Sandeen
@ 2020-10-20 2:54 ` kaixuxia
0 siblings, 0 replies; 9+ messages in thread
From: kaixuxia @ 2020-10-20 2:54 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs; +Cc: darrick.wong, Kaixu Xia
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: remove the unused BBMASK macro
2020-10-19 13:54 ` [PATCH] xfs: remove the unused BBMASK macro Eric Sandeen
2020-10-19 16:08 ` Darrick J. Wong
@ 2020-10-20 2:59 ` kaixuxia
1 sibling, 0 replies; 9+ messages in thread
From: kaixuxia @ 2020-10-20 2:59 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs; +Cc: darrick.wong, Kaixu Xia
On 2020/10/19 21:54, Eric Sandeen wrote:
> On 10/19/20 4:47 AM, xiakaixu1987@gmail.com wrote:
>> From: Kaixu Xia <kaixuxia@tencent.com>
>>
>> There are no callers of the BBMASK macro, so remove it.
>>
>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
>> ---
>> fs/xfs/libxfs/xfs_fs.h | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
>> index 2a2e3cfd94f0..8fd1e20f0d73 100644
>> --- a/fs/xfs/libxfs/xfs_fs.h
>> +++ b/fs/xfs/libxfs/xfs_fs.h
>> @@ -847,7 +847,6 @@ struct xfs_scrub_metadata {
>> */
>> #define BBSHIFT 9
>> #define BBSIZE (1<<BBSHIFT)
>> -#define BBMASK (BBSIZE-1)
>> #define BTOBB(bytes) (((__u64)(bytes) + BBSIZE - 1) >> BBSHIFT)
>> #define BTOBBT(bytes) ((__u64)(bytes) >> BBSHIFT)
>> #define BBTOB(bbs) ((bbs) << BBSHIFT)
>
>
> This header is shared with userspace, and the macro is used there,
> though only once.
>
> This header is also shipped as part of the "install-dev" fileset, and
> defines a public API, though I don't think BBMSK is actually used
> in any userspace interface.
Right...I didn't consider this situation, will drop this patch.
Thanks,
Kaixu
>
> -Eric
>
--
kaixuxia
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: remove the unused BBMASK macro
2020-10-19 9:47 [PATCH] xfs: remove the unused BBMASK macro xiakaixu1987
2020-10-19 9:47 ` [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number xiakaixu1987
2020-10-19 13:54 ` [PATCH] xfs: remove the unused BBMASK macro Eric Sandeen
@ 2020-10-27 18:44 ` Christoph Hellwig
2 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2020-10-27 18:44 UTC (permalink / raw)
To: xiakaixu1987; +Cc: linux-xfs, darrick.wong, Kaixu Xia
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: remove the unused BBMASK macro
2020-10-19 16:08 ` Darrick J. Wong
@ 2020-10-27 18:46 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2020-10-27 18:46 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Eric Sandeen, xiakaixu1987, linux-xfs, Kaixu Xia
On Mon, Oct 19, 2020 at 09:08:02AM -0700, Darrick J. Wong wrote:
> $ grep BBMASK /usr/include/
> /usr/include/xfs/xfs_fs.h:868:#define BBMASK (BBSIZE-1)
>
> This ships in a user-visible header file, so it can only be removed by
> going through the deprecation process.
I don't think we had such a strong process before. Not that I'm going
to complain much.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-27 18:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-19 9:47 [PATCH] xfs: remove the unused BBMASK macro xiakaixu1987
2020-10-19 9:47 ` [PATCH] xfs: use the SECTOR_SHIFT macro instead of the magic number xiakaixu1987
2020-10-19 16:32 ` Eric Sandeen
2020-10-20 2:54 ` kaixuxia
2020-10-19 13:54 ` [PATCH] xfs: remove the unused BBMASK macro Eric Sandeen
2020-10-19 16:08 ` Darrick J. Wong
2020-10-27 18:46 ` Christoph Hellwig
2020-10-20 2:59 ` kaixuxia
2020-10-27 18:44 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.