[PATCH 00/15] btrfs-progs: simplify chunk allocation a bit

[PATCH 00/15] btrfs-progs: simplify chunk allocation a bit

[Johannes Thumshirn]

While playing a bit with the RAID code, I've come up with some cleanups for
the chunk allocatin in progs. It's not aligned to what we're doing in the
kernel, but the code has diverged so much it is a daunting task to converge it
again.

Johannes Thumshirn (15):
  btrfs-progs: simplify minimal stripe number checking
  btrfs-progs: simplify assignment of number of RAID stripes
  btrfs-progs: introduce alloc_chunk_ctl structure
  btrfs-progs: cache number of devices for chunk allocation
  btrfs-progs: pass alloc_chunk_ctl to chunk_bytes_by_type
  btrfs-progs: introduce raid profile table for chunk allocation
  btrfs-progs: consolidate assignment of minimal stripe number
  btrfs-progs: consolidate assignment of sub_stripes
  btrfs-progs: consolidate setting of RAID1 stripes
  btrfs-progs: do table lookup for simple RAID profiles' num_stripes
  btrfs-progs: consolidate num_stripes sanity check
  btrfs-progs: compactify num_stripe setting in btrfs_alloc_chunk
  btrfs-progs: introduce init_alloc_chunk_ctl
  btrfs-progs: don't pretend RAID56 has a different stripe length
  btrfs-progs: consolidate num_stripes setting for striping RAID levels

 volumes.c | 261 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 148 insertions(+), 113 deletions(-)

-- 
2.26.2

[PATCH 01/15] btrfs-progs: simplify minimal stripe number checking

[Johannes Thumshirn]

In btrfs_alloc_chunk_ctrl() we have a recurring pattern, first we assign
num stripes, then we test if num_stripes is smaller than a hardcoded
boundary and after that we set min_stripes to this magic value.

Reverse the logic by first assigning min_stripes and then testing
num_stripes against min_stripes.

This will help further refactoring.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/volumes.c b/volumes.c
index 7f84fbbaa742..089363f66473 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1054,25 +1054,25 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		}
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1) {
-		num_stripes = min_t(u64, 2,
+		min_stripes = 2;
+		num_stripes = min_t(u64, min_stripes,
 				  btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < 2)
+		if (num_stripes < min_stripes)
 			return -ENOSPC;
-		min_stripes = 2;
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1C3) {
-		num_stripes = min_t(u64, 3,
+		min_stripes = 3;
+		num_stripes = min_t(u64, min_stripes,
 				  btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < 3)
+		if (num_stripes < min_stripes)
 			return -ENOSPC;
-		min_stripes = 3;
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1C4) {
-		num_stripes = min_t(u64, 4,
+		min_stripes = 4;
+		num_stripes = min_t(u64, min_stripes,
 				  btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < 4)
+		if (num_stripes < min_stripes)
 			return -ENOSPC;
-		min_stripes = 4;
 	}
 	if (type & BTRFS_BLOCK_GROUP_DUP) {
 		num_stripes = 2;
@@ -1085,32 +1085,32 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		min_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
+		min_stripes = 4;
 		num_stripes = btrfs_super_num_devices(info->super_copy);
 		if (num_stripes > max_stripes)
 			num_stripes = max_stripes;
-		if (num_stripes < 4)
+		if (num_stripes < min_stripes)
 			return -ENOSPC;
 		num_stripes &= ~(u32)1;
 		sub_stripes = 2;
-		min_stripes = 4;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID5)) {
+		min_stripes = 2;
 		num_stripes = btrfs_super_num_devices(info->super_copy);
 		if (num_stripes > max_stripes)
 			num_stripes = max_stripes;
-		if (num_stripes < 2)
+		if (num_stripes < min_stripes)
 			return -ENOSPC;
-		min_stripes = 2;
 		stripe_len = find_raid56_stripe_len(num_stripes - 1,
 				    btrfs_super_stripesize(info->super_copy));
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID6)) {
+		min_stripes = 3;
 		num_stripes = btrfs_super_num_devices(info->super_copy);
 		if (num_stripes > max_stripes)
 			num_stripes = max_stripes;
-		if (num_stripes < 3)
+		if (num_stripes < min_stripes)
 			return -ENOSPC;
-		min_stripes = 3;
 		stripe_len = find_raid56_stripe_len(num_stripes - 2,
 				    btrfs_super_stripesize(info->super_copy));
 	}
-- 
2.26.2

Re: [PATCH 01/15] btrfs-progs: simplify minimal stripe number checking

[Qu Wenruo]

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



On 2020/6/10 下午8:32, Johannes Thumshirn wrote:
> In btrfs_alloc_chunk_ctrl() we have a recurring pattern, first we assign
> num stripes, then we test if num_stripes is smaller than a hardcoded
> boundary and after that we set min_stripes to this magic value.
> 
> Reverse the logic by first assigning min_stripes and then testing
> num_stripes against min_stripes.
> 
> This will help further refactoring.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

I'm wondering why we don't use btrfs_raid_attr::devs_min.

In fact, I'm a little surprised why use so little of that structure, but
relies on so many if branches to do the check.

Thanks,
Qu

> ---
>  volumes.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/volumes.c b/volumes.c
> index 7f84fbbaa742..089363f66473 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -1054,25 +1054,25 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  		}
>  	}
>  	if (type & BTRFS_BLOCK_GROUP_RAID1) {
> -		num_stripes = min_t(u64, 2,
> +		min_stripes = 2;
> +		num_stripes = min_t(u64, min_stripes,
>  				  btrfs_super_num_devices(info->super_copy));
> -		if (num_stripes < 2)
> +		if (num_stripes < min_stripes)
>  			return -ENOSPC;
> -		min_stripes = 2;
>  	}
>  	if (type & BTRFS_BLOCK_GROUP_RAID1C3) {
> -		num_stripes = min_t(u64, 3,
> +		min_stripes = 3;
> +		num_stripes = min_t(u64, min_stripes,
>  				  btrfs_super_num_devices(info->super_copy));
> -		if (num_stripes < 3)
> +		if (num_stripes < min_stripes)
>  			return -ENOSPC;
> -		min_stripes = 3;
>  	}
>  	if (type & BTRFS_BLOCK_GROUP_RAID1C4) {
> -		num_stripes = min_t(u64, 4,
> +		min_stripes = 4;
> +		num_stripes = min_t(u64, min_stripes,
>  				  btrfs_super_num_devices(info->super_copy));
> -		if (num_stripes < 4)
> +		if (num_stripes < min_stripes)
>  			return -ENOSPC;
> -		min_stripes = 4;
>  	}
>  	if (type & BTRFS_BLOCK_GROUP_DUP) {
>  		num_stripes = 2;
> @@ -1085,32 +1085,32 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  		min_stripes = 2;
>  	}
>  	if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
> +		min_stripes = 4;
>  		num_stripes = btrfs_super_num_devices(info->super_copy);
>  		if (num_stripes > max_stripes)
>  			num_stripes = max_stripes;
> -		if (num_stripes < 4)
> +		if (num_stripes < min_stripes)
>  			return -ENOSPC;
>  		num_stripes &= ~(u32)1;
>  		sub_stripes = 2;
> -		min_stripes = 4;
>  	}
>  	if (type & (BTRFS_BLOCK_GROUP_RAID5)) {
> +		min_stripes = 2;
>  		num_stripes = btrfs_super_num_devices(info->super_copy);
>  		if (num_stripes > max_stripes)
>  			num_stripes = max_stripes;
> -		if (num_stripes < 2)
> +		if (num_stripes < min_stripes)
>  			return -ENOSPC;
> -		min_stripes = 2;
>  		stripe_len = find_raid56_stripe_len(num_stripes - 1,
>  				    btrfs_super_stripesize(info->super_copy));
>  	}
>  	if (type & (BTRFS_BLOCK_GROUP_RAID6)) {
> +		min_stripes = 3;
>  		num_stripes = btrfs_super_num_devices(info->super_copy);
>  		if (num_stripes > max_stripes)
>  			num_stripes = max_stripes;
> -		if (num_stripes < 3)
> +		if (num_stripes < min_stripes)
>  			return -ENOSPC;
> -		min_stripes = 3;
>  		stripe_len = find_raid56_stripe_len(num_stripes - 2,
>  				    btrfs_super_stripesize(info->super_copy));
>  	}
> 


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

[PATCH 02/15] btrfs-progs: simplify assignment of number of RAID stripes

[Johannes Thumshirn]

Simplify the assignment of the used number of RAID stripes in chunk
allocation.

For RAID Levels 0, 5, 6 and 10 we first assigned it to the number of
devices in the file-system and afterwards capped it to the upper bound of
max_stripes. We can just use the max() macro for this.

This will help in furhter refactorings.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/volumes.c b/volumes.c
index 089363f66473..2a33dc09d7e7 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1079,16 +1079,14 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		min_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID0)) {
-		num_stripes = btrfs_super_num_devices(info->super_copy);
-		if (num_stripes > max_stripes)
-			num_stripes = max_stripes;
+		num_stripes = min_t(u64, max_stripes,
+				    btrfs_super_num_devices(info->super_copy));
 		min_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
 		min_stripes = 4;
-		num_stripes = btrfs_super_num_devices(info->super_copy);
-		if (num_stripes > max_stripes)
-			num_stripes = max_stripes;
+		num_stripes = min_t(u64, max_stripes,
+				    btrfs_super_num_devices(info->super_copy));
 		if (num_stripes < min_stripes)
 			return -ENOSPC;
 		num_stripes &= ~(u32)1;
@@ -1096,9 +1094,8 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID5)) {
 		min_stripes = 2;
-		num_stripes = btrfs_super_num_devices(info->super_copy);
-		if (num_stripes > max_stripes)
-			num_stripes = max_stripes;
+		num_stripes = min_t(u64, max_stripes,
+				    btrfs_super_num_devices(info->super_copy));
 		if (num_stripes < min_stripes)
 			return -ENOSPC;
 		stripe_len = find_raid56_stripe_len(num_stripes - 1,
@@ -1106,9 +1103,8 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID6)) {
 		min_stripes = 3;
-		num_stripes = btrfs_super_num_devices(info->super_copy);
-		if (num_stripes > max_stripes)
-			num_stripes = max_stripes;
+		num_stripes = min_t(u64, max_stripes,
+				    btrfs_super_num_devices(info->super_copy));
 		if (num_stripes < min_stripes)
 			return -ENOSPC;
 		stripe_len = find_raid56_stripe_len(num_stripes - 2,
-- 
2.26.2

[PATCH 03/15] btrfs-progs: introduce alloc_chunk_ctl structure

[Johannes Thumshirn]

Introduce an alloc_chunk_ctl structure, which holds parameters to control
chunk allocation.

Furthermore use this throughout btrfs_alloc_chunk().

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 140 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 76 insertions(+), 64 deletions(-)

diff --git a/volumes.c b/volumes.c
index 2a33dc09d7e7..ea3e105859da 100644
--- a/volumes.c
+++ b/volumes.c
@@ -148,6 +148,16 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
 	},
 };
 
