[PATCH v2 0/2] btrfs: re-define btrfs_raid_types

[PATCH v2 0/2] btrfs: re-define btrfs_raid_types

[Qu Wenruo]

By the nature of BTRFS_BLOCK_GROUP_* profiles, converting the flag into
an index should only need one bits AND, one if () check for SINGLE
profile, one right shift to align the values, one ilog2() call which is
normally converted into ffs() assembly code.

But we're using a lot of if () branches to do the convert.

This patch will re-define btrfs_raid_types by:

- Move it to volumes.h
  btrfs_raid_types are only used internally, no need to be exposed
  through UAPI.

- Re-order btrfs_raid_types
  To make them match their value order

- Use ilog2() to convert them into index

- Inline btrfs_bg_flags_to_raid_index()
  It's just 5 assembly commands now.

Changelog:
v2:
- Fix the start value of BTRFS_RAID_RAID0

Qu Wenruo (2):
  btrfs: move definition of btrfs_raid_types to volumes.h
  btrfs: use ilog2() to replace if () branches for
    btrfs_bg_flags_to_raid_index()

 fs/btrfs/space-info.h           |  2 ++
 fs/btrfs/volumes.c              | 26 -----------------------
 fs/btrfs/volumes.h              | 37 ++++++++++++++++++++++++++++++++-
 include/uapi/linux/btrfs_tree.h | 13 ------------
 4 files changed, 38 insertions(+), 40 deletions(-)

-- 
2.33.1

[PATCH v2 1/2] btrfs: move definition of btrfs_raid_types to volumes.h

[Qu Wenruo]

It's only internally used as another way to represent btrfs profiles,
it's not exposed through any on-disk format.

Furthermore, since it's internal structure, its definition can change in
the future.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/volumes.h              | 13 +++++++++++++
 include/uapi/linux/btrfs_tree.h | 13 -------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 3b8130680749..e0c374a7c30b 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -17,6 +17,19 @@ extern struct mutex uuid_mutex;
 
 #define BTRFS_STRIPE_LEN	SZ_64K
 
+enum btrfs_raid_types {
+	BTRFS_RAID_RAID10,
+	BTRFS_RAID_RAID1,
+	BTRFS_RAID_DUP,
+	BTRFS_RAID_RAID0,
+	BTRFS_RAID_SINGLE,
+	BTRFS_RAID_RAID5,
+	BTRFS_RAID_RAID6,
+	BTRFS_RAID_RAID1C3,
+	BTRFS_RAID_RAID1C4,
+	BTRFS_NR_RAID_TYPES
+};
+
 struct btrfs_io_geometry {
 	/* remaining bytes before crossing a stripe */
 	u64 len;
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index e1c4c732aaba..819dec72f232 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -875,19 +875,6 @@ struct btrfs_dev_replace_item {
 #define BTRFS_BLOCK_GROUP_RESERVED	(BTRFS_AVAIL_ALLOC_BIT_SINGLE | \
 					 BTRFS_SPACE_INFO_GLOBAL_RSV)
 
-enum btrfs_raid_types {
-	BTRFS_RAID_RAID10,
-	BTRFS_RAID_RAID1,
-	BTRFS_RAID_DUP,
-	BTRFS_RAID_RAID0,
-	BTRFS_RAID_SINGLE,
-	BTRFS_RAID_RAID5,
-	BTRFS_RAID_RAID6,
-	BTRFS_RAID_RAID1C3,
-	BTRFS_RAID_RAID1C4,
-	BTRFS_NR_RAID_TYPES
-};
-
 #define BTRFS_BLOCK_GROUP_TYPE_MASK	(BTRFS_BLOCK_GROUP_DATA |    \
 					 BTRFS_BLOCK_GROUP_SYSTEM |  \
 					 BTRFS_BLOCK_GROUP_METADATA)
-- 
2.33.1

[PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Qu Wenruo]

In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
convert the BTRFS_BLOCK_GROUP_* bits to a index number.

But the truth is, there is really no such need for so many branches at
all.
Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to calculate
their values.

Only one fixed offset is needed to make the index sequential (the
lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved for
SINGLE).

Even with that calculation involved (one if(), one ilog2(), one minus),
it should still be way faster than the if () branches, and now it is
definitely small enough to be inlined.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/space-info.h |  2 ++
 fs/btrfs/volumes.c    | 26 --------------------------
 fs/btrfs/volumes.h    | 42 ++++++++++++++++++++++++++++++++----------
 3 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index cb5056472e79..5a0686ab9679 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -3,6 +3,8 @@
 #ifndef BTRFS_SPACE_INFO_H
 #define BTRFS_SPACE_INFO_H
 
+#include "volumes.h"
+
 struct btrfs_space_info {
 	spinlock_t lock;
 
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a8ea3f88c4db..94a3dfe709e8 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -154,32 +154,6 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
 	},
 };
 
-/*
- * Convert block group flags (BTRFS_BLOCK_GROUP_*) to btrfs_raid_types, which
- * can be used as index to access btrfs_raid_array[].
- */
-enum btrfs_raid_types __attribute_const__ btrfs_bg_flags_to_raid_index(u64 flags)
-{
-	if (flags & BTRFS_BLOCK_GROUP_RAID10)
-		return BTRFS_RAID_RAID10;
-	else if (flags & BTRFS_BLOCK_GROUP_RAID1)
-		return BTRFS_RAID_RAID1;
-	else if (flags & BTRFS_BLOCK_GROUP_RAID1C3)
-		return BTRFS_RAID_RAID1C3;
-	else if (flags & BTRFS_BLOCK_GROUP_RAID1C4)
-		return BTRFS_RAID_RAID1C4;
-	else if (flags & BTRFS_BLOCK_GROUP_DUP)
-		return BTRFS_RAID_DUP;
-	else if (flags & BTRFS_BLOCK_GROUP_RAID0)
-		return BTRFS_RAID_RAID0;
-	else if (flags & BTRFS_BLOCK_GROUP_RAID5)
-		return BTRFS_RAID_RAID5;
-	else if (flags & BTRFS_BLOCK_GROUP_RAID6)
-		return BTRFS_RAID_RAID6;
-
-	return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
-}
-
 const char *btrfs_bg_type_to_raid_name(u64 flags)
 {
 	const int index = btrfs_bg_flags_to_raid_index(flags);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index e0c374a7c30b..7038c6cee39a 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -17,19 +17,42 @@ extern struct mutex uuid_mutex;
 
 #define BTRFS_STRIPE_LEN	SZ_64K
 
+/*
+ * Here we use ilog2(BTRFS_BLOCK_GROUP_*) to convert the profile bits to
+ * an index.
+ * We reserve 0 for BTRFS_RAID_SINGLE, while the lowest profile, ilog2(RAID0),
+ * is 3, thus we need this shift to make all index numbers sequential.
+ */
+#define BTRFS_RAID_SHIFT	(ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
+
 enum btrfs_raid_types {
-	BTRFS_RAID_RAID10,
-	BTRFS_RAID_RAID1,
-	BTRFS_RAID_DUP,
-	BTRFS_RAID_RAID0,
-	BTRFS_RAID_SINGLE,
-	BTRFS_RAID_RAID5,
-	BTRFS_RAID_RAID6,
-	BTRFS_RAID_RAID1C3,
-	BTRFS_RAID_RAID1C4,
+	BTRFS_RAID_SINGLE  = 0,
+	BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >> BTRFS_RAID_SHIFT),
+	BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >> BTRFS_RAID_SHIFT),
+	BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >> BTRFS_RAID_SHIFT),
+	BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >> BTRFS_RAID_SHIFT),
+	BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >> BTRFS_RAID_SHIFT),
+	BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >> BTRFS_RAID_SHIFT),
+	BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >> BTRFS_RAID_SHIFT),
+	BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >> BTRFS_RAID_SHIFT),
 	BTRFS_NR_RAID_TYPES
 };
 
