Re: [PATCH 1/3] btrfs: Introduce macros to handle bytes and sector conversion

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2417 bytes --]



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
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]