+struct alloc_chunk_ctl {
+	enum btrfs_raid_types type;
+	int num_stripes;
+	int max_stripes;
+	int min_stripes;
+	int sub_stripes;
+	int stripe_len;
+	int total_devs;
+};
+
 struct stripe {
 	struct btrfs_device *dev;
 	u64 physical;
@@ -1016,14 +1026,10 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	u64 avail = 0;
 	u64 max_avail = 0;
 	u64 percent_max;
-	int num_stripes = 1;
-	int max_stripes = 0;
-	int min_stripes = 1;
-	int sub_stripes = 1;
+	struct alloc_chunk_ctl ctl;
 	int looped = 0;
 	int ret;
 	int index;
-	int stripe_len = BTRFS_STRIPE_LEN;
 	struct btrfs_key key;
 	u64 offset;
 
@@ -1031,17 +1037,23 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		return -ENOSPC;
 	}
 
+	ctl.num_stripes = 1;
+	ctl.max_stripes = 0;
+	ctl.min_stripes = 1;
+	ctl.sub_stripes = 1;
+	ctl.stripe_len = BTRFS_STRIPE_LEN;
+
 	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
 		if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
 			calc_size = SZ_8M;
 			max_chunk_size = calc_size * 2;
 			min_stripe_size = SZ_1M;
-			max_stripes = BTRFS_MAX_DEVS_SYS_CHUNK;
+			ctl.max_stripes = BTRFS_MAX_DEVS_SYS_CHUNK;
 		} else if (type & BTRFS_BLOCK_GROUP_DATA) {
 			calc_size = SZ_1G;
 			max_chunk_size = 10 * calc_size;
 			min_stripe_size = SZ_64M;
-			max_stripes = BTRFS_MAX_DEVS(info);
+			ctl.max_stripes = BTRFS_MAX_DEVS(info);
 		} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
 			/* for larger filesystems, use larger metadata chunks */
 			if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
@@ -1050,64 +1062,64 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 				max_chunk_size = SZ_256M;
 			calc_size = max_chunk_size;
 			min_stripe_size = SZ_32M;