+/*
+ * Convert block group flags (BTRFS_BLOCK_GROUP_*) to btrfs_raid_types, which
+ * can be used as index to access btrfs_raid_array[].
+ */
+static inline enum btrfs_raid_types __attribute_const__
+btrfs_bg_flags_to_raid_index(u64 flags)
+{
+	u64 profile = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
+
+	if (!profile)
+		return BTRFS_RAID_SINGLE;
+
+	return ilog2(profile >> BTRFS_RAID_SHIFT);
+}
+
 struct btrfs_io_geometry {
 	/* remaining bytes before crossing a stripe */
 	u64 len;
@@ -646,7 +669,6 @@ void btrfs_scratch_superblocks(struct btrfs_fs_info *fs_info,
 			       struct block_device *bdev,
 			       const char *device_path);
 
-enum btrfs_raid_types __attribute_const__ btrfs_bg_flags_to_raid_index(u64 flags);
 int btrfs_bg_type_to_factor(u64 flags);
 const char *btrfs_bg_type_to_raid_name(u64 flags);
 int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
-- 
2.33.1

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Nikolay Borisov]

On 27.10.21 г. 8:28, Qu Wenruo wrote:
> In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
> convert the BTRFS_BLOCK_GROUP_* bits to a index number.
> 
> But the truth is, there is really no such need for so many branches at
> all.
> Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
> BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to calculate
> their values.
> 
> Only one fixed offset is needed to make the index sequential (the
> lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved for
> SINGLE).
> 
> Even with that calculation involved (one if(), one ilog2(), one minus),
> it should still be way faster than the if () branches, and now it is
> definitely small enough to be inlined.
> 

Is this used in a performance critical path, are there any numbers which
prove that it's indeed faster?

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Qu Wenruo]

On 2021/10/27 14:37, Nikolay Borisov wrote:
>
>
> On 27.10.21 г. 8:28, Qu Wenruo wrote:
>> In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
>> convert the BTRFS_BLOCK_GROUP_* bits to a index number.
>>
>> But the truth is, there is really no such need for so many branches at
>> all.
>> Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
>> BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to calculate
>> their values.
>>
>> Only one fixed offset is needed to make the index sequential (the
>> lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved for
>> SINGLE).
>>
>> Even with that calculation involved (one if(), one ilog2(), one minus),
>> it should still be way faster than the if () branches, and now it is
>> definitely small enough to be inlined.
>>
>
> Is this used in a performance critical path,

Not really in a hot path.

Most of them are called in a per block group/chunk base.

The only hotter path is in __btrfs_map_block() where if we need full
stripe, we will call btrfs_chunk_max_errors() which in turn call the
function.

That's the hottest path I can find, and even for that case it's just
per-bio base.

> are there any numbers which prove that it's indeed faster?

No real world bench for it.

But from x86_75 asm code, it's definitely smaller, with only one branch.

New:
btrfs_bg_flags_to_raid_index:
         xorl    %eax, %eax
         andl    $2040, %edi
         je      .L2499
         shrq    $2, %rdi
         movl    $-1, %eax
         bsrq %rdi,%rax
.L2499:
         ret

Old:
btrfs_bg_flags_to_raid_index:
         xorl    %eax, %eax
         testb   $64, %dil
         jne     .L429
         movl    $1, %eax
         testb   $16, %dil
         jne     .L429
         movl    $7, %eax
         testl   $512, %edi
         jne     .L429
         movl    $8, %eax
         testl   $1024, %edi
         jne     .L429
         movl    $2, %eax
         testb   $32, %dil
         jne     .L429
         movl    $3, %eax
         testb   $8, %dil
         jne     .L429
         movl    $5, %eax
         testb   $-128, %dil
         jne     .L429
         andl    $256, %edi
         cmpq    $1, %rdi
         sbbl    %eax, %eax
         andl    $-2, %eax
         addl    $6, %eax
.L429:
         ret

Which I don't really believe the older code can be any faster,
considering so many branches, and pure lines of asm.

Thanks,
Qu

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Anand Jain]

On 27/10/2021 13:28, Qu Wenruo wrote:
> In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
> convert the BTRFS_BLOCK_GROUP_* bits to a index number.
> 
> But the truth is, there is really no such need for so many branches at
> all.
> Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
> BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to calculate
> their values.
> 
> Only one fixed offset is needed to make the index sequential (the
> lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved for
> SINGLE).
> 
> Even with that calculation involved (one if(), one ilog2(), one minus),
> it should still be way faster than the if () branches, and now it is
> definitely small enough to be inlined.
> 

  Why not just use reverse static index similar to

const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
<snip>
}

Thanks, Anand

> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   fs/btrfs/space-info.h |  2 ++
>   fs/btrfs/volumes.c    | 26 --------------------------
>   fs/btrfs/volumes.h    | 42 ++++++++++++++++++++++++++++++++----------
>   3 files changed, 34 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
> index cb5056472e79..5a0686ab9679 100644
> --- a/fs/btrfs/space-info.h
> +++ b/fs/btrfs/space-info.h
> @@ -3,6 +3,8 @@
>   #ifndef BTRFS_SPACE_INFO_H
>   #define BTRFS_SPACE_INFO_H
>   
> +#include "volumes.h"
> +
>   struct btrfs_space_info {
>   	spinlock_t lock;
>   
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index a8ea3f88c4db..94a3dfe709e8 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -154,32 +154,6 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>   	},
>   };
>   
> -/*
> - * Convert block group flags (BTRFS_BLOCK_GROUP_*) to btrfs_raid_types, which
> - * can be used as index to access btrfs_raid_array[].
> - */
> -enum btrfs_raid_types __attribute_const__ btrfs_bg_flags_to_raid_index(u64 flags)
> -{
> -	if (flags & BTRFS_BLOCK_GROUP_RAID10)
> -		return BTRFS_RAID_RAID10;
> -	else if (flags & BTRFS_BLOCK_GROUP_RAID1)
> -		return BTRFS_RAID_RAID1;
> -	else if (flags & BTRFS_BLOCK_GROUP_RAID1C3)
> -		return BTRFS_RAID_RAID1C3;
> -	else if (flags & BTRFS_BLOCK_GROUP_RAID1C4)
> -		return BTRFS_RAID_RAID1C4;
> -	else if (flags & BTRFS_BLOCK_GROUP_DUP)
> -		return BTRFS_RAID_DUP;
> -	else if (flags & BTRFS_BLOCK_GROUP_RAID0)
> -		return BTRFS_RAID_RAID0;
> -	else if (flags & BTRFS_BLOCK_GROUP_RAID5)
> -		return BTRFS_RAID_RAID5;
> -	else if (flags & BTRFS_BLOCK_GROUP_RAID6)
> -		return BTRFS_RAID_RAID6;
> -
> -	return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
> -}
> -
>   const char *btrfs_bg_type_to_raid_name(u64 flags)
>   {
>   	const int index = btrfs_bg_flags_to_raid_index(flags);
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index e0c374a7c30b..7038c6cee39a 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -17,19 +17,42 @@ extern struct mutex uuid_mutex;
>   
>   #define BTRFS_STRIPE_LEN	SZ_64K
>   
> +/*
> + * Here we use ilog2(BTRFS_BLOCK_GROUP_*) to convert the profile bits to
> + * an index.
> + * We reserve 0 for BTRFS_RAID_SINGLE, while the lowest profile, ilog2(RAID0),
> + * is 3, thus we need this shift to make all index numbers sequential.
> + */
> +#define BTRFS_RAID_SHIFT	(ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
> +
>   enum btrfs_raid_types {
> -	BTRFS_RAID_RAID10,
> -	BTRFS_RAID_RAID1,
> -	BTRFS_RAID_DUP,
> -	BTRFS_RAID_RAID0,
> -	BTRFS_RAID_SINGLE,
> -	BTRFS_RAID_RAID5,
> -	BTRFS_RAID_RAID6,
> -	BTRFS_RAID_RAID1C3,
> -	BTRFS_RAID_RAID1C4,
> +	BTRFS_RAID_SINGLE  = 0,
> +	BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >> BTRFS_RAID_SHIFT),
>   	BTRFS_NR_RAID_TYPES
>   };


>   
> +/*
> + * Convert block group flags (BTRFS_BLOCK_GROUP_*) to btrfs_raid_types, which
> + * can be used as index to access btrfs_raid_array[].
> + */
> +static inline enum btrfs_raid_types __attribute_const__
> +btrfs_bg_flags_to_raid_index(u64 flags)
> +{
> +	u64 profile = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
> +
> +	if (!profile)
> +		return BTRFS_RAID_SINGLE;
> +
> +	return ilog2(profile >> BTRFS_RAID_SHIFT);
> +}
> +
>   struct btrfs_io_geometry {
>   	/* remaining bytes before crossing a stripe */
>   	u64 len;
> @@ -646,7 +669,6 @@ void btrfs_scratch_superblocks(struct btrfs_fs_info *fs_info,
>   			       struct block_device *bdev,
>   			       const char *device_path);
>   
> -enum btrfs_raid_types __attribute_const__ btrfs_bg_flags_to_raid_index(u64 flags);
>   int btrfs_bg_type_to_factor(u64 flags);
>   const char *btrfs_bg_type_to_raid_name(u64 flags);
>   int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
> 

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Qu Wenruo]

On 2021/10/27 17:23, Anand Jain wrote:
> On 27/10/2021 13:28, Qu Wenruo wrote:
>> In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
>> convert the BTRFS_BLOCK_GROUP_* bits to a index number.
>>
>> But the truth is, there is really no such need for so many branches at
>> all.
>> Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
>> BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to calculate
>> their values.
>>
>> Only one fixed offset is needed to make the index sequential (the
>> lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved for
>> SINGLE).
>>
>> Even with that calculation involved (one if(), one ilog2(), one minus),
>> it should still be way faster than the if () branches, and now it is
>> definitely small enough to be inlined.
>>
>
>   Why not just use reverse static index similar to
>
> const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
> <snip>
> }

Sorry, I didn't get the point.

Mind to share more details?

Thanks,
Qu

>
> Thanks, Anand
>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/space-info.h |  2 ++
>>   fs/btrfs/volumes.c    | 26 --------------------------
>>   fs/btrfs/volumes.h    | 42 ++++++++++++++++++++++++++++++++----------
>>   3 files changed, 34 insertions(+), 36 deletions(-)
>>
>> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
>> index cb5056472e79..5a0686ab9679 100644
>> --- a/fs/btrfs/space-info.h
>> +++ b/fs/btrfs/space-info.h
>> @@ -3,6 +3,8 @@
>>   #ifndef BTRFS_SPACE_INFO_H
>>   #define BTRFS_SPACE_INFO_H
>> +#include "volumes.h"
>> +
>>   struct btrfs_space_info {
>>       spinlock_t lock;
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index a8ea3f88c4db..94a3dfe709e8 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -154,32 +154,6 @@ const struct btrfs_raid_attr
>> btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>       },
>>   };
>> -/*
>> - * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>> btrfs_raid_types, which
>> - * can be used as index to access btrfs_raid_array[].
>> - */
>> -enum btrfs_raid_types __attribute_const__
>> btrfs_bg_flags_to_raid_index(u64 flags)
>> -{
>> -    if (flags & BTRFS_BLOCK_GROUP_RAID10)
>> -        return BTRFS_RAID_RAID10;
>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1)
>> -        return BTRFS_RAID_RAID1;
>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C3)
>> -        return BTRFS_RAID_RAID1C3;
>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C4)
>> -        return BTRFS_RAID_RAID1C4;
>> -    else if (flags & BTRFS_BLOCK_GROUP_DUP)
>> -        return BTRFS_RAID_DUP;
>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID0)
>> -        return BTRFS_RAID_RAID0;
>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID5)
>> -        return BTRFS_RAID_RAID5;
>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID6)
>> -        return BTRFS_RAID_RAID6;
>> -
>> -    return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
>> -}
>> -
>>   const char *btrfs_bg_type_to_raid_name(u64 flags)
>>   {
>>       const int index = btrfs_bg_flags_to_raid_index(flags);
>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>> index e0c374a7c30b..7038c6cee39a 100644
>> --- a/fs/btrfs/volumes.h
>> +++ b/fs/btrfs/volumes.h
>> @@ -17,19 +17,42 @@ extern struct mutex uuid_mutex;
>>   #define BTRFS_STRIPE_LEN    SZ_64K
>> +/*
>> + * Here we use ilog2(BTRFS_BLOCK_GROUP_*) to convert the profile bits to
>> + * an index.
>> + * We reserve 0 for BTRFS_RAID_SINGLE, while the lowest profile,
>> ilog2(RAID0),
>> + * is 3, thus we need this shift to make all index numbers sequential.
>> + */
>> +#define BTRFS_RAID_SHIFT    (ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
>> +
>>   enum btrfs_raid_types {
>> -    BTRFS_RAID_RAID10,
>> -    BTRFS_RAID_RAID1,
>> -    BTRFS_RAID_DUP,
>> -    BTRFS_RAID_RAID0,
>> -    BTRFS_RAID_SINGLE,
>> -    BTRFS_RAID_RAID5,
>> -    BTRFS_RAID_RAID6,
>> -    BTRFS_RAID_RAID1C3,
>> -    BTRFS_RAID_RAID1C4,
>> +    BTRFS_RAID_SINGLE  = 0,
>> +    BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >>
>> BTRFS_RAID_SHIFT),
>> +    BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >>
>> BTRFS_RAID_SHIFT),
>> +    BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >>
>> BTRFS_RAID_SHIFT),
>> +    BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >>
>> BTRFS_RAID_SHIFT),
>> +    BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >>
>> BTRFS_RAID_SHIFT),
>> +    BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >>
>> BTRFS_RAID_SHIFT),
>> +    BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >>
>> BTRFS_RAID_SHIFT),
>> +    BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >>
>> BTRFS_RAID_SHIFT),
>>       BTRFS_NR_RAID_TYPES
>>   };
>
>
>> +/*
>> + * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>> btrfs_raid_types, which
>> + * can be used as index to access btrfs_raid_array[].
>> + */
>> +static inline enum btrfs_raid_types __attribute_const__
>> +btrfs_bg_flags_to_raid_index(u64 flags)
>> +{
>> +    u64 profile = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
>> +
>> +    if (!profile)
>> +        return BTRFS_RAID_SINGLE;
>> +
>> +    return ilog2(profile >> BTRFS_RAID_SHIFT);
>> +}
>> +
>>   struct btrfs_io_geometry {
>>       /* remaining bytes before crossing a stripe */
>>       u64 len;
>> @@ -646,7 +669,6 @@ void btrfs_scratch_superblocks(struct
>> btrfs_fs_info *fs_info,
>>                      struct block_device *bdev,
>>                      const char *device_path);
>> -enum btrfs_raid_types __attribute_const__
>> btrfs_bg_flags_to_raid_index(u64 flags);
>>   int btrfs_bg_type_to_factor(u64 flags);
>>   const char *btrfs_bg_type_to_raid_name(u64 flags);
>>   int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
>>
>

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Anand Jain]

