linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Misc btrfs-progs fixes
@ 2019-10-30 12:22 Nikolay Borisov
  2019-10-30 12:22 ` [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk Nikolay Borisov
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Nikolay Borisov @ 2019-10-30 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

Here are 2 cleanups and 1 minor fix for mkfs. The gist of the fix is to ensure 
sub_stripes is always set to 1 when mkfs creates blockgroups with alloc profile 
different than RAID10. This what kernels does. 

The other 2 patches are simple cleanups which reduce the number of arguments 
of btrfs_alloc_data_chunk. 

Nikolay Borisov (3):
  btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk
  btrfs-progs: Remove type argument from btrfs_alloc_data_chunk
  btrfs-progs: Remove convert param from btrfs_alloc_data_chunk

 convert/main.c |  4 +---
 volumes.c      | 52 ++++++++++++++++++----------------------------------
 volumes.h      |  3 +--
 3 files changed, 20 insertions(+), 39 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk
  2019-10-30 12:22 [PATCH 0/3] Misc btrfs-progs fixes Nikolay Borisov
@ 2019-10-30 12:22 ` Nikolay Borisov
  2019-10-30 12:42   ` Qu Wenruo
  2019-10-30 12:22 ` [PATCH 2/3] btrfs-progs: Remove type argument from btrfs_alloc_data_chunk Nikolay Borisov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2019-10-30 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

sub_stripe variables is by default initialized to 0 and it's overriden
only in case we have RAID10 mode. This leads to the following (minor)
artifacts on a freshly created filesystem:

item 3 key (FIRST_CHUNK_TREE CHUNK_ITEM 30408704) itemoff 15863 itemsize 112
		length 1073741824 owner 2 stripe_len 65536 type METADATA|RAID1
		io_align 65536 io_width 65536 sector_size 4096
		num_stripes 2 sub_stripes 0
			stripe 0 devid 2 offset 9437184
			dev_uuid a020fc2f-b526-4800-9278-156f2f431fe9
			stripe 1 devid 1 offset 30408704
			dev_uuid 0f78aa72-4626-4057-a8f2-285f46b2c664

After balance resulting chunk item is:

item 3 key (FIRST_CHUNK_TREE CHUNK_ITEM 3251634176) itemoff 15863 itemsize 112
		length 268435456 owner 2 stripe_len 65536 type METADATA|RAID1
		io_align 65536 io_width 65536 sector_size 4096
		num_stripes 2 sub_stripes 1
			stripe 0 devid 2 offset 3230662656
			dev_uuid a020fc2f-b526-4800-9278-156f2f431fe9
			stripe 1 devid 1 offset 3251634176
			dev_uuid 0f78aa72-4626-4057-a8f2-285f46b2c664

Kernel code usually initializes it to 1, since it takes the value from
the raid description table which has it set to 1 for all but RAID10 types.
In userspace it has be statically initialized by 1 since we don't have
btrfs_bg_flags_to_raid_index. Eventually the kernel/userspace needs
to be merged but for now it wouldn't bring much value if this function
is copied.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 volumes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/volumes.c b/volumes.c
index fbbc22b5b1b3..1d088d93e788 100644
--- a/volumes.c
+++ b/volumes.c
@@ -993,7 +993,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 	int num_stripes = 1;
 	int max_stripes = 0;
 	int min_stripes = 1;
-	int sub_stripes = 0;
+	int sub_stripes = 1;
 	int looped = 0;
 	int ret;
 	int index;
@@ -1258,7 +1258,7 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	struct map_lookup *map;
 	u64 calc_size = SZ_8M;
 	int num_stripes = 1;
