All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range
@ 2018-01-10  4:56 Qu Wenruo
  2018-01-10  4:56 ` [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to check if the first device extent is occupying reserved 0~1M range Qu Wenruo
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Qu Wenruo @ 2018-01-10  4:56 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba, nborisov

When creating btrfs, mkfs.btrfs will firstly create a temporary system
chunk as basis, and then created needed trees or new devices.

However the layout temporary system chunk is hard-coded and uses
reserved [0, 1M) range of devid 1.

Change the temporary chunk layout from old:

0	1M				4M	5M
|<----------- temp chunk -------------->|
  And it's 1:1 mapped, which means it's a SINGLE chunk,
  and stripe offset is also 0.

to new layout:

0	1M				4M	5M
	|<----------- temp chunk -------------->|
  And still keeps the 1:1 mapping.

The problem can only be exposed by "-m single" or "-M" where we reuse the
temporary chunk.

With other meta profiles, system and meta chunks are allocated by later
btrfs_alloc_chunk() call, and old SINGLE chunks are removed, so it will
be no such problem for other meta profiles.

Reported-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 mkfs/common.c | 29 +++++++++++++++++++++++------
 mkfs/main.c   |  7 ++++++-
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index dd5e7ecff479..5c5e9c3b9e01 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -100,6 +100,21 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
  *
  * The superblock signature is not valid, denotes a partially created
  * filesystem, needs to be finalized.
+ *
+ * The temporary fs will have the following chunk layout:
+ * Device extent:
+ * 0		1M				5M	......
+ * | Reserved	| dev extent for SYS chunk      |
+ *
+ * And chunk mapping will be:
+ * Chunk mapping:
+ * 0		1M				5M
+ * |		| System chunk, 1:1 mapped	|
+ *
+ * That's to say, there will only be *ONE* system chunk, mapped to
+ * [1M, 5M) physical offset.
+ * And the only chunk is also in logical address [1M, 5M), containing
+ * all essential tree blocks.
  */
 int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 {
@@ -154,8 +169,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 
 	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
 	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
-		cfg->blocks[i] = BTRFS_SUPER_INFO_OFFSET + SZ_1M +
-			cfg->nodesize * i;
+		cfg->blocks[i] = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER +
+			cfg->nodesize * (i - 1);
 	}
 
 	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
@@ -309,7 +324,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 
 	/* then we have chunk 0 */
 	btrfs_set_disk_key_objectid(&disk_key, BTRFS_FIRST_CHUNK_TREE_OBJECTID);
-	btrfs_set_disk_key_offset(&disk_key, 0);
+	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
 	btrfs_set_disk_key_type(&disk_key, BTRFS_CHUNK_ITEM_KEY);
 	btrfs_set_item_key(buf, &disk_key, nritems);
 	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
@@ -325,7 +340,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	btrfs_set_chunk_sector_size(buf, chunk, cfg->sectorsize);
 	btrfs_set_chunk_num_stripes(buf, chunk, 1);
 	btrfs_set_stripe_devid_nr(buf, chunk, 0, 1);
-	btrfs_set_stripe_offset_nr(buf, chunk, 0, 0);
+	btrfs_set_stripe_offset_nr(buf, chunk, 0,
+				   BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
 	nritems++;
 
 	write_extent_buffer(buf, super.dev_item.uuid,
@@ -363,7 +379,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 		sizeof(struct btrfs_dev_extent);
 
 	btrfs_set_disk_key_objectid(&disk_key, 1);
-	btrfs_set_disk_key_offset(&disk_key, 0);
+	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
 	btrfs_set_disk_key_type(&disk_key, BTRFS_DEV_EXTENT_KEY);
 	btrfs_set_item_key(buf, &disk_key, nritems);
 	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
@@ -374,7 +390,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 					BTRFS_CHUNK_TREE_OBJECTID);
 	btrfs_set_dev_extent_chunk_objectid(buf, dev_extent,
 					BTRFS_FIRST_CHUNK_TREE_OBJECTID);
-	btrfs_set_dev_extent_chunk_offset(buf, dev_extent, 0);
+	btrfs_set_dev_extent_chunk_offset(buf, dev_extent,
+					  BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
 
 	write_extent_buffer(buf, chunk_tree_uuid,
 		    (unsigned long)btrfs_dev_extent_chunk_tree_uuid(dev_extent),
diff --git a/mkfs/main.c b/mkfs/main.c
index d817ad8dfd1a..8e3d19acb6f2 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -81,10 +81,15 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 	bytes_used = btrfs_super_bytes_used(fs_info->super_copy);
 
 	root->fs_info->system_allocs = 1;
+	/*
+	 * First temporary system chunk must match the chunk layout
+	 * created in make_btrfs().
+	 */
 	ret = btrfs_make_block_group(trans, fs_info, bytes_used,
 				     BTRFS_BLOCK_GROUP_SYSTEM,
 				     BTRFS_FIRST_CHUNK_TREE_OBJECTID,
-				     0, BTRFS_MKFS_SYSTEM_GROUP_SIZE);
+				     BTRFS_BLOCK_RESERVED_1M_FOR_SUPER,
+				     BTRFS_MKFS_SYSTEM_GROUP_SIZE);
 	allocation->system += BTRFS_MKFS_SYSTEM_GROUP_SIZE;
 	if (ret)
 		return ret;
-- 
2.15.1


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

* [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to check if the first device extent is occupying reserved 0~1M range
  2018-01-10  4:56 [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Qu Wenruo
@ 2018-01-10  4:56 ` Qu Wenruo
  2018-01-10  8:57   ` Nikolay Borisov
  2018-01-10  8:37 ` [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Nikolay Borisov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2018-01-10  4:56 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba, nborisov

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../mkfs-tests/010-reserved-1M-for-single/test.sh  | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100755 tests/mkfs-tests/010-reserved-1M-for-single/test.sh

diff --git a/tests/mkfs-tests/010-reserved-1M-for-single/test.sh b/tests/mkfs-tests/010-reserved-1M-for-single/test.sh
new file mode 100755
index 000000000000..1a4936eaf72e
--- /dev/null
+++ b/tests/mkfs-tests/010-reserved-1M-for-single/test.sh
@@ -0,0 +1,37 @@
+#!/bin/bash
+# Test if "-m single" or "-M" can cause dev extent to use reserved 1M range
+#
+# Other profiles will cause mkfs.btrfs to allocate new meta/sys chunks
+# using btrfs_alloc_chunk() which won't use 0~1M range, so other profiles
+# are safe.
+
+source "$TOP/tests/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+prepare_test_dev
+
+do_one_test ()
+{
+	run_check "$TOP/mkfs.btrfs" -f $@ "$TEST_DEV"
+
+	# Use dev-extent tree to find first device extent
+	first_dev_extent=$(run_check_stdout "$TOP/btrfs" inspect-internal \
+		dump-tree -t device "$TEST_DEV" | \
+		grep -oP '(?<=DEV_EXTENT )[[:digit:]]*' | head -n1)
+
+	if [ -z $first_dev_extent ]; then
+		_fail "Failed to get first device extent"
+	fi
+
+	echo "First dev extent starts at $first_dev_extent" >> "$RESULTS"
+	echo "Reserved range is [0, $(( 1024 * 1024)))" >> "$RESULTS"
+	# First device extent should not start below 1M
+	if [ $first_dev_extent -lt $(( 1024 * 1024 )) ]; then
+		_fail "First device extent occupies reserved 0~1M range"
+	fi
+}
+
+do_one_test "-M"
+do_one_test "-m single"
-- 
2.15.1


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

* Re: [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range
  2018-01-10  4:56 [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Qu Wenruo
  2018-01-10  4:56 ` [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to check if the first device extent is occupying reserved 0~1M range Qu Wenruo
@ 2018-01-10  8:37 ` Nikolay Borisov
  2018-01-10 14:14 ` Nikolay Borisov
  2018-01-23 16:42 ` David Sterba
  3 siblings, 0 replies; 8+ messages in thread
From: Nikolay Borisov @ 2018-01-10  8:37 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: dsterba



On 10.01.2018 06:56, Qu Wenruo wrote:
> When creating btrfs, mkfs.btrfs will firstly create a temporary system
> chunk as basis, and then created needed trees or new devices.
> 
> However the layout temporary system chunk is hard-coded and uses
> reserved [0, 1M) range of devid 1.
> 
> Change the temporary chunk layout from old:
> 
> 0	1M				4M	5M
> |<----------- temp chunk -------------->|
>   And it's 1:1 mapped, which means it's a SINGLE chunk,
>   and stripe offset is also 0.
> 
> to new layout:
> 
> 0	1M				4M	5M
> 	|<----------- temp chunk -------------->|
>   And still keeps the 1:1 mapping.
> 
> The problem can only be exposed by "-m single" or "-M" where we reuse the
> temporary chunk.
> 
> With other meta profiles, system and meta chunks are allocated by later
> btrfs_alloc_chunk() call, and old SINGLE chunks are removed, so it will
> be no such problem for other meta profiles.
> 
> Reported-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Tested-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>

After creating in -M or in -m single I can see:

item 0 key (1 DEV_EXTENT 1048576) itemoff 3947 itemsize 48
and
item 1 key (1048576 BLOCK_GROUP_ITEM 4194304) itemoff 3938 itemsize 24
  block group used 4096 chunk_objectid 256 flags SYSTEM




> ---

>  mkfs/common.c | 29 +++++++++++++++++++++++------
>  mkfs/main.c   |  7 ++++++-
>  2 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/mkfs/common.c b/mkfs/common.c
> index dd5e7ecff479..5c5e9c3b9e01 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -100,6 +100,21 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>   *
>   * The superblock signature is not valid, denotes a partially created
>   * filesystem, needs to be finalized.
> + *
> + * The temporary fs will have the following chunk layout:
> + * Device extent:
> + * 0		1M				5M	......
> + * | Reserved	| dev extent for SYS chunk      |
> + *
> + * And chunk mapping will be:
> + * Chunk mapping:
> + * 0		1M				5M
> + * |		| System chunk, 1:1 mapped	|
> + *
> + * That's to say, there will only be *ONE* system chunk, mapped to
> + * [1M, 5M) physical offset.
> + * And the only chunk is also in logical address [1M, 5M), containing
> + * all essential tree blocks.
>   */
>  int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  {
> @@ -154,8 +169,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  
>  	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
>  	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
> -		cfg->blocks[i] = BTRFS_SUPER_INFO_OFFSET + SZ_1M +
> -			cfg->nodesize * i;
> +		cfg->blocks[i] = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER +
> +			cfg->nodesize * (i - 1);
>  	}
>  
>  	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
> @@ -309,7 +324,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  
>  	/* then we have chunk 0 */
>  	btrfs_set_disk_key_objectid(&disk_key, BTRFS_FIRST_CHUNK_TREE_OBJECTID);
> -	btrfs_set_disk_key_offset(&disk_key, 0);
> +	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  	btrfs_set_disk_key_type(&disk_key, BTRFS_CHUNK_ITEM_KEY);
>  	btrfs_set_item_key(buf, &disk_key, nritems);
>  	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -325,7 +340,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  	btrfs_set_chunk_sector_size(buf, chunk, cfg->sectorsize);
>  	btrfs_set_chunk_num_stripes(buf, chunk, 1);
>  	btrfs_set_stripe_devid_nr(buf, chunk, 0, 1);
> -	btrfs_set_stripe_offset_nr(buf, chunk, 0, 0);
> +	btrfs_set_stripe_offset_nr(buf, chunk, 0,
> +				   BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  	nritems++;
>  
>  	write_extent_buffer(buf, super.dev_item.uuid,
> @@ -363,7 +379,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  		sizeof(struct btrfs_dev_extent);
>  
>  	btrfs_set_disk_key_objectid(&disk_key, 1);
> -	btrfs_set_disk_key_offset(&disk_key, 0);
> +	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  	btrfs_set_disk_key_type(&disk_key, BTRFS_DEV_EXTENT_KEY);
>  	btrfs_set_item_key(buf, &disk_key, nritems);
>  	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -374,7 +390,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  					BTRFS_CHUNK_TREE_OBJECTID);
>  	btrfs_set_dev_extent_chunk_objectid(buf, dev_extent,
>  					BTRFS_FIRST_CHUNK_TREE_OBJECTID);
> -	btrfs_set_dev_extent_chunk_offset(buf, dev_extent, 0);
> +	btrfs_set_dev_extent_chunk_offset(buf, dev_extent,
> +					  BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  
>  	write_extent_buffer(buf, chunk_tree_uuid,
>  		    (unsigned long)btrfs_dev_extent_chunk_tree_uuid(dev_extent),
> diff --git a/mkfs/main.c b/mkfs/main.c
> index d817ad8dfd1a..8e3d19acb6f2 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -81,10 +81,15 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
>  	bytes_used = btrfs_super_bytes_used(fs_info->super_copy);
>  
>  	root->fs_info->system_allocs = 1;
> +	/*
> +	 * First temporary system chunk must match the chunk layout
> +	 * created in make_btrfs().
> +	 */
>  	ret = btrfs_make_block_group(trans, fs_info, bytes_used,
>  				     BTRFS_BLOCK_GROUP_SYSTEM,
>  				     BTRFS_FIRST_CHUNK_TREE_OBJECTID,
> -				     0, BTRFS_MKFS_SYSTEM_GROUP_SIZE);
> +				     BTRFS_BLOCK_RESERVED_1M_FOR_SUPER,
> +				     BTRFS_MKFS_SYSTEM_GROUP_SIZE);
>  	allocation->system += BTRFS_MKFS_SYSTEM_GROUP_SIZE;
>  	if (ret)
>  		return ret;
> 

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

* Re: [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to check if the first device extent is occupying reserved 0~1M range
  2018-01-10  4:56 ` [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to check if the first device extent is occupying reserved 0~1M range Qu Wenruo
@ 2018-01-10  8:57   ` Nikolay Borisov
  0 siblings, 0 replies; 8+ messages in thread
From: Nikolay Borisov @ 2018-01-10  8:57 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: dsterba



On 10.01.2018 06:56, Qu Wenruo wrote:
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  .../mkfs-tests/010-reserved-1M-for-single/test.sh  | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100755 tests/mkfs-tests/010-reserved-1M-for-single/test.sh
> 
> diff --git a/tests/mkfs-tests/010-reserved-1M-for-single/test.sh b/tests/mkfs-tests/010-reserved-1M-for-single/test.sh
> new file mode 100755
> index 000000000000..1a4936eaf72e
> --- /dev/null
> +++ b/tests/mkfs-tests/010-reserved-1M-for-single/test.sh
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +# Test if "-m single" or "-M" can cause dev extent to use reserved 1M range
> +#
> +# Other profiles will cause mkfs.btrfs to allocate new meta/sys chunks
> +# using btrfs_alloc_chunk() which won't use 0~1M range, so other profiles
> +# are safe.
> +
> +source "$TOP/tests/common"
> +
> +check_prereq mkfs.btrfs
> +check_prereq btrfs
> +
> +prepare_test_dev
> +
> +do_one_test ()
> +{
> +	run_check "$TOP/mkfs.btrfs" -f $@ "$TEST_DEV"
> +
> +	# Use dev-extent tree to find first device extent
> +	first_dev_extent=$(run_check_stdout "$TOP/btrfs" inspect-internal \
> +		dump-tree -t device "$TEST_DEV" | \
> +		grep -oP '(?<=DEV_EXTENT )[[:digit:]]*' | head -n1)
> +
> +	if [ -z $first_dev_extent ]; then
> +		_fail "Failed to get first device extent"
> +	fi
> +
> +	echo "First dev extent starts at $first_dev_extent" >> "$RESULTS"
> +	echo "Reserved range is [0, $(( 1024 * 1024)))" >> "$RESULTS"
> +	# First device extent should not start below 1M
> +	if [ $first_dev_extent -lt $(( 1024 * 1024 )) ]; then
> +		_fail "First device extent occupies reserved 0~1M range"
> +	fi
> +}
> +
> +do_one_test "-M"
> +do_one_test "-m single"
> 

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

* Re: [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range
  2018-01-10  4:56 [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Qu Wenruo
  2018-01-10  4:56 ` [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to check if the first device extent is occupying reserved 0~1M range Qu Wenruo
  2018-01-10  8:37 ` [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Nikolay Borisov
@ 2018-01-10 14:14 ` Nikolay Borisov
  2018-01-10 14:27   ` Qu Wenruo
  2018-01-23 16:42 ` David Sterba
  3 siblings, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2018-01-10 14:14 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: dsterba



On 10.01.2018 06:56, Qu Wenruo wrote:
> When creating btrfs, mkfs.btrfs will firstly create a temporary system
> chunk as basis, and then created needed trees or new devices.
> 
> However the layout temporary system chunk is hard-coded and uses
> reserved [0, 1M) range of devid 1.
> 
> Change the temporary chunk layout from old:
> 
> 0	1M				4M	5M
> |<----------- temp chunk -------------->|
>   And it's 1:1 mapped, which means it's a SINGLE chunk,
>   and stripe offset is also 0.
> 
> to new layout:
> 
> 0	1M				4M	5M
> 	|<----------- temp chunk -------------->|
>   And still keeps the 1:1 mapping.
> 
> The problem can only be exposed by "-m single" or "-M" where we reuse the
> temporary chunk.
> 
> With other meta profiles, system and meta chunks are allocated by later
> btrfs_alloc_chunk() call, and old SINGLE chunks are removed, so it will
> be no such problem for other meta profiles.
> 
> Reported-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>


Those changes break existing xfs tests:

btrfs/140	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/140.out.bad)
    --- tests/btrfs/140.out	2017-05-22 13:24:36.116301772 +0000
    +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/140.out.bad	2018-01-10 14:12:09.919034975 +0000
    @@ -1,39 +1,39 @@
     QA output created by 140
     wrote 131072/131072 bytes at offset 0
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 136708096
    +wrote 65536/65536 bytes at offset 137756672
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
    ...
    (Run 'diff -u tests/btrfs/140.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/140.out.bad'  to see the entire diff)
btrfs/141	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/141.out.bad)
    --- tests/btrfs/141.out	2017-05-22 13:24:36.117301793 +0000
    +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/141.out.bad	2018-01-10 14:12:10.661035432 +0000
    @@ -1,39 +1,39 @@
     QA output created by 141
     wrote 131072/131072 bytes at offset 0
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 136708096
    +wrote 65536/65536 bytes at offset 137756672
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
    ...
    (Run 'diff -u tests/btrfs/141.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/141.out.bad'  to see the entire diff)
btrfs/142	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/142.out.bad)
    --- tests/btrfs/142.out	2017-05-22 13:24:36.117301793 +0000
    +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/142.out.bad	2018-01-10 14:12:11.411035894 +0000
    @@ -1,39 +1,39 @@
     QA output created by 142
     wrote 131072/131072 bytes at offset 0
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 136708096
    +wrote 65536/65536 bytes at offset 137756672
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
    ...
    (Run 'diff -u tests/btrfs/142.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/142.out.bad'  to see the entire diff)
btrfs/143	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/143.out.bad)
    --- tests/btrfs/143.out	2017-05-22 13:24:36.117301793 +0000
    +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/143.out.bad	2018-01-10 14:12:12.305036444 +0000
    @@ -1,39 +1,39 @@
     QA output created by 143
     wrote 131072/131072 bytes at offset 0
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 136708096
    +wrote 65536/65536 bytes at offset 137756672
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
    ...
    (Run 'diff -u tests/btrfs/143.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/143.out.bad'  to see the entire diff)


All of these are offset by a single megabyte

> ---
>  mkfs/common.c | 29 +++++++++++++++++++++++------
>  mkfs/main.c   |  7 ++++++-
>  2 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/mkfs/common.c b/mkfs/common.c
> index dd5e7ecff479..5c5e9c3b9e01 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -100,6 +100,21 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>   *
>   * The superblock signature is not valid, denotes a partially created
>   * filesystem, needs to be finalized.
> + *
> + * The temporary fs will have the following chunk layout:
> + * Device extent:
> + * 0		1M				5M	......
> + * | Reserved	| dev extent for SYS chunk      |
> + *
> + * And chunk mapping will be:
> + * Chunk mapping:
> + * 0		1M				5M
> + * |		| System chunk, 1:1 mapped	|
> + *
> + * That's to say, there will only be *ONE* system chunk, mapped to
> + * [1M, 5M) physical offset.
> + * And the only chunk is also in logical address [1M, 5M), containing
> + * all essential tree blocks.
>   */
>  int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  {
> @@ -154,8 +169,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  
>  	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
>  	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
> -		cfg->blocks[i] = BTRFS_SUPER_INFO_OFFSET + SZ_1M +
> -			cfg->nodesize * i;
> +		cfg->blocks[i] = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER +
> +			cfg->nodesize * (i - 1);
>  	}
>  
>  	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
> @@ -309,7 +324,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  
>  	/* then we have chunk 0 */
>  	btrfs_set_disk_key_objectid(&disk_key, BTRFS_FIRST_CHUNK_TREE_OBJECTID);
> -	btrfs_set_disk_key_offset(&disk_key, 0);
> +	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  	btrfs_set_disk_key_type(&disk_key, BTRFS_CHUNK_ITEM_KEY);
>  	btrfs_set_item_key(buf, &disk_key, nritems);
>  	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -325,7 +340,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  	btrfs_set_chunk_sector_size(buf, chunk, cfg->sectorsize);
>  	btrfs_set_chunk_num_stripes(buf, chunk, 1);
>  	btrfs_set_stripe_devid_nr(buf, chunk, 0, 1);
> -	btrfs_set_stripe_offset_nr(buf, chunk, 0, 0);
> +	btrfs_set_stripe_offset_nr(buf, chunk, 0,
> +				   BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  	nritems++;
>  
>  	write_extent_buffer(buf, super.dev_item.uuid,
> @@ -363,7 +379,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  		sizeof(struct btrfs_dev_extent);
>  
>  	btrfs_set_disk_key_objectid(&disk_key, 1);
> -	btrfs_set_disk_key_offset(&disk_key, 0);
> +	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  	btrfs_set_disk_key_type(&disk_key, BTRFS_DEV_EXTENT_KEY);
>  	btrfs_set_item_key(buf, &disk_key, nritems);
>  	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -374,7 +390,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  					BTRFS_CHUNK_TREE_OBJECTID);
>  	btrfs_set_dev_extent_chunk_objectid(buf, dev_extent,
>  					BTRFS_FIRST_CHUNK_TREE_OBJECTID);
> -	btrfs_set_dev_extent_chunk_offset(buf, dev_extent, 0);
> +	btrfs_set_dev_extent_chunk_offset(buf, dev_extent,
> +					  BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>  
>  	write_extent_buffer(buf, chunk_tree_uuid,
>  		    (unsigned long)btrfs_dev_extent_chunk_tree_uuid(dev_extent),
> diff --git a/mkfs/main.c b/mkfs/main.c
> index d817ad8dfd1a..8e3d19acb6f2 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -81,10 +81,15 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
>  	bytes_used = btrfs_super_bytes_used(fs_info->super_copy);
>  
>  	root->fs_info->system_allocs = 1;
> +	/*
> +	 * First temporary system chunk must match the chunk layout
> +	 * created in make_btrfs().
> +	 */
>  	ret = btrfs_make_block_group(trans, fs_info, bytes_used,
>  				     BTRFS_BLOCK_GROUP_SYSTEM,
>  				     BTRFS_FIRST_CHUNK_TREE_OBJECTID,
> -				     0, BTRFS_MKFS_SYSTEM_GROUP_SIZE);
> +				     BTRFS_BLOCK_RESERVED_1M_FOR_SUPER,
> +				     BTRFS_MKFS_SYSTEM_GROUP_SIZE);
>  	allocation->system += BTRFS_MKFS_SYSTEM_GROUP_SIZE;
>  	if (ret)
>  		return ret;
> 

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

* Re: [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range
  2018-01-10 14:14 ` Nikolay Borisov
@ 2018-01-10 14:27   ` Qu Wenruo
  0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2018-01-10 14:27 UTC (permalink / raw)
  To: Nikolay Borisov, Qu Wenruo, linux-btrfs; +Cc: dsterba


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



On 2018年01月10日 22:14, Nikolay Borisov wrote:
> 
> 
> On 10.01.2018 06:56, Qu Wenruo wrote:
>> When creating btrfs, mkfs.btrfs will firstly create a temporary system
>> chunk as basis, and then created needed trees or new devices.
>>
>> However the layout temporary system chunk is hard-coded and uses
>> reserved [0, 1M) range of devid 1.
>>
>> Change the temporary chunk layout from old:
>>
>> 0	1M				4M	5M
>> |<----------- temp chunk -------------->|
>>   And it's 1:1 mapped, which means it's a SINGLE chunk,
>>   and stripe offset is also 0.
>>
>> to new layout:
>>
>> 0	1M				4M	5M
>> 	|<----------- temp chunk -------------->|
>>   And still keeps the 1:1 mapping.
>>
>> The problem can only be exposed by "-m single" or "-M" where we reuse the
>> temporary chunk.
>>
>> With other meta profiles, system and meta chunks are allocated by later
>> btrfs_alloc_chunk() call, and old SINGLE chunks are removed, so it will
>> be no such problem for other meta profiles.
>>
>> Reported-by: Nikolay Borisov <nborisov@suse.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> 
> Those changes break existing xfs tests:

These tests are manually getting the on-disk data to verify if the
repair is done correctly.

And if underlying chunk mapping changed, old hard-coded (or less flex)
location will definitely fail.

I'll update these test cases to use a more flex way to get on-disk file
extent offset.

Thanks for the report,
Qu

> 
> btrfs/140	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/140.out.bad)
>     --- tests/btrfs/140.out	2017-05-22 13:24:36.116301772 +0000
>     +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/140.out.bad	2018-01-10 14:12:09.919034975 +0000
>     @@ -1,39 +1,39 @@
>      QA output created by 140
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 136708096
>     +wrote 65536/65536 bytes at offset 137756672
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>     ...
>     (Run 'diff -u tests/btrfs/140.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/140.out.bad'  to see the entire diff)
> btrfs/141	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/141.out.bad)
>     --- tests/btrfs/141.out	2017-05-22 13:24:36.117301793 +0000
>     +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/141.out.bad	2018-01-10 14:12:10.661035432 +0000
>     @@ -1,39 +1,39 @@
>      QA output created by 141
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 136708096
>     +wrote 65536/65536 bytes at offset 137756672
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>     ...
>     (Run 'diff -u tests/btrfs/141.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/141.out.bad'  to see the entire diff)
> btrfs/142	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/142.out.bad)
>     --- tests/btrfs/142.out	2017-05-22 13:24:36.117301793 +0000
>     +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/142.out.bad	2018-01-10 14:12:11.411035894 +0000
>     @@ -1,39 +1,39 @@
>      QA output created by 142
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 136708096
>     +wrote 65536/65536 bytes at offset 137756672
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>     ...
>     (Run 'diff -u tests/btrfs/142.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/142.out.bad'  to see the entire diff)
> btrfs/143	 - output mismatch (see /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/143.out.bad)
>     --- tests/btrfs/143.out	2017-05-22 13:24:36.117301793 +0000
>     +++ /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/143.out.bad	2018-01-10 14:12:12.305036444 +0000
>     @@ -1,39 +1,39 @@
>      QA output created by 143
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 136708096
>     +wrote 65536/65536 bytes at offset 137756672
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -08260000:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>     ...
>     (Run 'diff -u tests/btrfs/143.out /root/xfstests-dev/results/ubuntu-virtual/4.15.0-rc5-nbor/2018-01-10-13-41-845986621//btrfs/btrfs/143.out.bad'  to see the entire diff)
> 
> 
> All of these are offset by a single megabyte
> 
>> ---
>>  mkfs/common.c | 29 +++++++++++++++++++++++------
>>  mkfs/main.c   |  7 ++++++-
>>  2 files changed, 29 insertions(+), 7 deletions(-)
>>
>> diff --git a/mkfs/common.c b/mkfs/common.c
>> index dd5e7ecff479..5c5e9c3b9e01 100644
>> --- a/mkfs/common.c
>> +++ b/mkfs/common.c
>> @@ -100,6 +100,21 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>>   *
>>   * The superblock signature is not valid, denotes a partially created
>>   * filesystem, needs to be finalized.
>> + *
>> + * The temporary fs will have the following chunk layout:
>> + * Device extent:
>> + * 0		1M				5M	......
>> + * | Reserved	| dev extent for SYS chunk      |
>> + *
>> + * And chunk mapping will be:
>> + * Chunk mapping:
>> + * 0		1M				5M
>> + * |		| System chunk, 1:1 mapped	|
>> + *
>> + * That's to say, there will only be *ONE* system chunk, mapped to
>> + * [1M, 5M) physical offset.
>> + * And the only chunk is also in logical address [1M, 5M), containing
>> + * all essential tree blocks.
>>   */
>>  int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>>  {
>> @@ -154,8 +169,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>>  
>>  	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
>>  	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
>> -		cfg->blocks[i] = BTRFS_SUPER_INFO_OFFSET + SZ_1M +
>> -			cfg->nodesize * i;
>> +		cfg->blocks[i] = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER +
>> +			cfg->nodesize * (i - 1);
>>  	}
>>  
>>  	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
>> @@ -309,7 +324,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>>  
>>  	/* then we have chunk 0 */
>>  	btrfs_set_disk_key_objectid(&disk_key, BTRFS_FIRST_CHUNK_TREE_OBJECTID);
>> -	btrfs_set_disk_key_offset(&disk_key, 0);
>> +	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>>  	btrfs_set_disk_key_type(&disk_key, BTRFS_CHUNK_ITEM_KEY);
>>  	btrfs_set_item_key(buf, &disk_key, nritems);
>>  	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
>> @@ -325,7 +340,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>>  	btrfs_set_chunk_sector_size(buf, chunk, cfg->sectorsize);
>>  	btrfs_set_chunk_num_stripes(buf, chunk, 1);
>>  	btrfs_set_stripe_devid_nr(buf, chunk, 0, 1);
>> -	btrfs_set_stripe_offset_nr(buf, chunk, 0, 0);
>> +	btrfs_set_stripe_offset_nr(buf, chunk, 0,
>> +				   BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>>  	nritems++;
>>  
>>  	write_extent_buffer(buf, super.dev_item.uuid,
>> @@ -363,7 +379,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>>  		sizeof(struct btrfs_dev_extent);
>>  
>>  	btrfs_set_disk_key_objectid(&disk_key, 1);
>> -	btrfs_set_disk_key_offset(&disk_key, 0);
>> +	btrfs_set_disk_key_offset(&disk_key, BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>>  	btrfs_set_disk_key_type(&disk_key, BTRFS_DEV_EXTENT_KEY);
>>  	btrfs_set_item_key(buf, &disk_key, nritems);
>>  	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
>> @@ -374,7 +390,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>>  					BTRFS_CHUNK_TREE_OBJECTID);
>>  	btrfs_set_dev_extent_chunk_objectid(buf, dev_extent,
>>  					BTRFS_FIRST_CHUNK_TREE_OBJECTID);
>> -	btrfs_set_dev_extent_chunk_offset(buf, dev_extent, 0);
>> +	btrfs_set_dev_extent_chunk_offset(buf, dev_extent,
>> +					  BTRFS_BLOCK_RESERVED_1M_FOR_SUPER);
>>  
>>  	write_extent_buffer(buf, chunk_tree_uuid,
>>  		    (unsigned long)btrfs_dev_extent_chunk_tree_uuid(dev_extent),
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index d817ad8dfd1a..8e3d19acb6f2 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -81,10 +81,15 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
>>  	bytes_used = btrfs_super_bytes_used(fs_info->super_copy);
>>  
>>  	root->fs_info->system_allocs = 1;
>> +	/*
>> +	 * First temporary system chunk must match the chunk layout
>> +	 * created in make_btrfs().
>> +	 */
>>  	ret = btrfs_make_block_group(trans, fs_info, bytes_used,
>>  				     BTRFS_BLOCK_GROUP_SYSTEM,
>>  				     BTRFS_FIRST_CHUNK_TREE_OBJECTID,
>> -				     0, BTRFS_MKFS_SYSTEM_GROUP_SIZE);
>> +				     BTRFS_BLOCK_RESERVED_1M_FOR_SUPER,
>> +				     BTRFS_MKFS_SYSTEM_GROUP_SIZE);
>>  	allocation->system += BTRFS_MKFS_SYSTEM_GROUP_SIZE;
>>  	if (ret)
>>  		return ret;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

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

* Re: [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range
  2018-01-10  4:56 [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Qu Wenruo
                   ` (2 preceding siblings ...)
  2018-01-10 14:14 ` Nikolay Borisov
@ 2018-01-23 16:42 ` David Sterba
  2018-01-24  0:42   ` Qu Wenruo
  3 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2018-01-23 16:42 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, dsterba, nborisov

On Wed, Jan 10, 2018 at 12:56:47PM +0800, Qu Wenruo wrote:
> When creating btrfs, mkfs.btrfs will firstly create a temporary system
> chunk as basis, and then created needed trees or new devices.
> 
> However the layout temporary system chunk is hard-coded and uses
> reserved [0, 1M) range of devid 1.
> 
> Change the temporary chunk layout from old:
> 
> 0	1M				4M	5M
> |<----------- temp chunk -------------->|
>   And it's 1:1 mapped, which means it's a SINGLE chunk,
>   and stripe offset is also 0.
> 
> to new layout:
> 
> 0	1M				4M	5M
> 	|<----------- temp chunk -------------->|
>   And still keeps the 1:1 mapping.
> 
> The problem can only be exposed by "-m single" or "-M" where we reuse the
> temporary chunk.
> 
> With other meta profiles, system and meta chunks are allocated by later
> btrfs_alloc_chunk() call, and old SINGLE chunks are removed, so it will
> be no such problem for other meta profiles.
> 
> Reported-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

The test mkfs-tests/010-minimal-size fails with this patch (devel
branch). I've added some debugging and the requested minimal size by
mkfs is not sufficient. I think it's because of the 1MB shift but
haven't looked closely.

The math in "btrfs-progs: mkfs: move source dir size calculation to its
own files" may need to be updated, I'm not sure how exactly to do that.
It would be good if you find the fix and we'll then see where to put it
(separate patch or fold it to some other one).

Otherwise I'm going to apply patches 1 and 2, thanks.

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

* Re: [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range
  2018-01-23 16:42 ` David Sterba
@ 2018-01-24  0:42   ` Qu Wenruo
  0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2018-01-24  0:42 UTC (permalink / raw)
  To: dsterba, Qu Wenruo, linux-btrfs, nborisov


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



On 2018年01月24日 00:42, David Sterba wrote:
> On Wed, Jan 10, 2018 at 12:56:47PM +0800, Qu Wenruo wrote:
>> When creating btrfs, mkfs.btrfs will firstly create a temporary system
>> chunk as basis, and then created needed trees or new devices.
>>
>> However the layout temporary system chunk is hard-coded and uses
>> reserved [0, 1M) range of devid 1.
>>
>> Change the temporary chunk layout from old:
>>
>> 0	1M				4M	5M
>> |<----------- temp chunk -------------->|
>>   And it's 1:1 mapped, which means it's a SINGLE chunk,
>>   and stripe offset is also 0.
>>
>> to new layout:
>>
>> 0	1M				4M	5M
>> 	|<----------- temp chunk -------------->|
>>   And still keeps the 1:1 mapping.
>>
>> The problem can only be exposed by "-m single" or "-M" where we reuse the
>> temporary chunk.
>>
>> With other meta profiles, system and meta chunks are allocated by later
>> btrfs_alloc_chunk() call, and old SINGLE chunks are removed, so it will
>> be no such problem for other meta profiles.
>>
>> Reported-by: Nikolay Borisov <nborisov@suse.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> The test mkfs-tests/010-minimal-size fails with this patch (devel
> branch). I've added some debugging and the requested minimal size by
> mkfs is not sufficient. I think it's because of the 1MB shift but
> haven't looked closely.

Patch under the way.

And this reminds me to refactor btrfs_alloc_chunk() to allow caller get
minimal chunk size to use other than manually read the code.

Thanks,
Qu

> 
> The math in "btrfs-progs: mkfs: move source dir size calculation to its
> own files" may need to be updated, I'm not sure how exactly to do that.
> It would be good if you find the fix and we'll then see where to put it
> (separate patch or fold it to some other one).
> 
> Otherwise I'm going to apply patches 1 and 2, thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

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

end of thread, other threads:[~2018-01-24  0:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10  4:56 [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Qu Wenruo
2018-01-10  4:56 ` [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to check if the first device extent is occupying reserved 0~1M range Qu Wenruo
2018-01-10  8:57   ` Nikolay Borisov
2018-01-10  8:37 ` [PATCH 1/2] btrfs-progs: mkfs: Prevent temporary system chunk to use space in reserved 1M range Nikolay Borisov
2018-01-10 14:14 ` Nikolay Borisov
2018-01-10 14:27   ` Qu Wenruo
2018-01-23 16:42 ` David Sterba
2018-01-24  0:42   ` Qu Wenruo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.