On 27/10/2021 18:41, Qu Wenruo wrote:
> 
> 
> On 2021/10/27 17:23, Anand Jain wrote:
>> On 27/10/2021 13:28, Qu Wenruo wrote:
>>> In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
>>> convert the BTRFS_BLOCK_GROUP_* bits to a index number.
>>>
>>> But the truth is, there is really no such need for so many branches at
>>> all.
>>> Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
>>> BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to calculate
>>> their values.
>>>
>>> Only one fixed offset is needed to make the index sequential (the
>>> lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved for
>>> SINGLE).
>>>
>>> Even with that calculation involved (one if(), one ilog2(), one minus),
>>> it should still be way faster than the if () branches, and now it is
>>> definitely small enough to be inlined.
>>>
>>
>>   Why not just use reverse static index similar to
>>
>> const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>> <snip>
>> }
> 
> Sorry, I didn't get the point.
> 
> Mind to share more details?
> 

Something like

/* Must match with the order of BTRFS_BLOCK_GROUP_XYZ */
+enum btrfs_raid_types {
+       BTRFS_RAID_RAID0,
+       BTRFS_RAID_RAID1,
+       BTRFS_RAID_DUP,
+       BTRFS_RAID_RAID10,
+       BTRFS_RAID_RAID5,
+       BTRFS_RAID_RAID6,
+       BTRFS_RAID_RAID1C3,
+       BTRFS_RAID_RAID1C4,
+       BTRFS_RAID_SINGLE,
+       BTRFS_NR_RAID_TYPES
+};

btrfs_bg_flags_to_raid_index(u64 flags)
{
        int ret;
	flags = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;

        ret = ilog2(flags);
        if (ret == 0)
                return  BTRFS_RAID_SINGLE;

        return ret - 3;
}

Should work. No?

Thanks, Anand

> Thanks,
> Qu
> 
>>
>> Thanks, Anand
>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>>   fs/btrfs/space-info.h |  2 ++
>>>   fs/btrfs/volumes.c    | 26 --------------------------
>>>   fs/btrfs/volumes.h    | 42 ++++++++++++++++++++++++++++++++----------
>>>   3 files changed, 34 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
>>> index cb5056472e79..5a0686ab9679 100644
>>> --- a/fs/btrfs/space-info.h
>>> +++ b/fs/btrfs/space-info.h
>>> @@ -3,6 +3,8 @@
>>>   #ifndef BTRFS_SPACE_INFO_H
>>>   #define BTRFS_SPACE_INFO_H
>>> +#include "volumes.h"
>>> +
>>>   struct btrfs_space_info {
>>>       spinlock_t lock;
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index a8ea3f88c4db..94a3dfe709e8 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -154,32 +154,6 @@ const struct btrfs_raid_attr
>>> btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>>       },
>>>   };
>>> -/*
>>> - * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>>> btrfs_raid_types, which
>>> - * can be used as index to access btrfs_raid_array[].
>>> - */
>>> -enum btrfs_raid_types __attribute_const__
>>> btrfs_bg_flags_to_raid_index(u64 flags)
>>> -{
>>> -    if (flags & BTRFS_BLOCK_GROUP_RAID10)
>>> -        return BTRFS_RAID_RAID10;
>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1)
>>> -        return BTRFS_RAID_RAID1;
>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C3)
>>> -        return BTRFS_RAID_RAID1C3;
>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C4)
>>> -        return BTRFS_RAID_RAID1C4;
>>> -    else if (flags & BTRFS_BLOCK_GROUP_DUP)
>>> -        return BTRFS_RAID_DUP;
>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID0)
>>> -        return BTRFS_RAID_RAID0;
>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID5)
>>> -        return BTRFS_RAID_RAID5;
>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID6)
>>> -        return BTRFS_RAID_RAID6;
>>> -
>>> -    return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
>>> -}
>>> -
>>>   const char *btrfs_bg_type_to_raid_name(u64 flags)
>>>   {
>>>       const int index = btrfs_bg_flags_to_raid_index(flags);
>>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>>> index e0c374a7c30b..7038c6cee39a 100644
>>> --- a/fs/btrfs/volumes.h
>>> +++ b/fs/btrfs/volumes.h
>>> @@ -17,19 +17,42 @@ extern struct mutex uuid_mutex;
>>>   #define BTRFS_STRIPE_LEN    SZ_64K
>>> +/*
>>> + * Here we use ilog2(BTRFS_BLOCK_GROUP_*) to convert the profile 
>>> bits to
>>> + * an index.
>>> + * We reserve 0 for BTRFS_RAID_SINGLE, while the lowest profile,
>>> ilog2(RAID0),
>>> + * is 3, thus we need this shift to make all index numbers sequential.
>>> + */
>>> +#define BTRFS_RAID_SHIFT    (ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
>>> +
>>>   enum btrfs_raid_types {
>>> -    BTRFS_RAID_RAID10,
>>> -    BTRFS_RAID_RAID1,
>>> -    BTRFS_RAID_DUP,
>>> -    BTRFS_RAID_RAID0,
>>> -    BTRFS_RAID_SINGLE,
>>> -    BTRFS_RAID_RAID5,
>>> -    BTRFS_RAID_RAID6,
>>> -    BTRFS_RAID_RAID1C3,
>>> -    BTRFS_RAID_RAID1C4,
>>> +    BTRFS_RAID_SINGLE  = 0,
>>> +    BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >>
>>> BTRFS_RAID_SHIFT),
>>> +    BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >>
>>> BTRFS_RAID_SHIFT),
>>> +    BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >>
>>> BTRFS_RAID_SHIFT),
>>> +    BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >>
>>> BTRFS_RAID_SHIFT),
>>> +    BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >>
>>> BTRFS_RAID_SHIFT),
>>> +    BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >>
>>> BTRFS_RAID_SHIFT),
>>> +    BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >>
>>> BTRFS_RAID_SHIFT),
>>> +    BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >>
>>> BTRFS_RAID_SHIFT),
>>>       BTRFS_NR_RAID_TYPES
>>>   };
>>
>>
>>> +/*
>>> + * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>>> btrfs_raid_types, which
>>> + * can be used as index to access btrfs_raid_array[].
>>> + */
>>> +static inline enum btrfs_raid_types __attribute_const__
>>> +btrfs_bg_flags_to_raid_index(u64 flags)
>>> +{
>>> +    u64 profile = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
>>> +
>>> +    if (!profile)
>>> +        return BTRFS_RAID_SINGLE;
>>> +
>>> +    return ilog2(profile >> BTRFS_RAID_SHIFT);
>>> +}
>>> +
>>>   struct btrfs_io_geometry {
>>>       /* remaining bytes before crossing a stripe */
>>>       u64 len;
>>> @@ -646,7 +669,6 @@ void btrfs_scratch_superblocks(struct
>>> btrfs_fs_info *fs_info,
>>>                      struct block_device *bdev,
>>>                      const char *device_path);
>>> -enum btrfs_raid_types __attribute_const__
>>> btrfs_bg_flags_to_raid_index(u64 flags);
>>>   int btrfs_bg_type_to_factor(u64 flags);
>>>   const char *btrfs_bg_type_to_raid_name(u64 flags);
>>>   int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
>>>
>>

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Qu Wenruo]