-	int sub_stripes = 0;
+	int sub_stripes = 1;
 	int ret;
 	int index;
 	int stripe_len = BTRFS_STRIPE_LEN;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] btrfs-progs: Remove type argument from btrfs_alloc_data_chunk
  2019-10-30 12:22 [PATCH 0/3] Misc btrfs-progs fixes Nikolay Borisov
  2019-10-30 12:22 ` [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk Nikolay Borisov
@ 2019-10-30 12:22 ` Nikolay Borisov
  2019-10-30 12:42   ` Qu Wenruo
  2019-10-30 12:22 ` [PATCH 3/3] btrfs-progs: Remove convert param " Nikolay Borisov
  2019-11-15 12:22 ` [PATCH 0/3] Misc btrfs-progs fixes David Sterba
  3 siblings, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2019-10-30 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

It's always set to BTRFS_BLOCK_GROUP_DATA so sink it into the function.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 convert/main.c | 3 +--
 volumes.c      | 6 +++---
 volumes.h      | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index bb689be9f3e4..9904deafba45 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -943,8 +943,7 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
 			len = min(max_chunk_size,
 				  cache->start + cache->size - cur);
 			ret = btrfs_alloc_data_chunk(trans, fs_info,
-					&cur_backup, len,
-					BTRFS_BLOCK_GROUP_DATA, 1);
+					&cur_backup, len, 1);
 			if (ret < 0)
 				break;
 			ret = btrfs_make_block_group(trans, fs_info, 0,
diff --git a/volumes.c b/volumes.c
index 1d088d93e788..87315a884b49 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1245,7 +1245,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
  */
 int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 			   struct btrfs_fs_info *info, u64 *start,
-			   u64 num_bytes, u64 type, int convert)
+			   u64 num_bytes, int convert)
 {
 	u64 dev_offset;
 	struct btrfs_root *extent_root = info->extent_root;
@@ -1328,7 +1328,7 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	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_type(chunk, type);
+	btrfs_set_stack_chunk_type(chunk, BTRFS_BLOCK_GROUP_DATA);
 	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);
@@ -1338,7 +1338,7 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	map->stripe_len = stripe_len;
 	map->io_align = stripe_len;
 	map->io_width = stripe_len;
-	map->type = type;
+	map->type = BTRFS_BLOCK_GROUP_DATA;
 	map->num_stripes = num_stripes;
 	map->sub_stripes = sub_stripes;
 
diff --git a/volumes.h b/volumes.h
index 586588c871ab..83ba827e422b 100644
--- a/volumes.h
+++ b/volumes.h
@@ -272,7 +272,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		      u64 *num_bytes, u64 type);
 int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 			   struct btrfs_fs_info *fs_info, u64 *start,
-			   u64 num_bytes, u64 type, int convert);
+			   u64 num_bytes, int convert);
 int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       int flags);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] btrfs-progs: Remove convert param from btrfs_alloc_data_chunk
  2019-10-30 12:22 [PATCH 0/3] Misc btrfs-progs fixes Nikolay Borisov
  2019-10-30 12:22 ` [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk Nikolay Borisov
  2019-10-30 12:22 ` [PATCH 2/3] btrfs-progs: Remove type argument from btrfs_alloc_data_chunk Nikolay Borisov
@ 2019-10-30 12:22 ` Nikolay Borisov
  2019-10-30 12:45   ` Qu Wenruo
  2019-11-15 12:22 ` [PATCH 0/3] Misc btrfs-progs fixes David Sterba
  3 siblings, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2019-10-30 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

convert is always set to true so there's no point in having it as a
function parameter or using it as a predicate inside btrfs_alloc_data_chunk.
Remove it and all relevant code which would have never been executed.
No semantics changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 convert/main.c |  3 +--
 volumes.c      | 44 ++++++++++++++------------------------------
 volumes.h      |  3 +--
 3 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 9904deafba45..416ab5d264a3 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -942,8 +942,7 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
 
 			len = min(max_chunk_size,
 				  cache->start + cache->size - cur);
-			ret = btrfs_alloc_data_chunk(trans, fs_info,
-					&cur_backup, len, 1);
+			ret = btrfs_alloc_data_chunk(trans, fs_info, &cur_backup, len);
 			if (ret < 0)
 				break;
 			ret = btrfs_make_block_group(trans, fs_info, 0,
diff --git a/volumes.c b/volumes.c
index 87315a884b49..39e824a43736 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1238,14 +1238,11 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 /*
  * Alloc a DATA chunk with SINGLE profile.
  *
- * If 'convert' is set, it will alloc a chunk with 1:1 mapping
- * (btrfs logical bytenr == on-disk bytenr)
- * For that case, caller must make sure the chunk and dev_extent are not
- * occupied.
+ * It allocates a chunk with 1:1 mapping (btrfs logical bytenr == on-disk bytenr)
+ * Caller must make sure the chunk and dev_extent are not occupied.
  */
 int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
-			   struct btrfs_fs_info *info, u64 *start,
-			   u64 num_bytes, int convert)
+			   struct btrfs_fs_info *info, u64 *start, u64 num_bytes)
 {
 	u64 dev_offset;
 	struct btrfs_root *extent_root = info->extent_root;
@@ -1264,25 +1261,18 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	int stripe_len = BTRFS_STRIPE_LEN;
 	struct btrfs_key key;
 
-	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
-	key.type = BTRFS_CHUNK_ITEM_KEY;
-	if (convert) {
-		if (*start != round_down(*start, info->sectorsize)) {
-			error("DATA chunk start not sectorsize aligned: %llu",
-					(unsigned long long)*start);
-			return -EINVAL;
-		}
-		key.offset = *start;
-		dev_offset = *start;
-	} else {
-		u64 tmp;
 
-		ret = find_next_chunk(info, &tmp);
-		key.offset = tmp;
-		if (ret)
-			return ret;
+	if (*start != round_down(*start, info->sectorsize)) {
+		error("DATA chunk start not sectorsize aligned: %llu",
+				(unsigned long long)*start);
+		return -EINVAL;
 	}
 
+	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
+	key.type = BTRFS_CHUNK_ITEM_KEY;
+	key.offset = *start;
+	dev_offset = *start;
+
 	chunk = kmalloc(btrfs_chunk_item_size(num_stripes), GFP_NOFS);
 	if (!chunk)
 		return -ENOMEM;
@@ -1303,12 +1293,8 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	while (index < num_stripes) {
 		struct btrfs_stripe *stripe;
 
-		if (convert)
-			ret = btrfs_insert_dev_extent(trans, device, key.offset,
-					calc_size, dev_offset);
-		else
-			ret = btrfs_alloc_dev_extent(trans, device, key.offset,
-					calc_size, &dev_offset);
+		ret = btrfs_insert_dev_extent(trans, device, key.offset, calc_size,
+				dev_offset);
 		BUG_ON(ret);
 
 		device->bytes_used += calc_size;
@@ -1345,8 +1331,6 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	ret = btrfs_insert_item(trans, chunk_root, &key, chunk,
 				btrfs_chunk_item_size(num_stripes));
 	BUG_ON(ret);
-	if (!convert)
-		*start = key.offset;
 
 	map->ce.start = key.offset;
 	map->ce.size = num_bytes;
diff --git a/volumes.h b/volumes.h
index 83ba827e422b..f6f05054b5c4 100644
--- a/volumes.h
+++ b/volumes.h
@@ -271,8 +271,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		      struct btrfs_fs_info *fs_info, u64 *start,
 		      u64 *num_bytes, u64 type);
 int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
-			   struct btrfs_fs_info *fs_info, u64 *start,
-			   u64 num_bytes, int convert);
+			   struct btrfs_fs_info *fs_info, u64 *start, u64 num_bytes);
 int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       int flags);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk
  2019-10-30 12:22 ` [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk Nikolay Borisov
@ 2019-10-30 12:42   ` Qu Wenruo
  0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2019-10-30 12:42 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs



On 2019/10/30 下午8:22, Nikolay Borisov wrote:
> sub_stripe variables is by default initialized to 0 and it's overriden
> only in case we have RAID10 mode. This leads to the following (minor)
> artifacts on a freshly created filesystem:
>
> item 3 key (FIRST_CHUNK_TREE CHUNK_ITEM 30408704) itemoff 15863 itemsize 112
> 		length 1073741824 owner 2 stripe_len 65536 type METADATA|RAID1
> 		io_align 65536 io_width 65536 sector_size 4096
> 		num_stripes 2 sub_stripes 0
> 			stripe 0 devid 2 offset 9437184
> 			dev_uuid a020fc2f-b526-4800-9278-156f2f431fe9
> 			stripe 1 devid 1 offset 30408704
> 			dev_uuid 0f78aa72-4626-4057-a8f2-285f46b2c664
>
> After balance resulting chunk item is:
>
> item 3 key (FIRST_CHUNK_TREE CHUNK_ITEM 3251634176) itemoff 15863 itemsize 112
> 		length 268435456 owner 2 stripe_len 65536 type METADATA|RAID1
> 		io_align 65536 io_width 65536 sector_size 4096
> 		num_stripes 2 sub_stripes 1
> 			stripe 0 devid 2 offset 3230662656
> 			dev_uuid a020fc2f-b526-4800-9278-156f2f431fe9
> 			stripe 1 devid 1 offset 3251634176
> 			dev_uuid 0f78aa72-4626-4057-a8f2-285f46b2c664
>
> Kernel code usually initializes it to 1, since it takes the value from
> the raid description table which has it set to 1 for all but RAID10 types.
> In userspace it has be statically initialized by 1 since we don't have
> btrfs_bg_flags_to_raid_index. Eventually the kernel/userspace needs
> to be merged but for now it wouldn't bring much value if this function
> is copied.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

