| Message ID | 20171124041008.32656-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 24.11.2017 06:10, Qu Wenruo wrote: > Since block I/O is always done in unit of 512 bytes, add BI_SECTOR_SIZE > and BI_SECTOR_SHIFT macros and to_sector() and to_bytes() for later > use. > > Although the best position to define such things should be bvec.h, and > to_sector() to_bytes() are also defined in device-mapper.h, there are > a lot of code defining their own SECTOR_SIZE and to_bytes() in > device-mapper is not using u64 (unsigned long long) but unsigned long, > which doesn't fit btrfs usage. > > So define btrfs' own macros and inlined converters. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/ctree.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index 8fc690384c58..8f6f57167661 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -3734,4 +3734,17 @@ static inline int btrfs_is_testing(struct btrfs_fs_info *fs_info) > #endif > return 0; > } > + > +#define BI_SECTOR_SHIFT (9) > +#define BI_SECTOR_SIZE (1 << BI_SECTOR_SHIFT) > + > +static inline u64 to_bytes(sector_t n) > +{ > + return ((u64)n << BI_SECTOR_SHIFT); > +} > + > +static inline sector_t to_sector(u64 n) > +{ > + return (n >> BI_SECTOR_SHIFT); > +} nit: I don't like the naming, I'd rather have something similar to what xfs has for its conversion macros i.e: XFS_FSS_TO_BB - filesystem sector to basic block (bb in this case is 512 bytes) so why not - BTRFS_B_TO_BB or BTRFS_B_TO_S and BTRFS_S_TO_B. Or something like that but just to_bytes and to_sector look a bit silly. It's not a big paint point but more of a "let's get the good ideas from other fsses" > #endif > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017年11月24日 14:53, Nikolay Borisov wrote: > > > On 24.11.2017 06:10, Qu Wenruo wrote: >> Since block I/O is always done in unit of 512 bytes, add BI_SECTOR_SIZE >> and BI_SECTOR_SHIFT macros and to_sector() and to_bytes() for later >> use. >> >> Although the best position to define such things should be bvec.h, and >> to_sector() to_bytes() are also defined in device-mapper.h, there are >> a lot of code defining their own SECTOR_SIZE and to_bytes() in >> device-mapper is not using u64 (unsigned long long) but unsigned long, >> which doesn't fit btrfs usage. >> >> So define btrfs' own macros and inlined converters. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/ctree.h | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h >> index 8fc690384c58..8f6f57167661 100644 >> --- a/fs/btrfs/ctree.h >> +++ b/fs/btrfs/ctree.h >> @@ -3734,4 +3734,17 @@ static inline int btrfs_is_testing(struct btrfs_fs_info *fs_info) >> #endif >> return 0; >> } >> + >> +#define BI_SECTOR_SHIFT (9) >> +#define BI_SECTOR_SIZE (1 << BI_SECTOR_SHIFT) >> + >> +static inline u64 to_bytes(sector_t n) >> +{ >> + return ((u64)n << BI_SECTOR_SHIFT); >> +} >> + >> +static inline sector_t to_sector(u64 n) >> +{ >> + return (n >> BI_SECTOR_SHIFT); >> +} > > nit: I don't like the naming, I'd rather have something similar to what > xfs has for its conversion macros i.e: > > XFS_FSS_TO_BB - filesystem sector to basic block (bb in this case is 512 > bytes) The problem is, without your explanation, I don't even know this macro is used to convert sectors to bytes. > > so why not - BTRFS_B_TO_BB or BTRFS_B_TO_S and BTRFS_S_TO_B. Or > something like that but just to_bytes and to_sector look a bit silly. > It's not a big paint point but more of a "let's get the good ideas from > other fsses" Yep, better ideas are always welcomed. BTW, the to_bytes() and to_sector() from device-mapper.h is the easier to understand one I can found in variant headers. (I don't even need to add any comment, unlike the initials from XFS) Thanks, Qu > >> #endif >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Fri, Nov 24, 2017 at 08:53:09AM +0200, Nikolay Borisov wrote: > On 24.11.2017 06:10, Qu Wenruo wrote: > > Since block I/O is always done in unit of 512 bytes, add BI_SECTOR_SIZE > > and BI_SECTOR_SHIFT macros and to_sector() and to_bytes() for later > > use. > > > > Although the best position to define such things should be bvec.h, and > > to_sector() to_bytes() are also defined in device-mapper.h, there are > > a lot of code defining their own SECTOR_SIZE and to_bytes() in > > device-mapper is not using u64 (unsigned long long) but unsigned long, > > which doesn't fit btrfs usage. > > > > So define btrfs' own macros and inlined converters. > > > > Signed-off-by: Qu Wenruo <wqu@suse.com> > > --- > > fs/btrfs/ctree.h | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > > index 8fc690384c58..8f6f57167661 100644 > > --- a/fs/btrfs/ctree.h > > +++ b/fs/btrfs/ctree.h > > @@ -3734,4 +3734,17 @@ static inline int btrfs_is_testing(struct btrfs_fs_info *fs_info) > > #endif > > return 0; > > } > > + > > +#define BI_SECTOR_SHIFT (9) > > +#define BI_SECTOR_SIZE (1 << BI_SECTOR_SHIFT) > > + > > +static inline u64 to_bytes(sector_t n) > > +{ > > + return ((u64)n << BI_SECTOR_SHIFT); > > +} > > + > > +static inline sector_t to_sector(u64 n) > > +{ > > + return (n >> BI_SECTOR_SHIFT); > > +} > > nit: I don't like the naming, I'd rather have something similar to what > xfs has for its conversion macros i.e: > > XFS_FSS_TO_BB - filesystem sector to basic block (bb in this case is 512 > bytes) > > so why not - BTRFS_B_TO_BB or BTRFS_B_TO_S and BTRFS_S_TO_B. Or > something like that but just to_bytes and to_sector look a bit silly. > It's not a big paint point but more of a "let's get the good ideas from > other fsses" I don't like the naming in the patch either, but I don't think we should copy the xfs naming. There are differences in the building objects, thers's nothing like a 'basic block' and thus BB in btrfs does not refer to anything we know. It's fine for xfs as this has been there since the beginning. The wordy_naming in btrfs is similar in that way and has been part of the format (names of the on-disk structures etc), so we're past the time to switch to something completely different. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 8fc690384c58..8f6f57167661 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -3734,4 +3734,17 @@ static inline int btrfs_is_testing(struct btrfs_fs_info *fs_info) #endif return 0; } + +#define BI_SECTOR_SHIFT (9) +#define BI_SECTOR_SIZE (1 << BI_SECTOR_SHIFT) + +static inline u64 to_bytes(sector_t n) +{ + return ((u64)n << BI_SECTOR_SHIFT); +} + +static inline sector_t to_sector(u64 n) +{ + return (n >> BI_SECTOR_SHIFT); +} #endif
Since block I/O is always done in unit of 512 bytes, add BI_SECTOR_SIZE and BI_SECTOR_SHIFT macros and to_sector() and to_bytes() for later use. Although the best position to define such things should be bvec.h, and to_sector() to_bytes() are also defined in device-mapper.h, there are a lot of code defining their own SECTOR_SIZE and to_bytes() in device-mapper is not using u64 (unsigned long long) but unsigned long, which doesn't fit btrfs usage. So define btrfs' own macros and inlined converters. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/ctree.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)