On 2021/10/28 09:04, Anand Jain wrote:
>
>
> On 27/10/2021 18:41, Qu Wenruo wrote:
>>
>>
>> On 2021/10/27 17:23, Anand Jain wrote:
>>> On 27/10/2021 13:28, Qu Wenruo wrote:
>>>> In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
>>>> convert the BTRFS_BLOCK_GROUP_* bits to a index number.
>>>>
>>>> But the truth is, there is really no such need for so many branches at
>>>> all.
>>>> Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
>>>> BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to calculate
>>>> their values.
>>>>
>>>> Only one fixed offset is needed to make the index sequential (the
>>>> lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved for
>>>> SINGLE).
>>>>
>>>> Even with that calculation involved (one if(), one ilog2(), one minus),
>>>> it should still be way faster than the if () branches, and now it is
>>>> definitely small enough to be inlined.
>>>>
>>>
>>>   Why not just use reverse static index similar to
>>>
>>> const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>> <snip>
>>> }
>>
>> Sorry, I didn't get the point.
>>
>> Mind to share more details?
>>
>
> Something like
>
> /* Must match with the order of BTRFS_BLOCK_GROUP_XYZ */
> +enum btrfs_raid_types {
> +       BTRFS_RAID_RAID0,
> +       BTRFS_RAID_RAID1,
> +       BTRFS_RAID_DUP,
> +       BTRFS_RAID_RAID10,
> +       BTRFS_RAID_RAID5,
> +       BTRFS_RAID_RAID6,
> +       BTRFS_RAID_RAID1C3,
> +       BTRFS_RAID_RAID1C4,
> +       BTRFS_RAID_SINGLE,
> +       BTRFS_NR_RAID_TYPES

OK I got your point.

But the point here is, I don't want to enum btrfs_raid_types to hide the
value and rely on us to make it in proper order.

Thus that's why my version have all the ilog2() values.

And in that case, it's just a difference between the shift value and
where we put RAID_SINGLE.

> +};
>
> btrfs_bg_flags_to_raid_index(u64 flags)
> {
>         int ret;
>      flags = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
>
>         ret = ilog2(flags);
>         if (ret == 0)
>                 return  BTRFS_RAID_SINGLE;

In fact, ilog2(0) behavior is undefined, and should be avoided.

In this case, we still need to check if (flags == 0), and just change
the shift value.

Thanks,
Qu

>
>         return ret - 3;
> }
>
> Should work. No?
>
> Thanks, Anand
>
>> Thanks,
>> Qu
>>
>>>
>>> Thanks, Anand
>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>>   fs/btrfs/space-info.h |  2 ++
>>>>   fs/btrfs/volumes.c    | 26 --------------------------
>>>>   fs/btrfs/volumes.h    | 42 ++++++++++++++++++++++++++++++++----------
>>>>   3 files changed, 34 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
>>>> index cb5056472e79..5a0686ab9679 100644
>>>> --- a/fs/btrfs/space-info.h
>>>> +++ b/fs/btrfs/space-info.h
>>>> @@ -3,6 +3,8 @@
>>>>   #ifndef BTRFS_SPACE_INFO_H
>>>>   #define BTRFS_SPACE_INFO_H
>>>> +#include "volumes.h"
>>>> +
>>>>   struct btrfs_space_info {
>>>>       spinlock_t lock;
>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>> index a8ea3f88c4db..94a3dfe709e8 100644
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -154,32 +154,6 @@ const struct btrfs_raid_attr
>>>> btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>>>       },
>>>>   };
>>>> -/*
>>>> - * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>>>> btrfs_raid_types, which
>>>> - * can be used as index to access btrfs_raid_array[].
>>>> - */
>>>> -enum btrfs_raid_types __attribute_const__
>>>> btrfs_bg_flags_to_raid_index(u64 flags)
>>>> -{
>>>> -    if (flags & BTRFS_BLOCK_GROUP_RAID10)
>>>> -        return BTRFS_RAID_RAID10;
>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1)
>>>> -        return BTRFS_RAID_RAID1;
>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C3)
>>>> -        return BTRFS_RAID_RAID1C3;
>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C4)
>>>> -        return BTRFS_RAID_RAID1C4;
>>>> -    else if (flags & BTRFS_BLOCK_GROUP_DUP)
>>>> -        return BTRFS_RAID_DUP;
>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID0)
>>>> -        return BTRFS_RAID_RAID0;
>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID5)
>>>> -        return BTRFS_RAID_RAID5;
>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID6)
>>>> -        return BTRFS_RAID_RAID6;
>>>> -
>>>> -    return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
>>>> -}
>>>> -
>>>>   const char *btrfs_bg_type_to_raid_name(u64 flags)
>>>>   {
>>>>       const int index = btrfs_bg_flags_to_raid_index(flags);
>>>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>>>> index e0c374a7c30b..7038c6cee39a 100644
>>>> --- a/fs/btrfs/volumes.h
>>>> +++ b/fs/btrfs/volumes.h
>>>> @@ -17,19 +17,42 @@ extern struct mutex uuid_mutex;
>>>>   #define BTRFS_STRIPE_LEN    SZ_64K
>>>> +/*
>>>> + * Here we use ilog2(BTRFS_BLOCK_GROUP_*) to convert the profile
>>>> bits to
>>>> + * an index.
>>>> + * We reserve 0 for BTRFS_RAID_SINGLE, while the lowest profile,
>>>> ilog2(RAID0),
>>>> + * is 3, thus we need this shift to make all index numbers sequential.
>>>> + */
>>>> +#define BTRFS_RAID_SHIFT    (ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
>>>> +
>>>>   enum btrfs_raid_types {
>>>> -    BTRFS_RAID_RAID10,
>>>> -    BTRFS_RAID_RAID1,
>>>> -    BTRFS_RAID_DUP,
>>>> -    BTRFS_RAID_RAID0,
>>>> -    BTRFS_RAID_SINGLE,
>>>> -    BTRFS_RAID_RAID5,
>>>> -    BTRFS_RAID_RAID6,
>>>> -    BTRFS_RAID_RAID1C3,
>>>> -    BTRFS_RAID_RAID1C4,
>>>> +    BTRFS_RAID_SINGLE  = 0,
>>>> +    BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >>
>>>> BTRFS_RAID_SHIFT),
>>>> +    BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >>
>>>> BTRFS_RAID_SHIFT),
>>>> +    BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >>
>>>> BTRFS_RAID_SHIFT),
>>>> +    BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >>
>>>> BTRFS_RAID_SHIFT),
>>>> +    BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >>
>>>> BTRFS_RAID_SHIFT),
>>>> +    BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >>
>>>> BTRFS_RAID_SHIFT),
>>>> +    BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >>
>>>> BTRFS_RAID_SHIFT),
>>>> +    BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >>
>>>> BTRFS_RAID_SHIFT),
>>>>       BTRFS_NR_RAID_TYPES
>>>>   };
>>>
>>>
>>>> +/*
>>>> + * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>>>> btrfs_raid_types, which
>>>> + * can be used as index to access btrfs_raid_array[].
>>>> + */
>>>> +static inline enum btrfs_raid_types __attribute_const__
>>>> +btrfs_bg_flags_to_raid_index(u64 flags)
>>>> +{
>>>> +    u64 profile = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
>>>> +
>>>> +    if (!profile)
>>>> +        return BTRFS_RAID_SINGLE;
>>>> +
>>>> +    return ilog2(profile >> BTRFS_RAID_SHIFT);
>>>> +}
>>>> +
>>>>   struct btrfs_io_geometry {
>>>>       /* remaining bytes before crossing a stripe */
>>>>       u64 len;
>>>> @@ -646,7 +669,6 @@ void btrfs_scratch_superblocks(struct
>>>> btrfs_fs_info *fs_info,
>>>>                      struct block_device *bdev,
>>>>                      const char *device_path);
>>>> -enum btrfs_raid_types __attribute_const__
>>>> btrfs_bg_flags_to_raid_index(u64 flags);
>>>>   int btrfs_bg_type_to_factor(u64 flags);
>>>>   const char *btrfs_bg_type_to_raid_name(u64 flags);
>>>>   int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
>>>>
>>>

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Anand Jain]