-			max_stripes = BTRFS_MAX_DEVS(info);
+			ctl.max_stripes = BTRFS_MAX_DEVS(info);
 		}
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1) {
-		min_stripes = 2;
-		num_stripes = min_t(u64, min_stripes,
+		ctl.min_stripes = 2;
+		ctl.num_stripes = min_t(u64, ctl.min_stripes,
 				  btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < min_stripes)
+		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1C3) {
-		min_stripes = 3;
-		num_stripes = min_t(u64, min_stripes,
+		ctl.min_stripes = 3;
+		ctl.num_stripes = min_t(u64, ctl.min_stripes,
 				  btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < min_stripes)
+		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1C4) {
-		min_stripes = 4;
-		num_stripes = min_t(u64, min_stripes,
+		ctl.min_stripes = 4;
+		ctl.num_stripes = min_t(u64, ctl.min_stripes,
 				  btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < min_stripes)
+		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
 	if (type & BTRFS_BLOCK_GROUP_DUP) {
-		num_stripes = 2;
-		min_stripes = 2;
+		ctl.num_stripes = 2;
+		ctl.min_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID0)) {
-		num_stripes = min_t(u64, max_stripes,
+		ctl.num_stripes = min_t(u64, ctl.max_stripes,
 				    btrfs_super_num_devices(info->super_copy));
-		min_stripes = 2;
+		ctl.min_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
-		min_stripes = 4;
-		num_stripes = min_t(u64, max_stripes,
+		ctl.min_stripes = 4;
+		ctl.num_stripes = min_t(u64, ctl.max_stripes,
 				    btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < min_stripes)
+		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
-		num_stripes &= ~(u32)1;
-		sub_stripes = 2;
+		ctl.num_stripes &= ~(u32)1;
+		ctl.sub_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID5)) {
-		min_stripes = 2;
-		num_stripes = min_t(u64, max_stripes,
+		ctl.min_stripes = 2;
+		ctl.num_stripes = min_t(u64, ctl.max_stripes,
 				    btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < min_stripes)
+		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
-		stripe_len = find_raid56_stripe_len(num_stripes - 1,
+		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 1,
 				    btrfs_super_stripesize(info->super_copy));
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID6)) {
-		min_stripes = 3;
-		num_stripes = min_t(u64, max_stripes,
+		ctl.min_stripes = 3;
+		ctl.num_stripes = min_t(u64, ctl.max_stripes,
 				    btrfs_super_num_devices(info->super_copy));
-		if (num_stripes < min_stripes)
+		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
-		stripe_len = find_raid56_stripe_len(num_stripes - 2,
+		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 2,
 				    btrfs_super_stripesize(info->super_copy));
 	}
 
@@ -1116,18 +1128,18 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	max_chunk_size = min(percent_max, max_chunk_size);
 
 again:
-	if (chunk_bytes_by_type(type, calc_size, num_stripes, sub_stripes) >
-	    max_chunk_size) {
+	if (chunk_bytes_by_type(type, calc_size, ctl.num_stripes,
+				ctl.sub_stripes) > max_chunk_size) {
 		calc_size = max_chunk_size;
-		calc_size /= num_stripes;
-		calc_size /= stripe_len;
-		calc_size *= stripe_len;
+		calc_size /= ctl.num_stripes;
+		calc_size /= ctl.stripe_len;
+		calc_size *= ctl.stripe_len;
 	}
 	/* we don't want tiny stripes */
 	calc_size = max_t(u64, calc_size, min_stripe_size);
 
-	calc_size /= stripe_len;
-	calc_size *= stripe_len;
+	calc_size /= ctl.stripe_len;
+	calc_size *= ctl.stripe_len;
 	INIT_LIST_HEAD(&private_devs);
 	cur = dev_list->next;
 	index = 0;
@@ -1138,7 +1150,7 @@ again:
 		min_free = calc_size;
 
 	/* build a private list of devices we will allocate from */
-	while(index < num_stripes) {
+	while(index < ctl.num_stripes) {
 		device = list_entry(cur, struct btrfs_device, dev_list);
 		ret = btrfs_device_avail_bytes(trans, device, &avail);
 		if (ret)
@@ -1154,13 +1166,13 @@ again:
 		if (cur == dev_list)
 			break;
 	}
-	if (index < num_stripes) {
+	if (index < ctl.num_stripes) {
 		list_splice(&private_devs, dev_list);
-		if (index >= min_stripes) {
-			num_stripes = index;
+		if (index >= ctl.min_stripes) {
+			ctl.num_stripes = index;
 			if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
-				num_stripes /= sub_stripes;
-				num_stripes *= sub_stripes;
+				ctl.num_stripes /= ctl.sub_stripes;
+				ctl.num_stripes *= ctl.sub_stripes;
 			}
 			looped = 1;
 			goto again;
@@ -1179,11 +1191,11 @@ again:
 	key.type = BTRFS_CHUNK_ITEM_KEY;
 	key.offset = offset;
 
-	chunk = kmalloc(btrfs_chunk_item_size(num_stripes), GFP_NOFS);
+	chunk = kmalloc(btrfs_chunk_item_size(ctl.num_stripes), GFP_NOFS);
 	if (!chunk)
 		return -ENOMEM;
 
-	map = kmalloc(btrfs_map_lookup_size(num_stripes), GFP_NOFS);
+	map = kmalloc(btrfs_map_lookup_size(ctl.num_stripes), GFP_NOFS);
 	if (!map) {
 		kfree(chunk);
 		return -ENOMEM;
@@ -1191,9 +1203,9 @@ again:
 
 	stripes = &chunk->stripe;
 	*num_bytes = chunk_bytes_by_type(type, calc_size,
-					 num_stripes, sub_stripes);
+					 ctl.num_stripes, ctl.sub_stripes);
 	index = 0;
-	while(index < num_stripes) {
+	while(index < ctl.num_stripes) {
 		struct btrfs_stripe *stripe;
 		BUG_ON(list_empty(&private_devs));
 		cur = private_devs.next;
@@ -1201,7 +1213,7 @@ again:
 
 		/* loop over this device again if we're doing a dup group */
 		if (!(type & BTRFS_BLOCK_GROUP_DUP) ||
-		    (index == num_stripes - 1))
+		    (index == ctl.num_stripes - 1))
 			list_move(&device->dev_list, dev_list);
 
 		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
@@ -1227,23 +1239,23 @@ again:
 	/* key was set above */
 	btrfs_set_stack_chunk_length(chunk, *num_bytes);
 	btrfs_set_stack_chunk_owner(chunk, extent_root->root_key.objectid);
-	btrfs_set_stack_chunk_stripe_len(chunk, stripe_len);
+	btrfs_set_stack_chunk_stripe_len(chunk, ctl.stripe_len);
 	btrfs_set_stack_chunk_type(chunk, type);
-	btrfs_set_stack_chunk_num_stripes(chunk, num_stripes);
-	btrfs_set_stack_chunk_io_align(chunk, stripe_len);
-	btrfs_set_stack_chunk_io_width(chunk, stripe_len);
+	btrfs_set_stack_chunk_num_stripes(chunk, ctl.num_stripes);
+	btrfs_set_stack_chunk_io_align(chunk, ctl.stripe_len);
+	btrfs_set_stack_chunk_io_width(chunk, ctl.stripe_len);
 	btrfs_set_stack_chunk_sector_size(chunk, info->sectorsize);
-	btrfs_set_stack_chunk_sub_stripes(chunk, sub_stripes);
+	btrfs_set_stack_chunk_sub_stripes(chunk, ctl.sub_stripes);
 	map->sector_size = info->sectorsize;
-	map->stripe_len = stripe_len;
-	map->io_align = stripe_len;
-	map->io_width = stripe_len;
+	map->stripe_len = ctl.stripe_len;
+	map->io_align = ctl.stripe_len;
+	map->io_width = ctl.stripe_len;
 	map->type = type;
-	map->num_stripes = num_stripes;
-	map->sub_stripes = sub_stripes;
+	map->num_stripes = ctl.num_stripes;
+	map->sub_stripes = ctl.sub_stripes;
 
 	ret = btrfs_insert_item(trans, chunk_root, &key, chunk,
-				btrfs_chunk_item_size(num_stripes));
+				btrfs_chunk_item_size(ctl.num_stripes));
 	BUG_ON(ret);
 	*start = key.offset;;
 
@@ -1256,7 +1268,7 @@ again:
 
 	if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
 		ret = btrfs_add_system_chunk(info, &key,
-				    chunk, btrfs_chunk_item_size(num_stripes));
+			    chunk, btrfs_chunk_item_size(ctl.num_stripes));
 		if (ret < 0)
 			goto out_chunk;
 	}
-- 
2.26.2

[PATCH 04/15] btrfs-progs: cache number of devices for chunk allocation

[Johannes Thumshirn]

Cache the total number of devices usable for chunk allocation in
alloc_chunk_ctl instread of reading it from the super-block over and over
again.

As it's a) unlikely to have more than 4 billion devices and the result of
the max_t() gets truncated to int anyways, change the max_t calls to
simple max(), while we're at it.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/volumes.c b/volumes.c
index ea3e105859da..539c3d8648c6 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1042,6 +1042,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	ctl.min_stripes = 1;
 	ctl.sub_stripes = 1;
 	ctl.stripe_len = BTRFS_STRIPE_LEN;
+	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
 
 	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
 		if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
@@ -1067,22 +1068,19 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1) {
 		ctl.min_stripes = 2;
-		ctl.num_stripes = min_t(u64, ctl.min_stripes,
-				  btrfs_super_num_devices(info->super_copy));
+		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1C3) {
 		ctl.min_stripes = 3;
-		ctl.num_stripes = min_t(u64, ctl.min_stripes,
-				  btrfs_super_num_devices(info->super_copy));
+		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
 	if (type & BTRFS_BLOCK_GROUP_RAID1C4) {
 		ctl.min_stripes = 4;
-		ctl.num_stripes = min_t(u64, ctl.min_stripes,
-				  btrfs_super_num_devices(info->super_copy));
+		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
@@ -1091,14 +1089,12 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		ctl.min_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID0)) {
-		ctl.num_stripes = min_t(u64, ctl.max_stripes,
-				    btrfs_super_num_devices(info->super_copy));
+		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		ctl.min_stripes = 2;
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
 		ctl.min_stripes = 4;
-		ctl.num_stripes = min_t(u64, ctl.max_stripes,
-				    btrfs_super_num_devices(info->super_copy));
+		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 		ctl.num_stripes &= ~(u32)1;
@@ -1106,8 +1102,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID5)) {
 		ctl.min_stripes = 2;
-		ctl.num_stripes = min_t(u64, ctl.max_stripes,
-				    btrfs_super_num_devices(info->super_copy));
+		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 1,
@@ -1115,8 +1110,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 	if (type & (BTRFS_BLOCK_GROUP_RAID6)) {
 		ctl.min_stripes = 3;
-		ctl.num_stripes = min_t(u64, ctl.max_stripes,
-				    btrfs_super_num_devices(info->super_copy));
+		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 2,
-- 
2.26.2

[PATCH 05/15] btrfs-progs: pass alloc_chunk_ctl to chunk_bytes_by_type

[Johannes Thumshirn]

Pass the whole alloc_chunk_ctl to chunk_bytes_by_type instead of its
num_stripes and sub_stripes members.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/volumes.c b/volumes.c
index 539c3d8648c6..04bc3d19a025 100644
--- a/volumes.c
+++ b/volumes.c
@@ -883,21 +883,21 @@ int btrfs_add_system_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 	return 0;
 }
 
-static u64 chunk_bytes_by_type(u64 type, u64 calc_size, int num_stripes,
-			       int sub_stripes)
+static u64 chunk_bytes_by_type(u64 type, u64 calc_size,
+			       struct alloc_chunk_ctl *ctl)
 {
 	if (type & (BTRFS_BLOCK_GROUP_RAID1 | BTRFS_BLOCK_GROUP_DUP))
 		return calc_size;
 	else if (type & (BTRFS_BLOCK_GROUP_RAID1C3 | BTRFS_BLOCK_GROUP_RAID1C4))
 		return calc_size;
 	else if (type & BTRFS_BLOCK_GROUP_RAID10)
-		return calc_size * (num_stripes / sub_stripes);
+		return calc_size * (ctl->num_stripes / ctl->sub_stripes);
 	else if (type & BTRFS_BLOCK_GROUP_RAID5)
-		return calc_size * (num_stripes - 1);
+		return calc_size * (ctl->num_stripes - 1);
 	else if (type & BTRFS_BLOCK_GROUP_RAID6)
-		return calc_size * (num_stripes - 2);
+		return calc_size * (ctl->num_stripes - 2);
 	else
-		return calc_size * num_stripes;
+		return calc_size * ctl->num_stripes;
 }
 
 
@@ -1122,8 +1122,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	max_chunk_size = min(percent_max, max_chunk_size);
 
 again:
-	if (chunk_bytes_by_type(type, calc_size, ctl.num_stripes,
-				ctl.sub_stripes) > max_chunk_size) {
+	if (chunk_bytes_by_type(type, calc_size, &ctl) > max_chunk_size) {
 		calc_size = max_chunk_size;
 		calc_size /= ctl.num_stripes;
 		calc_size /= ctl.stripe_len;
@@ -1196,8 +1195,7 @@ again:
 	}
 
 	stripes = &chunk->stripe;
-	*num_bytes = chunk_bytes_by_type(type, calc_size,
-					 ctl.num_stripes, ctl.sub_stripes);
+	*num_bytes = chunk_bytes_by_type(type, calc_size, &ctl);
 	index = 0;
 	while(index < ctl.num_stripes) {
 		struct btrfs_stripe *stripe;
-- 
2.26.2

[PATCH 06/15] btrfs-progs: introduce raid profile table for chunk allocation

[Johannes Thumshirn]

Introduce a table holding the paramenters for chunk allocation per RAID
profile.

Also convert all assignments of hardcoded numbers to table lookups in this
process. Further changes will reduce code duplication even more.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 95 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 79 insertions(+), 16 deletions(-)

diff --git a/volumes.c b/volumes.c
index 04bc3d19a025..fc14283db2bb 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1005,6 +1005,68 @@ error:
 				- 2 * sizeof(struct btrfs_chunk))	\
 				/ sizeof(struct btrfs_stripe) + 1)
 
+static const struct btrfs_raid_profile {
+	int	num_stripes;
+	int	max_stripes;
+	int	min_stripes;
+	int	sub_stripes;
+} btrfs_raid_profile_table[BTRFS_NR_RAID_TYPES] = {
+	[BTRFS_RAID_RAID10] = {
+		.num_stripes = 0,
+		.max_stripes = 0,
+		.min_stripes = 4,
+		.sub_stripes = 2,
+	},
+	[BTRFS_RAID_RAID1] = {
+		.num_stripes = 0,
+		.max_stripes = 0,
+		.min_stripes = 2,
+		.sub_stripes = 1,
+	},
+	[BTRFS_RAID_RAID1C3] = {
+		.num_stripes = 0,
+		.max_stripes = 0,
+		.min_stripes = 3,
+		.sub_stripes = 1,
+	},
+	[BTRFS_RAID_RAID1C4] = {
+		.num_stripes = 0,
+		.max_stripes = 0,
+		.min_stripes = 4,
+		.sub_stripes = 1,
+	},
+	[BTRFS_RAID_DUP] = {
+		.num_stripes = 2,
+		.max_stripes = 0,
+		.min_stripes = 2,
+		.sub_stripes = 1,
+	},
+	[BTRFS_RAID_RAID0] = {
+		.num_stripes = 0,
+		.max_stripes = 0,
+		.min_stripes = 2,
+		.sub_stripes = 1,
+	},
+	[BTRFS_RAID_SINGLE] = {
+		.num_stripes = 1,
+		.max_stripes = 0,
+		.min_stripes = 1,
+		.sub_stripes = 1,
+	},
+	[BTRFS_RAID_RAID5] = {
+		.num_stripes = 0,
+		.max_stripes = 0,
+		.min_stripes = 2,
+		.sub_stripes = 1,
+	},
+	[BTRFS_RAID_RAID6] = {
+		.num_stripes = 0,
+		.max_stripes = 0,
+		.min_stripes = 3,
+		.sub_stripes = 1,
+	},
+};
+
 int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		      struct btrfs_fs_info *info, u64 *start,
 		      u64 *num_bytes, u64 type)
@@ -1037,6 +1099,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		return -ENOSPC;
 	}
 
+	ctl.type = btrfs_bg_flags_to_raid_index(type);
 	ctl.num_stripes = 1;
 	ctl.max_stripes = 0;
 	ctl.min_stripes = 1;
@@ -1066,50 +1129,50 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 			ctl.max_stripes = BTRFS_MAX_DEVS(info);
 		}
 	}
-	if (type & BTRFS_BLOCK_GROUP_RAID1) {
-		ctl.min_stripes = 2;
+	if (ctl.type == BTRFS_RAID_RAID1) {
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
-	if (type & BTRFS_BLOCK_GROUP_RAID1C3) {
-		ctl.min_stripes = 3;
+	if (ctl.type == BTRFS_RAID_RAID1C3) {
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
-	if (type & BTRFS_BLOCK_GROUP_RAID1C4) {
-		ctl.min_stripes = 4;
+	if (ctl.type == BTRFS_RAID_RAID1C4) {
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
-	if (type & BTRFS_BLOCK_GROUP_DUP) {
+	if (ctl.type == BTRFS_RAID_DUP) {
 		ctl.num_stripes = 2;
-		ctl.min_stripes = 2;
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 	}
-	if (type & (BTRFS_BLOCK_GROUP_RAID0)) {
+	if (ctl.type == BTRFS_RAID_RAID0) {
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		ctl.min_stripes = 2;
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 	}
-	if (type & (BTRFS_BLOCK_GROUP_RAID10)) {
-		ctl.min_stripes = 4;
+	if (ctl.type == BTRFS_RAID_RAID10) {
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 		ctl.num_stripes &= ~(u32)1;
 		ctl.sub_stripes = 2;
 	}
-	if (type & (BTRFS_BLOCK_GROUP_RAID5)) {
-		ctl.min_stripes = 2;
+	if (ctl.type == BTRFS_RAID_RAID5) {
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 1,
 				    btrfs_super_stripesize(info->super_copy));
 	}
-	if (type & (BTRFS_BLOCK_GROUP_RAID6)) {
-		ctl.min_stripes = 3;
+	if (ctl.type == BTRFS_RAID_RAID6) {
+		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
-- 
2.26.2

Re: [PATCH 06/15] btrfs-progs: introduce raid profile table for chunk allocation

[David Sterba]

On Wed, Jun 10, 2020 at 09:32:49PM +0900, Johannes Thumshirn wrote:
> Introduce a table holding the paramenters for chunk allocation per RAID
> profile.
> 
> Also convert all assignments of hardcoded numbers to table lookups in this
> process. Further changes will reduce code duplication even more.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  volumes.c | 95 +++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 79 insertions(+), 16 deletions(-)
> 
> diff --git a/volumes.c b/volumes.c
> index 04bc3d19a025..fc14283db2bb 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -1005,6 +1005,68 @@ error:
>  				- 2 * sizeof(struct btrfs_chunk))	\
>  				/ sizeof(struct btrfs_stripe) + 1)
>  
> +static const struct btrfs_raid_profile {
> +	int	num_stripes;
> +	int	max_stripes;
> +	int	min_stripes;
> +	int	sub_stripes;
> +} btrfs_raid_profile_table[BTRFS_NR_RAID_TYPES] = {
> +	[BTRFS_RAID_RAID10] = {
> +		.num_stripes = 0,
> +		.max_stripes = 0,
> +		.min_stripes = 4,
> +		.sub_stripes = 2,
> +	},
...

This duplicates btrfs_raid_array values, the member variable names don't
match so this makes it hard to compare. I wouldn't mind to create this
table as an intermediate step to clean up the code but in the end we
really don't want to keep btrfs_raid_profile, in the same file even.

Re: [PATCH 06/15] btrfs-progs: introduce raid profile table for chunk allocation

[Johannes Thumshirn]

On 11/06/2020 15:40, David Sterba wrote:
> On Wed, Jun 10, 2020 at 09:32:49PM +0900, Johannes Thumshirn wrote:
>> Introduce a table holding the paramenters for chunk allocation per RAID
>> profile.
>>
>> Also convert all assignments of hardcoded numbers to table lookups in this
>> process. Further changes will reduce code duplication even more.
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> ---
>>  volumes.c | 95 +++++++++++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 79 insertions(+), 16 deletions(-)
>>
>> diff --git a/volumes.c b/volumes.c
>> index 04bc3d19a025..fc14283db2bb 100644
>> --- a/volumes.c
>> +++ b/volumes.c
>> @@ -1005,6 +1005,68 @@ error:
>>  				- 2 * sizeof(struct btrfs_chunk))	\
>>  				/ sizeof(struct btrfs_stripe) + 1)
>>  
>> +static const struct btrfs_raid_profile {
>> +	int	num_stripes;
>> +	int	max_stripes;
>> +	int	min_stripes;
>> +	int	sub_stripes;
>> +} btrfs_raid_profile_table[BTRFS_NR_RAID_TYPES] = {
>> +	[BTRFS_RAID_RAID10] = {
>> +		.num_stripes = 0,
>> +		.max_stripes = 0,
>> +		.min_stripes = 4,
>> +		.sub_stripes = 2,
>> +	},
> ...
> 
> This duplicates btrfs_raid_array values, the member variable names don't
> match so this makes it hard to compare. I wouldn't mind to create this
> table as an intermediate step to clean up the code but in the end we
> really don't want to keep btrfs_raid_profile, in the same file even.
> 

Yes I got confused in the middle and opted for this intermediate step. 
I hope I can reduce it even more and have it slowly converged to the 
kernel's implementation.

[PATCH 07/15] btrfs-progs: consolidate assignment of minimal stripe number

[Johannes Thumshirn]

Now that we have a table holding the min_stripes value we can consolidate
all setting of alloc_chunk_ctl::min_stripes to a signle table lookup.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/volumes.c b/volumes.c
index fc14283db2bb..9076f055f6bd 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1102,7 +1102,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	ctl.type = btrfs_bg_flags_to_raid_index(type);
 	ctl.num_stripes = 1;
 	ctl.max_stripes = 0;
-	ctl.min_stripes = 1;
+	ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 	ctl.sub_stripes = 1;
 	ctl.stripe_len = BTRFS_STRIPE_LEN;
 	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
@@ -1149,7 +1149,6 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 	if (ctl.type == BTRFS_RAID_DUP) {
 		ctl.num_stripes = 2;
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 	}
 	if (ctl.type == BTRFS_RAID_RAID0) {
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-- 
2.26.2

[PATCH 08/15] btrfs-progs: consolidate assignment of sub_stripes

[Johannes Thumshirn]

Now that we have a table holding the sub_stripes value we can consolidate
all setting of alloc_chunk_ctl::sub_stripes to a signle table lookup.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/volumes.c b/volumes.c
index 9076f055f6bd..32d3dfe0decd 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1103,7 +1103,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	ctl.num_stripes = 1;
 	ctl.max_stripes = 0;
 	ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
-	ctl.sub_stripes = 1;
+	ctl.sub_stripes = btrfs_raid_profile_table[ctl.type].sub_stripes;
 	ctl.stripe_len = BTRFS_STRIPE_LEN;
 	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
 
@@ -1160,7 +1160,6 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 		ctl.num_stripes &= ~(u32)1;
-		ctl.sub_stripes = 2;
 	}
 	if (ctl.type == BTRFS_RAID_RAID5) {
 		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
-- 
2.26.2

[PATCH 09/15] btrfs-progs: consolidate setting of RAID1 stripes

[Johannes Thumshirn]

All 3 different RAID1 profiles use the same calculation mehod for the
number of used stripes.

Now that we do table lookups fo rmost of the allocation control
parameters, we can consolidate all 3 RAID1 profiles into a single branch
for the calculation.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/volumes.c b/volumes.c
index 32d3dfe0decd..24e6d151c313 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1129,20 +1129,9 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 			ctl.max_stripes = BTRFS_MAX_DEVS(info);
 		}
 	}
-	if (ctl.type == BTRFS_RAID_RAID1) {
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
-		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
-		if (ctl.num_stripes < ctl.min_stripes)
-			return -ENOSPC;
-	}
-	if (ctl.type == BTRFS_RAID_RAID1C3) {
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
-		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
-		if (ctl.num_stripes < ctl.min_stripes)
-			return -ENOSPC;
-	}
-	if (ctl.type == BTRFS_RAID_RAID1C4) {
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
+	if (ctl.type == BTRFS_RAID_RAID1 ||
+	    ctl.type == BTRFS_RAID_RAID1C3 ||
+	    ctl.type == BTRFS_RAID_RAID1C4) {
 		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
-- 
2.26.2

[PATCH 10/15] btrfs-progs: do table lookup for simple RAID profiles' num_stripes

[Johannes Thumshirn]

For the two simple "RAID" profiles single and dup, we can simply fallback
to a table lookup to get num_stripes.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/volumes.c b/volumes.c
index 24e6d151c313..80b4b373f012 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1100,7 +1100,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 
 	ctl.type = btrfs_bg_flags_to_raid_index(type);
-	ctl.num_stripes = 1;
+	ctl.num_stripes = btrfs_raid_profile_table[ctl.type].num_stripes;
 	ctl.max_stripes = 0;
 	ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 	ctl.sub_stripes = btrfs_raid_profile_table[ctl.type].sub_stripes;
@@ -1136,9 +1136,6 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ctl.num_stripes < ctl.min_stripes)
 			return -ENOSPC;
 	}
-	if (ctl.type == BTRFS_RAID_DUP) {
-		ctl.num_stripes = 2;
-	}
 	if (ctl.type == BTRFS_RAID_RAID0) {
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
-- 
2.26.2

[PATCH 11/15] btrfs-progs: consolidate num_stripes sanity check

[Johannes Thumshirn]

For all RAID profiles in btrfs_alloc_chunk() we're doing a sanity check if
the number of stripes is smaller than the minimal number of stripes
needed for this profile.

Consolidate this per-profile check to a single check after assigning the
number of stripes to further reduce code.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/volumes.c b/volumes.c
index 80b4b373f012..80144a763b72 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1133,36 +1133,27 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	    ctl.type == BTRFS_RAID_RAID1C3 ||
 	    ctl.type == BTRFS_RAID_RAID1C4) {
 		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
-		if (ctl.num_stripes < ctl.min_stripes)
-			return -ENOSPC;
 	}
 	if (ctl.type == BTRFS_RAID_RAID0) {
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 	}
 	if (ctl.type == BTRFS_RAID_RAID10) {
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		if (ctl.num_stripes < ctl.min_stripes)
-			return -ENOSPC;
 		ctl.num_stripes &= ~(u32)1;
 	}
 	if (ctl.type == BTRFS_RAID_RAID5) {
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		if (ctl.num_stripes < ctl.min_stripes)
-			return -ENOSPC;
 		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 1,
 				    btrfs_super_stripesize(info->super_copy));
 	}
 	if (ctl.type == BTRFS_RAID_RAID6) {
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		if (ctl.num_stripes < ctl.min_stripes)
-			return -ENOSPC;
 		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 2,
 				    btrfs_super_stripesize(info->super_copy));
 	}
+	if (ctl.num_stripes < ctl.min_stripes)
+		return -ENOSPC;
 
 	/* we don't want a chunk larger than 10% of the FS */
 	percent_max = div_factor(btrfs_super_total_bytes(info->super_copy), 1);
-- 
2.26.2

[PATCH 12/15] btrfs-progs: compactify num_stripe setting in btrfs_alloc_chunk

[Johannes Thumshirn]

Now that most of the RAID profile dependent chunk allocation parameters
have been converted to table lookus and moved out of the if-statement
maze, all that remains is the actual calculation of the number of stripes.

Compact the 5 if statements into a single switch statemnt to make the code
a bit more compact and more intuitive to follow.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/volumes.c b/volumes.c
index 80144a763b72..57d0db5463ef 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1129,28 +1129,31 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 			ctl.max_stripes = BTRFS_MAX_DEVS(info);
 		}
 	}
-	if (ctl.type == BTRFS_RAID_RAID1 ||
-	    ctl.type == BTRFS_RAID_RAID1C3 ||
-	    ctl.type == BTRFS_RAID_RAID1C4) {
+	switch (ctl.type) {
+	case BTRFS_RAID_RAID1:
+	case BTRFS_RAID_RAID1C3:
+	case BTRFS_RAID_RAID1C4:
 		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
-	}
-	if (ctl.type == BTRFS_RAID_RAID0) {
+		break;
+	case BTRFS_RAID_RAID0:
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
-	}
-	if (ctl.type == BTRFS_RAID_RAID10) {
+		break;
+	case BTRFS_RAID_RAID10:
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		ctl.num_stripes &= ~(u32)1;
-	}
-	if (ctl.type == BTRFS_RAID_RAID5) {
+		break;
+	case BTRFS_RAID_RAID5:
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 1,
 				    btrfs_super_stripesize(info->super_copy));
-	}
-	if (ctl.type == BTRFS_RAID_RAID6) {
+		break;
+	case BTRFS_RAID_RAID6:
 		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
 		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 2,
 				    btrfs_super_stripesize(info->super_copy));
+		break;
+	default:
+		break;
 	}
 	if (ctl.num_stripes < ctl.min_stripes)
 		return -ENOSPC;
-- 
2.26.2

[PATCH 13/15] btrfs-progs: introduce init_alloc_chunk_ctl

[Johannes Thumshirn]

Factor out setting of alloc_chuk_ctl fileds in a separate function
init_alloc_chunk_ctl.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 70 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/volumes.c b/volumes.c
index 57d0db5463ef..aacff6e0656b 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1067,6 +1067,44 @@ static const struct btrfs_raid_profile {
 	},
 };
 
+static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
+				 struct alloc_chunk_ctl *ctl)
+{
+	int type = ctl->type;
+
+	ctl->num_stripes = btrfs_raid_profile_table[type].num_stripes;
+	ctl->min_stripes = btrfs_raid_profile_table[type].min_stripes;
+	ctl->sub_stripes = btrfs_raid_profile_table[type].sub_stripes;
+	ctl->stripe_len = BTRFS_STRIPE_LEN;
+
+	switch (type) {
+	case BTRFS_RAID_RAID1:
+	case BTRFS_RAID_RAID1C3:
+	case BTRFS_RAID_RAID1C4:
+		ctl->num_stripes = min(ctl->min_stripes, ctl->total_devs);
+		break;
+	case BTRFS_RAID_RAID0:
+		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
+		break;
+	case BTRFS_RAID_RAID10:
+		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
+		ctl->num_stripes &= ~(u32)1;
+		break;
+	case BTRFS_RAID_RAID5:
+		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
+		ctl->stripe_len = find_raid56_stripe_len(ctl->num_stripes - 1,
+				    btrfs_super_stripesize(info->super_copy));
+		break;
+	case BTRFS_RAID_RAID6:
+		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
+		ctl->stripe_len = find_raid56_stripe_len(ctl->num_stripes - 2,
+				    btrfs_super_stripesize(info->super_copy));
+		break;
+	default:
+		break;
+	}
+}
+
 int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		      struct btrfs_fs_info *info, u64 *start,
 		      u64 *num_bytes, u64 type)
@@ -1100,11 +1138,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	}
 
 	ctl.type = btrfs_bg_flags_to_raid_index(type);
-	ctl.num_stripes = btrfs_raid_profile_table[ctl.type].num_stripes;
 	ctl.max_stripes = 0;
-	ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
-	ctl.sub_stripes = btrfs_raid_profile_table[ctl.type].sub_stripes;
-	ctl.stripe_len = BTRFS_STRIPE_LEN;
 	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
 
 	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
@@ -1129,32 +1163,8 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 			ctl.max_stripes = BTRFS_MAX_DEVS(info);
 		}
 	}