I guess the tree-checker skips this check except for RAID10 just to work
around this problem.

Thanks,
Qu
> ---
>  volumes.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/volumes.c b/volumes.c
> index fbbc22b5b1b3..1d088d93e788 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -993,7 +993,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  	int num_stripes = 1;
>  	int max_stripes = 0;
>  	int min_stripes = 1;
> -	int sub_stripes = 0;
> +	int sub_stripes = 1;
>  	int looped = 0;
>  	int ret;
>  	int index;
> @@ -1258,7 +1258,7 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  	struct map_lookup *map;
>  	u64 calc_size = SZ_8M;
>  	int num_stripes = 1;
> -	int sub_stripes = 0;
> +	int sub_stripes = 1;
>  	int ret;
>  	int index;
>  	int stripe_len = BTRFS_STRIPE_LEN;
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] btrfs-progs: Remove type argument from btrfs_alloc_data_chunk
  2019-10-30 12:22 ` [PATCH 2/3] btrfs-progs: Remove type argument from btrfs_alloc_data_chunk Nikolay Borisov
@ 2019-10-30 12:42   ` Qu Wenruo
  0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2019-10-30 12:42 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs



On 2019/10/30 下午8:22, Nikolay Borisov wrote:
> It's always set to BTRFS_BLOCK_GROUP_DATA so sink it into the function.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  convert/main.c | 3 +--
>  volumes.c      | 6 +++---
>  volumes.h      | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/convert/main.c b/convert/main.c
> index bb689be9f3e4..9904deafba45 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -943,8 +943,7 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
>  			len = min(max_chunk_size,
>  				  cache->start + cache->size - cur);
>  			ret = btrfs_alloc_data_chunk(trans, fs_info,
> -					&cur_backup, len,
> -					BTRFS_BLOCK_GROUP_DATA, 1);
> +					&cur_backup, len, 1);
>  			if (ret < 0)
>  				break;
>  			ret = btrfs_make_block_group(trans, fs_info, 0,
> diff --git a/volumes.c b/volumes.c
> index 1d088d93e788..87315a884b49 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -1245,7 +1245,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>   */
>  int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  			   struct btrfs_fs_info *info, u64 *start,
> -			   u64 num_bytes, u64 type, int convert)
> +			   u64 num_bytes, int convert)
>  {
>  	u64 dev_offset;
>  	struct btrfs_root *extent_root = info->extent_root;
> @@ -1328,7 +1328,7 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  	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_type(chunk, type);
> +	btrfs_set_stack_chunk_type(chunk, BTRFS_BLOCK_GROUP_DATA);
>  	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);
> @@ -1338,7 +1338,7 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  	map->stripe_len = stripe_len;
>  	map->io_align = stripe_len;
>  	map->io_width = stripe_len;
> -	map->type = type;
> +	map->type = BTRFS_BLOCK_GROUP_DATA;
>  	map->num_stripes = num_stripes;
>  	map->sub_stripes = sub_stripes;
>
> diff --git a/volumes.h b/volumes.h
> index 586588c871ab..83ba827e422b 100644
> --- a/volumes.h
> +++ b/volumes.h
> @@ -272,7 +272,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  		      u64 *num_bytes, u64 type);
>  int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  			   struct btrfs_fs_info *fs_info, u64 *start,
> -			   u64 num_bytes, u64 type, int convert);
> +			   u64 num_bytes, int convert);
>  int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>  		       int flags);
>  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] btrfs-progs: Remove convert param from btrfs_alloc_data_chunk
  2019-10-30 12:22 ` [PATCH 3/3] btrfs-progs: Remove convert param " Nikolay Borisov
@ 2019-10-30 12:45   ` Qu Wenruo
  0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2019-10-30 12:45 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs



On 2019/10/30 下午8:22, Nikolay Borisov wrote:
> convert is always set to true so there's no point in having it as a
> function parameter or using it as a predicate inside btrfs_alloc_data_chunk.
> Remove it and all relevant code which would have never been executed.
> No semantics changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Just one tip for further patches.

The function btrfs_alloc_data_chunk() is purely used for convert to
create a 1:1 mapped data chunk.

It would be be even better to rename the function to indicate it better.

Thanks,
Qu

> ---
>  convert/main.c |  3 +--
>  volumes.c      | 44 ++++++++++++++------------------------------
>  volumes.h      |  3 +--
>  3 files changed, 16 insertions(+), 34 deletions(-)
>
> diff --git a/convert/main.c b/convert/main.c
> index 9904deafba45..416ab5d264a3 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -942,8 +942,7 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
>
>  			len = min(max_chunk_size,
>  				  cache->start + cache->size - cur);
> -			ret = btrfs_alloc_data_chunk(trans, fs_info,
> -					&cur_backup, len, 1);
> +			ret = btrfs_alloc_data_chunk(trans, fs_info, &cur_backup, len);
>  			if (ret < 0)
>  				break;
>  			ret = btrfs_make_block_group(trans, fs_info, 0,
> diff --git a/volumes.c b/volumes.c
> index 87315a884b49..39e824a43736 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -1238,14 +1238,11 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  /*
>   * Alloc a DATA chunk with SINGLE profile.
>   *
> - * If 'convert' is set, it will alloc a chunk with 1:1 mapping
> - * (btrfs logical bytenr == on-disk bytenr)
> - * For that case, caller must make sure the chunk and dev_extent are not
> - * occupied.
> + * It allocates a chunk with 1:1 mapping (btrfs logical bytenr == on-disk bytenr)
> + * Caller must make sure the chunk and dev_extent are not occupied.
>   */
>  int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
> -			   struct btrfs_fs_info *info, u64 *start,
> -			   u64 num_bytes, int convert)
> +			   struct btrfs_fs_info *info, u64 *start, u64 num_bytes)
>  {
>  	u64 dev_offset;
>  	struct btrfs_root *extent_root = info->extent_root;
> @@ -1264,25 +1261,18 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  	int stripe_len = BTRFS_STRIPE_LEN;
>  	struct btrfs_key key;
>
> -	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
> -	key.type = BTRFS_CHUNK_ITEM_KEY;
> -	if (convert) {
> -		if (*start != round_down(*start, info->sectorsize)) {
> -			error("DATA chunk start not sectorsize aligned: %llu",
> -					(unsigned long long)*start);
> -			return -EINVAL;
> -		}
> -		key.offset = *start;
> -		dev_offset = *start;
> -	} else {
> -		u64 tmp;
>
> -		ret = find_next_chunk(info, &tmp);
> -		key.offset = tmp;
> -		if (ret)
> -			return ret;
> +	if (*start != round_down(*start, info->sectorsize)) {
> +		error("DATA chunk start not sectorsize aligned: %llu",
> +				(unsigned long long)*start);
> +		return -EINVAL;
>  	}
>
> +	key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
> +	key.type = BTRFS_CHUNK_ITEM_KEY;
> +	key.offset = *start;
> +	dev_offset = *start;
> +
>  	chunk = kmalloc(btrfs_chunk_item_size(num_stripes), GFP_NOFS);
>  	if (!chunk)
>  		return -ENOMEM;
> @@ -1303,12 +1293,8 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  	while (index < num_stripes) {
>  		struct btrfs_stripe *stripe;
>
> -		if (convert)
> -			ret = btrfs_insert_dev_extent(trans, device, key.offset,
> -					calc_size, dev_offset);
> -		else
> -			ret = btrfs_alloc_dev_extent(trans, device, key.offset,
> -					calc_size, &dev_offset);
> +		ret = btrfs_insert_dev_extent(trans, device, key.offset, calc_size,
> +				dev_offset);
>  		BUG_ON(ret);
>
>  		device->bytes_used += calc_size;
> @@ -1345,8 +1331,6 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  	ret = btrfs_insert_item(trans, chunk_root, &key, chunk,
>  				btrfs_chunk_item_size(num_stripes));
>  	BUG_ON(ret);
> -	if (!convert)
> -		*start = key.offset;
>
>  	map->ce.start = key.offset;
>  	map->ce.size = num_bytes;
> diff --git a/volumes.h b/volumes.h
> index 83ba827e422b..f6f05054b5c4 100644
> --- a/volumes.h
> +++ b/volumes.h
> @@ -271,8 +271,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
>  		      struct btrfs_fs_info *fs_info, u64 *start,
>  		      u64 *num_bytes, u64 type);
>  int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
> -			   struct btrfs_fs_info *fs_info, u64 *start,
> -			   u64 num_bytes, int convert);
> +			   struct btrfs_fs_info *fs_info, u64 *start, u64 num_bytes);
>  int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>  		       int flags);
>  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] Misc btrfs-progs fixes
  2019-10-30 12:22 [PATCH 0/3] Misc btrfs-progs fixes Nikolay Borisov
                   ` (2 preceding siblings ...)
  2019-10-30 12:22 ` [PATCH 3/3] btrfs-progs: Remove convert param " Nikolay Borisov