On 28/10/2021 15:10, Qu Wenruo wrote:
> 
> 
> On 2021/10/28 09:04, Anand Jain wrote:
>>
>>
>> On 27/10/2021 18:41, Qu Wenruo wrote:
>>>
>>>
>>> On 2021/10/27 17:23, Anand Jain wrote:
>>>> On 27/10/2021 13:28, Qu Wenruo wrote:
>>>>> In function btrfs_bg_flags_to_raid_index(), we use quite some if () to
>>>>> convert the BTRFS_BLOCK_GROUP_* bits to a index number.
>>>>>
>>>>> But the truth is, there is really no such need for so many branches at
>>>>> all.
>>>>> Since all BTRFS_BLOCK_GROUP_* flags are just one single bit set inside
>>>>> BTRFS_BLOCK_GROUP_PROFILES_MASK, we can easily use ilog2() to 
>>>>> calculate
>>>>> their values.
>>>>>
>>>>> Only one fixed offset is needed to make the index sequential (the
>>>>> lowest profile bit starts at ilog2(1 << 3) while we have 0 reserved 
>>>>> for
>>>>> SINGLE).
>>>>>
>>>>> Even with that calculation involved (one if(), one ilog2(), one 
>>>>> minus),
>>>>> it should still be way faster than the if () branches, and now it is
>>>>> definitely small enough to be inlined.
>>>>>
>>>>
>>>>   Why not just use reverse static index similar to
>>>>
>>>> const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>>> <snip>
>>>> }
>>>
>>> Sorry, I didn't get the point.
>>>
>>> Mind to share more details?
>>>
>>
>> Something like
>>
>> /* Must match with the order of BTRFS_BLOCK_GROUP_XYZ */
>> +enum btrfs_raid_types {
>> +       BTRFS_RAID_RAID0,
>> +       BTRFS_RAID_RAID1,
>> +       BTRFS_RAID_DUP,
>> +       BTRFS_RAID_RAID10,
>> +       BTRFS_RAID_RAID5,
>> +       BTRFS_RAID_RAID6,
>> +       BTRFS_RAID_RAID1C3,
>> +       BTRFS_RAID_RAID1C4,
>> +       BTRFS_RAID_SINGLE,
>> +       BTRFS_NR_RAID_TYPES
> 
> OK I got your point.
> 
> But the point here is, I don't want to enum btrfs_raid_types to hide the
> value and rely on us to make it in proper order.
> 
> Thus that's why my version have all the ilog2() values.
> 
> And in that case, it's just a difference between the shift value and
> where we put RAID_SINGLE.

  Agree. If some commit misses the order, we are in the soup.

  More comments below.

> 
>> +};
>>
>> btrfs_bg_flags_to_raid_index(u64 flags)
>> {
>>         int ret;
>>      flags = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
>>
>>         ret = ilog2(flags);
>>         if (ret == 0)
>>                 return  BTRFS_RAID_SINGLE;
> 
> In fact, ilog2(0) behavior is undefined, and should be avoided.
> 
> In this case, we still need to check if (flags == 0), and just change
> the shift value.
> 
> Thanks,
> Qu
> 
>>
>>         return ret - 3;
>> }
>>
>> Should work. No?
>>
>> Thanks, Anand
>>
>>> Thanks,
>>> Qu
>>>
>>>>
>>>> Thanks, Anand
>>>>
>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>>> ---
>>>>>   fs/btrfs/space-info.h |  2 ++
>>>>>   fs/btrfs/volumes.c    | 26 --------------------------
>>>>>   fs/btrfs/volumes.h    | 42 
>>>>> ++++++++++++++++++++++++++++++++----------
>>>>>   3 files changed, 34 insertions(+), 36 deletions(-)
>>>>>
>>>>> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
>>>>> index cb5056472e79..5a0686ab9679 100644
>>>>> --- a/fs/btrfs/space-info.h
>>>>> +++ b/fs/btrfs/space-info.h
>>>>> @@ -3,6 +3,8 @@
>>>>>   #ifndef BTRFS_SPACE_INFO_H
>>>>>   #define BTRFS_SPACE_INFO_H
>>>>> +#include "volumes.h"
>>>>> +
>>>>>   struct btrfs_space_info {
>>>>>       spinlock_t lock;
>>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>>> index a8ea3f88c4db..94a3dfe709e8 100644
>>>>> --- a/fs/btrfs/volumes.c
>>>>> +++ b/fs/btrfs/volumes.c
>>>>> @@ -154,32 +154,6 @@ const struct btrfs_raid_attr
>>>>> btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>>>>       },
>>>>>   };
>>>>> -/*
>>>>> - * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>>>>> btrfs_raid_types, which
>>>>> - * can be used as index to access btrfs_raid_array[].
>>>>> - */
>>>>> -enum btrfs_raid_types __attribute_const__
>>>>> btrfs_bg_flags_to_raid_index(u64 flags)
>>>>> -{
>>>>> -    if (flags & BTRFS_BLOCK_GROUP_RAID10)
>>>>> -        return BTRFS_RAID_RAID10;
>>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1)
>>>>> -        return BTRFS_RAID_RAID1;
>>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C3)
>>>>> -        return BTRFS_RAID_RAID1C3;
>>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID1C4)
>>>>> -        return BTRFS_RAID_RAID1C4;
>>>>> -    else if (flags & BTRFS_BLOCK_GROUP_DUP)
>>>>> -        return BTRFS_RAID_DUP;
>>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID0)
>>>>> -        return BTRFS_RAID_RAID0;
>>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID5)
>>>>> -        return BTRFS_RAID_RAID5;
>>>>> -    else if (flags & BTRFS_BLOCK_GROUP_RAID6)
>>>>> -        return BTRFS_RAID_RAID6;
>>>>> -
>>>>> -    return BTRFS_RAID_SINGLE; /* BTRFS_BLOCK_GROUP_SINGLE */
>>>>> -}
>>>>> -
>>>>>   const char *btrfs_bg_type_to_raid_name(u64 flags)
>>>>>   {
>>>>>       const int index = btrfs_bg_flags_to_raid_index(flags);
>>>>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>>>>> index e0c374a7c30b..7038c6cee39a 100644
>>>>> --- a/fs/btrfs/volumes.h
>>>>> +++ b/fs/btrfs/volumes.h
>>>>> @@ -17,19 +17,42 @@ extern struct mutex uuid_mutex;
>>>>>   #define BTRFS_STRIPE_LEN    SZ_64K
>>>>> +/*
>>>>> + * Here we use ilog2(BTRFS_BLOCK_GROUP_*) to convert the profile
>>>>> bits to
>>>>> + * an index.
>>>>> + * We reserve 0 for BTRFS_RAID_SINGLE, while the lowest profile,
>>>>> ilog2(RAID0),
>>>>> + * is 3, thus we need this shift to make all index numbers 
>>>>> sequential.
>>>>> + */
>>>>> +#define BTRFS_RAID_SHIFT    (ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
>>>>> +

David may need this to be.

#define BTRFS_BG_FLAG_TO_INDEX(f) \
	ilog2(f >> (ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1))


>>>>>   enum btrfs_raid_types {
>>>>> -    BTRFS_RAID_RAID10,
>>>>> -    BTRFS_RAID_RAID1,
>>>>> -    BTRFS_RAID_DUP,
>>>>> -    BTRFS_RAID_RAID0,
>>>>> -    BTRFS_RAID_SINGLE,
>>>>> -    BTRFS_RAID_RAID5,
>>>>> -    BTRFS_RAID_RAID6,
>>>>> -    BTRFS_RAID_RAID1C3,
>>>>> -    BTRFS_RAID_RAID1C4,
>>>>> +    BTRFS_RAID_SINGLE  = 0,
>>>>> +    BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >>
>>>>> BTRFS_RAID_SHIFT),

BTRFS_RAID_RAID0   = BTRFS_BG_FLAG_TO_INDEX(BTRFS_BLOCK_GROUP_RAID0),
::
etc.

>>>>> +    BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >>
>>>>> BTRFS_RAID_SHIFT),
>>>>> +    BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >>
>>>>> BTRFS_RAID_SHIFT),
>>>>> +    BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >>
>>>>> BTRFS_RAID_SHIFT),
>>>>> +    BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >>
>>>>> BTRFS_RAID_SHIFT),
>>>>> +    BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >>
>>>>> BTRFS_RAID_SHIFT),
>>>>> +    BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >>
>>>>> BTRFS_RAID_SHIFT),
>>>>> +    BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >>
>>>>> BTRFS_RAID_SHIFT),
>>>>>       BTRFS_NR_RAID_TYPES
>>>>>   };
>>>>
>>>>
>>>>> +/*
>>>>> + * Convert block group flags (BTRFS_BLOCK_GROUP_*) to
>>>>> btrfs_raid_types, which
>>>>> + * can be used as index to access btrfs_raid_array[].
>>>>> + */
>>>>> +static inline enum btrfs_raid_types __attribute_const__
>>>>> +btrfs_bg_flags_to_raid_index(u64 flags)
>>>>> +{
>>>>> +    u64 profile = flags & BTRFS_BLOCK_GROUP_PROFILE_MASK;
>>>>> +
>>>>> +    if (!profile)
>>>>> +        return BTRFS_RAID_SINGLE;
>>>>> +
>>>>> +    return ilog2(profile >> BTRFS_RAID_SHIFT);

return BTRFS_BG_FLAG_TO_INDEX(profile);

Otherwise looks good.

Thanks, Anand


>>>>> +}
>>>>> +
>>>>>   struct btrfs_io_geometry {
>>>>>       /* remaining bytes before crossing a stripe */
>>>>>       u64 len;
>>>>> @@ -646,7 +669,6 @@ void btrfs_scratch_superblocks(struct
>>>>> btrfs_fs_info *fs_info,
>>>>>                      struct block_device *bdev,
>>>>>                      const char *device_path);
>>>>> -enum btrfs_raid_types __attribute_const__
>>>>> btrfs_bg_flags_to_raid_index(u64 flags);
>>>>>   int btrfs_bg_type_to_factor(u64 flags);
>>>>>   const char *btrfs_bg_type_to_raid_name(u64 flags);
>>>>>   int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info);
>>>>>
>>>>

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[David Sterba]