-	switch (ctl.type) {
-	case BTRFS_RAID_RAID1:
-	case BTRFS_RAID_RAID1C3:
-	case BTRFS_RAID_RAID1C4:
-		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
-		break;
-	case BTRFS_RAID_RAID0:
-		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		break;
-	case BTRFS_RAID_RAID10:
-		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		ctl.num_stripes &= ~(u32)1;
-		break;
-	case BTRFS_RAID_RAID5:
-		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 1,
-				    btrfs_super_stripesize(info->super_copy));
-		break;
-	case BTRFS_RAID_RAID6:
-		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
-		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 2,
-				    btrfs_super_stripesize(info->super_copy));
-		break;
-	default:
-		break;
-	}
+
+	init_alloc_chunk_ctl(info, &ctl);
 	if (ctl.num_stripes < ctl.min_stripes)
 		return -ENOSPC;
 
-- 
2.26.2

Re: [PATCH 13/15] btrfs-progs: introduce init_alloc_chunk_ctl

[Qu Wenruo]

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



On 2020/6/10 下午8:32, Johannes Thumshirn wrote:
> Factor out setting of alloc_chuk_ctl fileds in a separate function
> init_alloc_chunk_ctl.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  volumes.c | 70 +++++++++++++++++++++++++++++++------------------------
>  1 file changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/volumes.c b/volumes.c
> index 57d0db5463ef..aacff6e0656b 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -1067,6 +1067,44 @@ static const struct btrfs_raid_profile {
>  	},
>  };
>  
> +static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
> +				 struct alloc_chunk_ctl *ctl)
> +{
> +	int type = ctl->type;
> +
> +	ctl->num_stripes = btrfs_raid_profile_table[type].num_stripes;
> +	ctl->min_stripes = btrfs_raid_profile_table[type].min_stripes;
> +	ctl->sub_stripes = btrfs_raid_profile_table[type].sub_stripes;
> +	ctl->stripe_len = BTRFS_STRIPE_LEN;
> +
> +	switch (type) {
> +	case BTRFS_RAID_RAID1:
> +	case BTRFS_RAID_RAID1C3:
> +	case BTRFS_RAID_RAID1C4:
> +		ctl->num_stripes = min(ctl->min_stripes, ctl->total_devs);

The kernel code looks more elegant to me, no switch at all and takes
full advantage of btrfs_raid_attr.

Although your work is already pretty awesome, mind to make it even better?

Thanks,
Qu

> +		break;
> +	case BTRFS_RAID_RAID0:
> +		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
> +		break;
> +	case BTRFS_RAID_RAID10:
> +		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
> +		ctl->num_stripes &= ~(u32)1;
> +		break;
> +	case BTRFS_RAID_RAID5:
> +		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
> +		ctl->stripe_len = find_raid56_stripe_len(ctl->num_stripes - 1,
> +				    btrfs_super_stripesize(info->super_copy));
> +		break;
> +	case BTRFS_RAID_RAID6:
> +		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
> +		ctl->stripe_len = find_raid56_stripe_len(ctl->num_stripes - 2,
> +				    btrfs_super_stripesize(info->super_copy));
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>  int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  		      struct btrfs_fs_info *info, u64 *start,
>  		      u64 *num_bytes, u64 type)
> @@ -1100,11 +1138,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  	}
>  
>  	ctl.type = btrfs_bg_flags_to_raid_index(type);
> -	ctl.num_stripes = btrfs_raid_profile_table[ctl.type].num_stripes;
>  	ctl.max_stripes = 0;
> -	ctl.min_stripes = btrfs_raid_profile_table[ctl.type].min_stripes;
> -	ctl.sub_stripes = btrfs_raid_profile_table[ctl.type].sub_stripes;
> -	ctl.stripe_len = BTRFS_STRIPE_LEN;
>  	ctl.total_devs = btrfs_super_num_devices(info->super_copy);
>  
>  	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
> @@ -1129,32 +1163,8 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  			ctl.max_stripes = BTRFS_MAX_DEVS(info);
>  		}
>  	}
> -	switch (ctl.type) {
> -	case BTRFS_RAID_RAID1:
> -	case BTRFS_RAID_RAID1C3:
> -	case BTRFS_RAID_RAID1C4:
> -		ctl.num_stripes = min(ctl.min_stripes, ctl.total_devs);
> -		break;
> -	case BTRFS_RAID_RAID0:
> -		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
> -		break;
> -	case BTRFS_RAID_RAID10:
> -		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
> -		ctl.num_stripes &= ~(u32)1;
> -		break;
> -	case BTRFS_RAID_RAID5:
> -		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
> -		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 1,
> -				    btrfs_super_stripesize(info->super_copy));
> -		break;
> -	case BTRFS_RAID_RAID6:
> -		ctl.num_stripes = min(ctl.max_stripes, ctl.total_devs);
> -		ctl.stripe_len = find_raid56_stripe_len(ctl.num_stripes - 2,
> -				    btrfs_super_stripesize(info->super_copy));
> -		break;
> -	default:
> -		break;
> -	}
> +
> +	init_alloc_chunk_ctl(info, &ctl);
>  	if (ctl.num_stripes < ctl.min_stripes)
>  		return -ENOSPC;
>  
> 


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