@ 2019-11-15 12:22 ` David Sterba
  3 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-11-15 12:22 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On Wed, Oct 30, 2019 at 02:22:24PM +0200, Nikolay Borisov wrote:
> Here are 2 cleanups and 1 minor fix for mkfs. The gist of the fix is to ensure 
> sub_stripes is always set to 1 when mkfs creates blockgroups with alloc profile 
> different than RAID10. This what kernels does. 
> 
> The other 2 patches are simple cleanups which reduce the number of arguments 
> of btrfs_alloc_data_chunk. 
> 
> Nikolay Borisov (3):
>   btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk
>   btrfs-progs: Remove type argument from btrfs_alloc_data_chunk
>   btrfs-progs: Remove convert param from btrfs_alloc_data_chunk

Added to devel, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-11-15 12:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 12:22 [PATCH 0/3] Misc btrfs-progs fixes Nikolay Borisov
2019-10-30 12:22 ` [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk Nikolay Borisov
2019-10-30 12:42   ` Qu Wenruo
2019-10-30 12:22 ` [PATCH 2/3] btrfs-progs: Remove type argument from btrfs_alloc_data_chunk Nikolay Borisov
2019-10-30 12:42   ` Qu Wenruo
2019-10-30 12:22 ` [PATCH 3/3] btrfs-progs: Remove convert param " Nikolay Borisov
2019-10-30 12:45   ` Qu Wenruo
2019-11-15 12:22 ` [PATCH 0/3] Misc btrfs-progs fixes David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).