On Wed, Oct 27, 2021 at 01:28:59PM +0800, Qu Wenruo wrote:
> +#define BTRFS_RAID_SHIFT	(ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
> +
>  enum btrfs_raid_types {
> -	BTRFS_RAID_RAID10,
> -	BTRFS_RAID_RAID1,
> -	BTRFS_RAID_DUP,
> -	BTRFS_RAID_RAID0,
> -	BTRFS_RAID_SINGLE,
> -	BTRFS_RAID_RAID5,
> -	BTRFS_RAID_RAID6,
> -	BTRFS_RAID_RAID1C3,
> -	BTRFS_RAID_RAID1C4,
> +	BTRFS_RAID_SINGLE  = 0,
> +	BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >> BTRFS_RAID_SHIFT),
> +	BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >> BTRFS_RAID_SHIFT),

Please use const_ilog2 in case all the terms in the expression are
constants that can be evaluated at the enum definition time.

I agree that deriving the indexes from the flags is safe but all the
magic around that scares me a bit.

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[Qu Wenruo]

On 2021/10/29 22:11, David Sterba wrote:
> On Wed, Oct 27, 2021 at 01:28:59PM +0800, Qu Wenruo wrote:
>> +#define BTRFS_RAID_SHIFT	(ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
>> +
>>   enum btrfs_raid_types {
>> -	BTRFS_RAID_RAID10,
>> -	BTRFS_RAID_RAID1,
>> -	BTRFS_RAID_DUP,
>> -	BTRFS_RAID_RAID0,
>> -	BTRFS_RAID_SINGLE,
>> -	BTRFS_RAID_RAID5,
>> -	BTRFS_RAID_RAID6,
>> -	BTRFS_RAID_RAID1C3,
>> -	BTRFS_RAID_RAID1C4,
>> +	BTRFS_RAID_SINGLE  = 0,
>> +	BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >> BTRFS_RAID_SHIFT),
>> +	BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >> BTRFS_RAID_SHIFT),
>> +	BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >> BTRFS_RAID_SHIFT),
>> +	BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >> BTRFS_RAID_SHIFT),
>> +	BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >> BTRFS_RAID_SHIFT),
>> +	BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >> BTRFS_RAID_SHIFT),
>> +	BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >> BTRFS_RAID_SHIFT),
>> +	BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >> BTRFS_RAID_SHIFT),
>
> Please use const_ilog2 in case all the terms in the expression are
> constants that can be evaluated at the enum definition time.