[PATCH 14/15] btrfs-progs: don't pretend RAID56 has a different stripe length

[Johannes Thumshirn]

Since it's addition in 2009 BTRFS RAID5/6 uses a constant stripe length
and we're lacking the meta-data to define a per stripe length, so it's
unlikely to change in the near future for RAID5/6.

So let's not pretend something we don't do and remove the RAID5/6 stripe
length special casing.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/volumes.c b/volumes.c
index aacff6e0656b..ec31acd57aa7 100644
--- a/volumes.c
+++ b/volumes.c
@@ -900,13 +900,6 @@ static u64 chunk_bytes_by_type(u64 type, u64 calc_size,
 		return calc_size * ctl->num_stripes;
 }
 
-
-static u32 find_raid56_stripe_len(u32 data_devices, u32 dev_stripe_target)
-{
-	/* TODO, add a way to store the preferred stripe size */
-	return BTRFS_STRIPE_LEN;
-}
-
 /*
  * btrfs_device_avail_bytes - count bytes available for alloc_chunk
  *
@@ -1092,13 +1085,9 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 		break;
 	case BTRFS_RAID_RAID5:
 		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
-		ctl->stripe_len = find_raid56_stripe_len(ctl->num_stripes - 1,
-				    btrfs_super_stripesize(info->super_copy));
 		break;
 	case BTRFS_RAID_RAID6:
 		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
-		ctl->stripe_len = find_raid56_stripe_len(ctl->num_stripes - 2,
-				    btrfs_super_stripesize(info->super_copy));
 		break;
 	default:
 		break;
-- 
2.26.2

[PATCH 15/15] btrfs-progs: consolidate num_stripes setting for striping RAID levels

[Johannes Thumshirn]

All striping RAID Levels use the same method to set the number of RAID
stripes, so consolidate it.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 volumes.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/volumes.c b/volumes.c
index ec31acd57aa7..7c57d6cb376e 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1077,17 +1077,12 @@ static void init_alloc_chunk_ctl(struct btrfs_fs_info *info,
 		ctl->num_stripes = min(ctl->min_stripes, ctl->total_devs);
 		break;
 	case BTRFS_RAID_RAID0:
-		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
-		break;
 	case BTRFS_RAID_RAID10:
-		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
-		ctl->num_stripes &= ~(u32)1;
-		break;
 	case BTRFS_RAID_RAID5:
-		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
-		break;
 	case BTRFS_RAID_RAID6:
 		ctl->num_stripes = min(ctl->max_stripes, ctl->total_devs);
+		if (type == BTRFS_RAID_RAID10)
+			ctl->num_stripes &= ~(u32)1;
 		break;
 	default:
 		break;
-- 
2.26.2

Re: [PATCH 00/15] btrfs-progs: simplify chunk allocation a bit

[David Sterba]

On Wed, Jun 10, 2020 at 09:32:43PM +0900, Johannes Thumshirn wrote:
> While playing a bit with the RAID code, I've come up with some cleanups for
> the chunk allocatin in progs. It's not aligned to what we're doing in the
> kernel, but the code has diverged so much it is a daunting task to converge it
> again.

Thanks, converging the code would take time and smaller steps are fine.
> 
> Johannes Thumshirn (15):
>   btrfs-progs: simplify minimal stripe number checking
>   btrfs-progs: simplify assignment of number of RAID stripes
>   btrfs-progs: introduce alloc_chunk_ctl structure
>   btrfs-progs: cache number of devices for chunk allocation
>   btrfs-progs: pass alloc_chunk_ctl to chunk_bytes_by_type
>   btrfs-progs: introduce raid profile table for chunk allocation
>   btrfs-progs: consolidate assignment of minimal stripe number
>   btrfs-progs: consolidate assignment of sub_stripes
>   btrfs-progs: consolidate setting of RAID1 stripes
>   btrfs-progs: do table lookup for simple RAID profiles' num_stripes
>   btrfs-progs: consolidate num_stripes sanity check
>   btrfs-progs: compactify num_stripe setting in btrfs_alloc_chunk
>   btrfs-progs: introduce init_alloc_chunk_ctl
>   btrfs-progs: don't pretend RAID56 has a different stripe length
>   btrfs-progs: consolidate num_stripes setting for striping RAID levels

Added to devel.

Re: [PATCH 00/15] btrfs-progs: simplify chunk allocation a bit

[Qu Wenruo]

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



On 2020/6/10 下午8:32, Johannes Thumshirn wrote:
> While playing a bit with the RAID code, I've come up with some cleanups for
> the chunk allocatin in progs. It's not aligned to what we're doing in the
> kernel, but the code has diverged so much it is a daunting task to converge it
> again.

The refactor looks pretty good, and passes all existing self tests.

But comparing with kernel code, there are still too many if branches for
different profiles, unlike kernel fully utilizing btrfs_raid_attr.

With that fixed, it would be perfect to be merged.

Thanks,
Qu
> 
> Johannes Thumshirn (15):
>   btrfs-progs: simplify minimal stripe number checking
>   btrfs-progs: simplify assignment of number of RAID stripes
>   btrfs-progs: introduce alloc_chunk_ctl structure
>   btrfs-progs: cache number of devices for chunk allocation
>   btrfs-progs: pass alloc_chunk_ctl to chunk_bytes_by_type
>   btrfs-progs: introduce raid profile table for chunk allocation
>   btrfs-progs: consolidate assignment of minimal stripe number
>   btrfs-progs: consolidate assignment of sub_stripes
>   btrfs-progs: consolidate setting of RAID1 stripes
>   btrfs-progs: do table lookup for simple RAID profiles' num_stripes
>   btrfs-progs: consolidate num_stripes sanity check
>   btrfs-progs: compactify num_stripe setting in btrfs_alloc_chunk
>   btrfs-progs: introduce init_alloc_chunk_ctl
>   btrfs-progs: don't pretend RAID56 has a different stripe length
>   btrfs-progs: consolidate num_stripes setting for striping RAID levels
> 
>  volumes.c | 261 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 148 insertions(+), 113 deletions(-)
> 


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

Re: [PATCH 00/15] btrfs-progs: simplify chunk allocation a bit

[Johannes Thumshirn]

On 23/06/2020 08:33, Qu Wenruo wrote:
> But comparing with kernel code, there are still too many if branches for
> different profiles, unlike kernel fully utilizing btrfs_raid_attr.

Yes that'll come in the next step. I still need to fully understand the 
mapping between the current variables and the ones used in btrfs_raid_attr.

Thanks,
	Johannes