Why? Kernel ilog2() is already handling the constants.

That's the biggest difference between kernel ilog2() and btrfs-progs
ilog2().

Only in btrfs-progs we need const_ilog2().

In fact, the comment of kernel ilog2() already shows this:

/**
  * ilog2 - log base 2 of 32-bit or a 64-bit unsigned value
  * @n: parameter
  *
  * constant-capable log of base 2 calculation
  * - this can be used to initialise global variables from constant
data, hence
  * the massive ternary operator construction
  *
  * selects the appropriately-sized optimised version depending on sizeof(n)
  */

>
> I agree that deriving the indexes from the flags is safe but all the
> magic around that scares me a bit.

That's indeed a little concerning.

That's why I spare no code to make it as hard as possible to break, like
all the explicit definition of each BTRFS_RAID_* number.

To me the only possible problem in the future is someone adding two or
more profiles in one go, and possibly corrupt the BTRFS_NR_RAID_TYPES.

But all BTRFS_RAID_* number should be pretty safe.

Thanks,
Qu

Re: [PATCH v2 2/2] btrfs: use ilog2() to replace if () branches for btrfs_bg_flags_to_raid_index()

[David Sterba]

On Sat, Oct 30, 2021 at 07:38:49AM +0800, Qu Wenruo wrote:
> On 2021/10/29 22:11, David Sterba wrote:
> > On Wed, Oct 27, 2021 at 01:28:59PM +0800, Qu Wenruo wrote:
> >> +#define BTRFS_RAID_SHIFT	(ilog2(BTRFS_BLOCK_GROUP_RAID0) - 1)
> >> +
> >>   enum btrfs_raid_types {
> >> -	BTRFS_RAID_RAID10,
> >> -	BTRFS_RAID_RAID1,
> >> -	BTRFS_RAID_DUP,
> >> -	BTRFS_RAID_RAID0,
> >> -	BTRFS_RAID_SINGLE,
> >> -	BTRFS_RAID_RAID5,
> >> -	BTRFS_RAID_RAID6,
> >> -	BTRFS_RAID_RAID1C3,
> >> -	BTRFS_RAID_RAID1C4,
> >> +	BTRFS_RAID_SINGLE  = 0,
> >> +	BTRFS_RAID_RAID0   = ilog2(BTRFS_BLOCK_GROUP_RAID0 >> BTRFS_RAID_SHIFT),
> >> +	BTRFS_RAID_RAID1   = ilog2(BTRFS_BLOCK_GROUP_RAID1 >> BTRFS_RAID_SHIFT),
> >> +	BTRFS_RAID_DUP     = ilog2(BTRFS_BLOCK_GROUP_DUP >> BTRFS_RAID_SHIFT),
> >> +	BTRFS_RAID_RAID10  = ilog2(BTRFS_BLOCK_GROUP_RAID10 >> BTRFS_RAID_SHIFT),
> >> +	BTRFS_RAID_RAID5   = ilog2(BTRFS_BLOCK_GROUP_RAID5 >> BTRFS_RAID_SHIFT),
> >> +	BTRFS_RAID_RAID6   = ilog2(BTRFS_BLOCK_GROUP_RAID6 >> BTRFS_RAID_SHIFT),
> >> +	BTRFS_RAID_RAID1C3 = ilog2(BTRFS_BLOCK_GROUP_RAID1C3 >> BTRFS_RAID_SHIFT),
> >> +	BTRFS_RAID_RAID1C4 = ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >> BTRFS_RAID_SHIFT),
> >
> > Please use const_ilog2 in case all the terms in the expression are
> > constants that can be evaluated at the enum definition time.
> 
> Why? Kernel ilog2() is already handling the constants.
> 
> That's the biggest difference between kernel ilog2() and btrfs-progs
> ilog2().
> 
> Only in btrfs-progs we need const_ilog2().
> 
> In fact, the comment of kernel ilog2() already shows this:
> 
> /**
>   * ilog2 - log base 2 of 32-bit or a 64-bit unsigned value
>   * @n: parameter
>   *
>   * constant-capable log of base 2 calculation
>   * - this can be used to initialise global variables from constant
> data, hence
>   * the massive ternary operator construction
>   *
>   * selects the appropriately-sized optimised version depending on sizeof(n)
>   */

Right, ilog2 is fine.

> > I agree that deriving the indexes from the flags is safe but all the
> > magic around that scares me a bit.
> 
> That's indeed a little concerning.
> 
> That's why I spare no code to make it as hard as possible to break, like
> all the explicit definition of each BTRFS_RAID_* number.

Yeah, as somebody else commented adding a wrapper for the definitions
may be more explicit, ie. the part

	ilog2(BTRFS_BLOCK_GROUP_RAID1C4 >> BTRFS_RAID_SHIFT)

hiding that it's ilog2 and doing the shift

	DEFINE_IT(BTRFS_BLOCK_GROUP_RAID1C4)

using a macro should be all fine for the enum so it's all compile-time
constant.

> To me the only possible problem in the future is someone adding two or
> more profiles in one go, and possibly corrupt the BTRFS_NR_RAID_TYPES.

I may have two more profiles to add, I'll add them one by one anyway as
it's the cleaner.