All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all temporary chunks
@ 2021-10-11  9:42 Qu Wenruo
  2021-10-11  9:42 ` [PATCH 1/3] btrfs-progs: rename @data parameter to @profile in extent allocation path Qu Wenruo
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Qu Wenruo @ 2021-10-11  9:42 UTC (permalink / raw)
  To: linux-btrfs

There is a bug report that with certain mkfs options, mkfs.btrfs may
fail to cleanup some temporary chunks, leading to "btrfs filesystem df"
warning about multiple profiles:

  WARNING: Multiple block group profiles detected, see 'man btrfs(5)'.
  WARNING:   Metadata: single, raid1 

The easiest way to reproduce is "mkfs.btrfs -f -R free-space-tree -m dup
-d dup".

It turns out that, the old _recow_root() can not handle tree levels > 0,
while with newer free space tree creation timing, the free space tree
can reach level 1 or higher.

To fix the problem, Patch 2 will do the proper full tree re-CoW, with
extra transaction commitment to make sure all free space tree get
re-CoWed.

The 3rd patch will do the extra verification during mkfs-tests.

The first patch is just to fix a confusing parameter which also caused
u64 -> int width reduction and can be problematic in the future.

Qu Wenruo (3):
  btrfs-progs: rename @data parameter to @profile in extent allocation
    path
  btrfs-progs: mkfs: recow all tree blocks properly
  btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary
    chunks

 kernel-shared/extent-tree.c                 | 26 +++---
 mkfs/main.c                                 | 87 ++++++++++++++++++---
 tests/mkfs-tests/001-basic-profiles/test.sh | 10 ++-
 3 files changed, 97 insertions(+), 26 deletions(-)

-- 
2.33.0


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

* [PATCH 1/3] btrfs-progs: rename @data parameter to @profile in extent allocation path
  2021-10-11  9:42 [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all temporary chunks Qu Wenruo
@ 2021-10-11  9:42 ` Qu Wenruo
  2021-10-11  9:42 ` [PATCH 2/3] btrfs-progs: mkfs: recow all tree blocks properly Qu Wenruo
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2021-10-11  9:42 UTC (permalink / raw)
  To: linux-btrfs

In function btrfs_reserve_extent(), we call find_free_extent() passing
"u64 profile" into "int data".

This is definitely a width reduction, but when looking further into the
code, it's more serious than that, in fact the "int data" parameter is
not really to indicate whether it's data extent, but really a block
group profile (with block group type).

This is not only width reduction, but also confusing.

Thankfully so for we don't have any BLOCK_GROUP bits beyond 32 bits, so
the width reduction is not causing a big problem.

This patch will rename the "int data" parameter to a more proper one,
"u64 profile" in all involved call paths.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/extent-tree.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/kernel-shared/extent-tree.c b/kernel-shared/extent-tree.c
index 0d62a14ea723..436948624843 100644
--- a/kernel-shared/extent-tree.c
+++ b/kernel-shared/extent-tree.c
@@ -54,7 +54,7 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 			 u64 owner_offset, int refs_to_drop);
 static struct btrfs_block_group *
 btrfs_find_block_group(struct btrfs_root *root, struct btrfs_block_group
-		       *hint, u64 search_start, int data, int owner);
+		       *hint, u64 search_start, u64 profile, int owner);
 
 static int remove_sb_from_cache(struct btrfs_root *root,
 				struct btrfs_block_group *cache)
@@ -264,7 +264,7 @@ static int block_group_bits(struct btrfs_block_group *cache, u64 bits)
 
 static int noinline find_search_start(struct btrfs_root *root,
 			      struct btrfs_block_group **cache_ret,
-			      u64 *start_ret, int num, int data)
+			      u64 *start_ret, int num, u64 profile)
 {
 	int ret;
 	struct btrfs_block_group *cache = *cache_ret;
@@ -282,7 +282,7 @@ again:
 		goto out;
 
 	last = max(search_start, cache->start);
-	if (cache->ro || !block_group_bits(cache, data))
+	if (cache->ro || !block_group_bits(cache, profile))
 		goto new_group;
 
 	if (btrfs_is_zoned(root->fs_info)) {
@@ -339,7 +339,7 @@ wrapped:
 
 static struct btrfs_block_group *
 btrfs_find_block_group(struct btrfs_root *root, struct btrfs_block_group
-		       *hint, u64 search_start, int data, int owner)
+		       *hint, u64 search_start, u64 profile, int owner)
 {
 	struct btrfs_block_group *cache;
 	struct btrfs_block_group *found_group = NULL;
@@ -357,7 +357,7 @@ btrfs_find_block_group(struct btrfs_root *root, struct btrfs_block_group
 	if (search_start) {
 		struct btrfs_block_group *shint;
 		shint = btrfs_lookup_block_group(info, search_start);
-		if (shint && !shint->ro && block_group_bits(shint, data)) {
+		if (shint && !shint->ro && block_group_bits(shint, profile)) {
 			used = shint->used;
 			if (used + shint->pinned <
 			    div_factor(shint->length, factor)) {
@@ -365,7 +365,7 @@ btrfs_find_block_group(struct btrfs_root *root, struct btrfs_block_group
 			}
 		}
 	}
-	if (hint && !hint->ro && block_group_bits(hint, data)) {
+	if (hint && !hint->ro && block_group_bits(hint, profile)) {
 		used = hint->used;
 		if (used + hint->pinned <
 		    div_factor(hint->length, factor)) {
@@ -390,7 +390,7 @@ again:
 		last = cache->start + cache->length;
 		used = cache->used;
 
-		if (!cache->ro && block_group_bits(cache, data)) {
+		if (!cache->ro && block_group_bits(cache, profile)) {
 			if (full_search)
 				free_check = cache->length;
 			else
@@ -2177,7 +2177,7 @@ static int noinline find_free_extent(struct btrfs_trans_handle *trans,
 				     u64 search_start, u64 search_end,
 				     u64 hint_byte, struct btrfs_key *ins,
 				     u64 exclude_start, u64 exclude_nr,
-				     int data)
+				     u64 profile)
 {
 	int ret;
 	u64 orig_search_start = search_start;
@@ -2198,11 +2198,11 @@ static int noinline find_free_extent(struct btrfs_trans_handle *trans,
 		if (!block_group)
 			hint_byte = search_start;
 		block_group = btrfs_find_block_group(root, block_group,
-						     hint_byte, data, 1);
+						     hint_byte, profile, 1);
 	} else {
 		block_group = btrfs_find_block_group(root,
 						     trans->block_group,
-						     search_start, data, 1);
+						     search_start, profile, 1);
 	}
 
 	total_needed += empty_size;
@@ -2217,7 +2217,7 @@ check_failed:
 						       orig_search_start);
 	}
 	ret = find_search_start(root, &block_group, &search_start,
-				total_needed, data);
+				total_needed, profile);
 	if (ret)
 		goto new_group;
 
@@ -2255,7 +2255,7 @@ check_failed:
 		goto new_group;
 	}
 
-	if (!(data & BTRFS_BLOCK_GROUP_DATA)) {
+	if (!(profile & BTRFS_BLOCK_GROUP_DATA)) {
 		if (check_crossing_stripes(info, ins->objectid, num_bytes)) {
 			struct btrfs_block_group *bg_cache;
 			u64 bg_offset;
@@ -2295,7 +2295,7 @@ new_group:
 	}
 	cond_resched();
 	block_group = btrfs_find_block_group(root, block_group,
-					     search_start, data, 0);
+					     search_start, profile, 0);
 	goto check_failed;
 
 error:
-- 
2.33.0


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

* [PATCH 2/3] btrfs-progs: mkfs: recow all tree blocks properly
  2021-10-11  9:42 [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all temporary chunks Qu Wenruo
  2021-10-11  9:42 ` [PATCH 1/3] btrfs-progs: rename @data parameter to @profile in extent allocation path Qu Wenruo
@ 2021-10-11  9:42 ` Qu Wenruo
  2021-10-11  9:43 ` [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks Qu Wenruo
  2021-10-11 10:31 ` [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all " David Sterba
  3 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2021-10-11  9:42 UTC (permalink / raw)
  To: linux-btrfs; +Cc: FireFish5000

[BUG]
Since btrfs-progs v5.14, mkfs.btrfs no longer cleans up the temporary
SINGLE metadata chunks if "-R free-space-tree" is specified:

 $ mkfs.btrfs  -f -R free-space-tree -m dup -d dup /dev/test/test
 $ btrfs ins dump-tree -t chunk /dev/test/test | grep "type METADATA"
		length 8388608 owner 2 stripe_len 65536 type METADATA
		length 268435456 owner 2 stripe_len 65536 type METADATA|DUP

[CAUSE]
Since commit 4b6cf2a3eb78 ("btrfs-progs: mkfs: generate free space tree
at make_btrfs() time"), free space tree is created when the temporary
btrfs image is created.

This behavior itself has no problem at all.

The problem happens when "-m DUP -d DUP" (or other profiles) is
specified.

This makes btrfs to create extra chunks, enlarging free space tree so
that it can be as high as level 1.

During mkfs, we rely on recow_roots() to re-CoW all tree blocks to the
newly allocated chunks.

But __recow_root() can only handle tree root at level 0, as it forces
root node to be CoWed, not bothering the children leaves/nodes.

This makes part of the free space cache tree still live on the old
temporary chunks, leaving later cleanup_temp_chunks() unable to delete
temporary SINGLE chunks.

[FIX]
Rework __recow_root() to do a proper CoW of the whole tree.

But above rework is not enough, as if a free space tree block is
allocated during current transaction, but before new chunks added.
Then the reworked __recow_root() can't CoW it, as btrfs_search_slot()
won't CoW a tree block allocated in current transaction.

So this patch will also commit current transaction before calling
recow_roots(), to force us to re-cow all tree blocks.

This shouldn't be a problem, as at the time of calling, we should have
less than a dozen tree blocks, thus there won't be a performance impact.

Reported-by: FireFish5000 <firefish5000@gmail.com>
Fixes: 4b6cf2a3eb78 ("btrfs-progs: mkfs: generate free space tree at make_btrfs() time")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 mkfs/main.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 76 insertions(+), 11 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index 4d50bd25c440..f8a1becce9bf 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -209,21 +209,59 @@ err:
 }
 
 static int __recow_root(struct btrfs_trans_handle *trans,
-			 struct btrfs_root *root)
+			struct btrfs_root *root)
 {
-	struct extent_buffer *tmp;
+	struct btrfs_path path;
+	struct btrfs_key key;
 	int ret;
 
-	if (trans->transid != btrfs_root_generation(&root->root_item)) {
-		extent_buffer_get(root->node);
-		ret = __btrfs_cow_block(trans, root, root->node,
-					NULL, 0, &tmp, 0, 0);
-		if (ret)
-			return ret;
-		free_extent_buffer(tmp);
-	}
+	btrfs_init_path(&path);
+	key.objectid = 0;
+	key.type = 0;
+	key.offset = 0;
 
-	return 0;
+	/* Get a path to the most-left leaves */
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	while (true) {
+		struct btrfs_key found_key;
+
+		/*
+		 * Our parent nodes must be no newer than the leaf, thus
+		 * if the leaf is as new as the trans, no need to re-cow.
+		 */
+		if (btrfs_header_generation(path.nodes[0]) == trans->transid)
+			goto next;
+
+		/*
+		 * Grab the key of current tree block and do a CoW search to
+		 * the current tree block.
+		 */
+		btrfs_item_key_to_cpu(path.nodes[0], &key, 0);
+		btrfs_release_path(&path);
+
+		/* This will ensure this leaf and all its parent get CoWed */
+		ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
+		if (ret < 0)
+			goto out;
+		ret = 0;
+		btrfs_item_key_to_cpu(path.nodes[0], &found_key, 0);
+		ASSERT(btrfs_comp_cpu_keys(&key, &found_key) == 0);
+
+next:
+		ret = btrfs_next_sibling_tree_block(trans->fs_info, &path);
+		if (ret < 0)
+			goto out;
+		if (ret > 0) {
+			ret = 0;
+			goto out;
+		}
+	}
+out:
+	btrfs_release_path(&path);
+	return ret;
 }
 
 static int recow_roots(struct btrfs_trans_handle *trans,
@@ -1472,6 +1510,33 @@ raid_groups:
 		goto out;
 	}
 
+	/*
+	 * Commit current trans so we can cow all existing tree blocks
+	 * to newly created raid groups.
+	 * As currently we use btrfs_search_slot() to CoW tree blocks in
+	 * recow_roots(), if a tree block is already modified in current trans,
+	 * it won't be re-CoWed, thus it will stay in temporary chunks.
+	 */
+	ret = btrfs_commit_transaction(trans, root);
+	if (ret) {
+		errno = -ret;
+		error("unable to commit transaction before recowing trees: %m");
+		goto out;
+	}
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		errno = -PTR_ERR(trans);
+		error("failed to start transaction: %m");
+		goto error;
+	}
+	/* CoW all tree blocks to newly created chunks */
+	ret = recow_roots(trans, root);
+	if (ret) {
+		errno = -ret;
+		error("unable to CoW tree blocks to new profiles: %m");
+		goto out;
+	}
+
 	ret = create_data_reloc_tree(trans);
 	if (ret) {
 		error("unable to create data reloc tree: %d", ret);
-- 
2.33.0


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

* [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks
  2021-10-11  9:42 [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all temporary chunks Qu Wenruo
  2021-10-11  9:42 ` [PATCH 1/3] btrfs-progs: rename @data parameter to @profile in extent allocation path Qu Wenruo
  2021-10-11  9:42 ` [PATCH 2/3] btrfs-progs: mkfs: recow all tree blocks properly Qu Wenruo
@ 2021-10-11  9:43 ` Qu Wenruo
  2021-10-11 10:29   ` David Sterba
  2021-10-11 10:40   ` David Sterba
  2021-10-11 10:31 ` [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all " David Sterba
  3 siblings, 2 replies; 8+ messages in thread
From: Qu Wenruo @ 2021-10-11  9:43 UTC (permalink / raw)
  To: linux-btrfs

Since current "btrfs filesystem df" command will warn if there are
multiple profiles of the same type, it's a good way to detect left-over
temporary chunks.

This patch will enhance the existing mkfs-tests/001-basic-profiles test
case to also check for the warning messages, to make sure mkfs.btrfs has
properly cleaned up all temporary chunks.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/mkfs-tests/001-basic-profiles/test.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/mkfs-tests/001-basic-profiles/test.sh b/tests/mkfs-tests/001-basic-profiles/test.sh
index b3ba50d71ddc..e44f9344bc6f 100755
--- a/tests/mkfs-tests/001-basic-profiles/test.sh
+++ b/tests/mkfs-tests/001-basic-profiles/test.sh
@@ -11,11 +11,17 @@ setup_root_helper
 
 test_get_info()
 {
+	tmp_out=$(mktemp --tmpdir btrfs-progs-mkfs-tests-get-info.XXXXXX)
 	run_check $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super "$dev1"
 	run_check $SUDO_HELPER "$TOP/btrfs" check "$dev1"
 	run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT"
-	run_check "$TOP/btrfs" filesystem df "$TEST_MNT"
-	run_check $SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
+	run_check_stdout "$TOP/btrfs" filesystem df "$TEST_MNT" > "$tmp_out"
+	if grep -q "Multiple block group profiles detected" "$tmp_out"; then
+		rm -- "$tmp_out"
+		_fail "temporary chunks are not properly cleaned up"
+	fi
+	rm -- "$tmp_out"
+	run_check$SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
 	run_check $SUDO_HELPER "$TOP/btrfs" device usage "$TEST_MNT"
 	run_check $SUDO_HELPER umount "$TEST_MNT"
 }
-- 
2.33.0


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

* Re: [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks
  2021-10-11  9:43 ` [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks Qu Wenruo
@ 2021-10-11 10:29   ` David Sterba
  2021-10-11 10:40   ` David Sterba
  1 sibling, 0 replies; 8+ messages in thread
From: David Sterba @ 2021-10-11 10:29 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Mon, Oct 11, 2021 at 05:43:00PM +0800, Qu Wenruo wrote:
> Since current "btrfs filesystem df" command will warn if there are
> multiple profiles of the same type, it's a good way to detect left-over
> temporary chunks.
> 
> This patch will enhance the existing mkfs-tests/001-basic-profiles test
> case to also check for the warning messages, to make sure mkfs.btrfs has
> properly cleaned up all temporary chunks.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/mkfs-tests/001-basic-profiles/test.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/mkfs-tests/001-basic-profiles/test.sh b/tests/mkfs-tests/001-basic-profiles/test.sh
> index b3ba50d71ddc..e44f9344bc6f 100755
> --- a/tests/mkfs-tests/001-basic-profiles/test.sh
> +++ b/tests/mkfs-tests/001-basic-profiles/test.sh
> @@ -11,11 +11,17 @@ setup_root_helper
>  
>  test_get_info()
>  {
> +	tmp_out=$(mktemp --tmpdir btrfs-progs-mkfs-tests-get-info.XXXXXX)
>  	run_check $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super "$dev1"
>  	run_check $SUDO_HELPER "$TOP/btrfs" check "$dev1"
>  	run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT"
> -	run_check "$TOP/btrfs" filesystem df "$TEST_MNT"
> -	run_check $SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
> +	run_check_stdout "$TOP/btrfs" filesystem df "$TEST_MNT" > "$tmp_out"
> +	if grep -q "Multiple block group profiles detected" "$tmp_out"; then
> +		rm -- "$tmp_out"
> +		_fail "temporary chunks are not properly cleaned up"
> +	fi
> +	rm -- "$tmp_out"
> +	run_check$SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
                 ^
Missing space so the test won't run properly


>  	run_check $SUDO_HELPER "$TOP/btrfs" device usage "$TEST_MNT"
>  	run_check $SUDO_HELPER umount "$TEST_MNT"
>  }
> -- 
> 2.33.0

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

* Re: [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all temporary chunks
  2021-10-11  9:42 [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all temporary chunks Qu Wenruo
                   ` (2 preceding siblings ...)
  2021-10-11  9:43 ` [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks Qu Wenruo
@ 2021-10-11 10:31 ` David Sterba
  3 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2021-10-11 10:31 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Mon, Oct 11, 2021 at 05:42:57PM +0800, Qu Wenruo wrote:
> There is a bug report that with certain mkfs options, mkfs.btrfs may
> fail to cleanup some temporary chunks, leading to "btrfs filesystem df"
> warning about multiple profiles:
> 
>   WARNING: Multiple block group profiles detected, see 'man btrfs(5)'.
>   WARNING:   Metadata: single, raid1 
> 
> The easiest way to reproduce is "mkfs.btrfs -f -R free-space-tree -m dup
> -d dup".

Running with -R free-space-tree has revealed several bugs already, it's
going to be default in 5.15 so hopefully we'll catch them all.

> It turns out that, the old _recow_root() can not handle tree levels > 0,
> while with newer free space tree creation timing, the free space tree
> can reach level 1 or higher.
> 
> To fix the problem, Patch 2 will do the proper full tree re-CoW, with
> extra transaction commitment to make sure all free space tree get
> re-CoWed.
> 
> The 3rd patch will do the extra verification during mkfs-tests.
> 
> The first patch is just to fix a confusing parameter which also caused
> u64 -> int width reduction and can be problematic in the future.
> 
> Qu Wenruo (3):
>   btrfs-progs: rename @data parameter to @profile in extent allocation
>     path
>   btrfs-progs: mkfs: recow all tree blocks properly
>   btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary
>     chunks

Thanks, added to devel.

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

* Re: [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks
  2021-10-11  9:43 ` [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks Qu Wenruo
  2021-10-11 10:29   ` David Sterba
@ 2021-10-11 10:40   ` David Sterba
  2021-10-11 11:12     ` Qu Wenruo
  1 sibling, 1 reply; 8+ messages in thread
From: David Sterba @ 2021-10-11 10:40 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Mon, Oct 11, 2021 at 05:43:00PM +0800, Qu Wenruo wrote:
> Since current "btrfs filesystem df" command will warn if there are
> multiple profiles of the same type, it's a good way to detect left-over
> temporary chunks.
> 
> This patch will enhance the existing mkfs-tests/001-basic-profiles test
> case to also check for the warning messages, to make sure mkfs.btrfs has
> properly cleaned up all temporary chunks.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/mkfs-tests/001-basic-profiles/test.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/mkfs-tests/001-basic-profiles/test.sh b/tests/mkfs-tests/001-basic-profiles/test.sh
> index b3ba50d71ddc..e44f9344bc6f 100755
> --- a/tests/mkfs-tests/001-basic-profiles/test.sh
> +++ b/tests/mkfs-tests/001-basic-profiles/test.sh
> @@ -11,11 +11,17 @@ setup_root_helper
>  
>  test_get_info()
>  {
> +	tmp_out=$(mktemp --tmpdir btrfs-progs-mkfs-tests-get-info.XXXXXX)
>  	run_check $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super "$dev1"
>  	run_check $SUDO_HELPER "$TOP/btrfs" check "$dev1"
>  	run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT"
> -	run_check "$TOP/btrfs" filesystem df "$TEST_MNT"
> -	run_check $SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
> +	run_check_stdout "$TOP/btrfs" filesystem df "$TEST_MNT" > "$tmp_out"
> +	if grep -q "Multiple block group profiles detected" "$tmp_out"; then
> +		rm -- "$tmp_out"
> +		_fail "temporary chunks are not properly cleaned up"
> +	fi
> +	rm -- "$tmp_out"
> +	run_check$SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
>  	run_check $SUDO_HELPER "$TOP/btrfs" device usage "$TEST_MNT"
>  	run_check $SUDO_HELPER umount "$TEST_MNT"

This still fails in case mkfs/001-basic-tests with '-d raid0 -m raid0'

====== RUN CHECK root_helper mount /dev/loop0 .../btrfs-progs/tests/mnt
====== RUN CHECK .../btrfs-progs/btrfs filesystem df .../btrfs-progs/tests/mnt
WARNING: Multiple block group profiles detected, see 'man btrfs(5)'.
WARNING:   Metadata: single, raid0
WARNING:   System: single, raid0
Data, raid0: total=204.75MiB, used=0.00B
System, raid0: total=8.00MiB, used=0.00B
System, single: total=32.00MiB, used=16.00KiB
Metadata, raid0: total=204.75MiB, used=128.00KiB
Metadata, single: total=208.00MiB, used=0.00B
GlobalReserve, unknown: total=3.25MiB, used=0.00B
temporary chunks are not properly cleaned up
test failed for case 001-basic-profiles


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

* Re: [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks
  2021-10-11 10:40   ` David Sterba
@ 2021-10-11 11:12     ` Qu Wenruo
  0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2021-10-11 11:12 UTC (permalink / raw)
  To: dsterba, Qu Wenruo, linux-btrfs



On 2021/10/11 18:40, David Sterba wrote:
> On Mon, Oct 11, 2021 at 05:43:00PM +0800, Qu Wenruo wrote:
>> Since current "btrfs filesystem df" command will warn if there are
>> multiple profiles of the same type, it's a good way to detect left-over
>> temporary chunks.
>>
>> This patch will enhance the existing mkfs-tests/001-basic-profiles test
>> case to also check for the warning messages, to make sure mkfs.btrfs has
>> properly cleaned up all temporary chunks.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   tests/mkfs-tests/001-basic-profiles/test.sh | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/mkfs-tests/001-basic-profiles/test.sh b/tests/mkfs-tests/001-basic-profiles/test.sh
>> index b3ba50d71ddc..e44f9344bc6f 100755
>> --- a/tests/mkfs-tests/001-basic-profiles/test.sh
>> +++ b/tests/mkfs-tests/001-basic-profiles/test.sh
>> @@ -11,11 +11,17 @@ setup_root_helper
>>
>>   test_get_info()
>>   {
>> +	tmp_out=$(mktemp --tmpdir btrfs-progs-mkfs-tests-get-info.XXXXXX)
>>   	run_check $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super "$dev1"
>>   	run_check $SUDO_HELPER "$TOP/btrfs" check "$dev1"
>>   	run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT"
>> -	run_check "$TOP/btrfs" filesystem df "$TEST_MNT"
>> -	run_check $SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
>> +	run_check_stdout "$TOP/btrfs" filesystem df "$TEST_MNT" > "$tmp_out"
>> +	if grep -q "Multiple block group profiles detected" "$tmp_out"; then
>> +		rm -- "$tmp_out"
>> +		_fail "temporary chunks are not properly cleaned up"
>> +	fi
>> +	rm -- "$tmp_out"
>> +	run_check$SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
>>   	run_check $SUDO_HELPER "$TOP/btrfs" device usage "$TEST_MNT"
>>   	run_check $SUDO_HELPER umount "$TEST_MNT"
>
> This still fails in case mkfs/001-basic-tests with '-d raid0 -m raid0'
>
> ====== RUN CHECK root_helper mount /dev/loop0 .../btrfs-progs/tests/mnt
> ====== RUN CHECK .../btrfs-progs/btrfs filesystem df .../btrfs-progs/tests/mnt
> WARNING: Multiple block group profiles detected, see 'man btrfs(5)'.
> WARNING:   Metadata: single, raid0
> WARNING:   System: single, raid0
> Data, raid0: total=204.75MiB, used=0.00B
> System, raid0: total=8.00MiB, used=0.00B
> System, single: total=32.00MiB, used=16.00KiB
> Metadata, raid0: total=204.75MiB, used=128.00KiB
> Metadata, single: total=208.00MiB, used=0.00B
> GlobalReserve, unknown: total=3.25MiB, used=0.00B
> temporary chunks are not properly cleaned up
> test failed for case 001-basic-profiles
>
It passes here (as I'm using sudo make test-mkfs, the missing space is
not causing problem).

BTW, my branch is based on v5.14.2 directly, thus I guess there may be
some extra bugs introduced in devel.

Mind to test my branch directly?
https://github.com/adam900710/btrfs-progs/tree/mkfs_profile_regression

Let me re-check when rebased to devel branch.

Thanks,
Qu

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

end of thread, other threads:[~2021-10-11 11:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11  9:42 [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all temporary chunks Qu Wenruo
2021-10-11  9:42 ` [PATCH 1/3] btrfs-progs: rename @data parameter to @profile in extent allocation path Qu Wenruo
2021-10-11  9:42 ` [PATCH 2/3] btrfs-progs: mkfs: recow all tree blocks properly Qu Wenruo
2021-10-11  9:43 ` [PATCH 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks Qu Wenruo
2021-10-11 10:29   ` David Sterba
2021-10-11 10:40   ` David Sterba
2021-10-11 11:12     ` Qu Wenruo
2021-10-11 10:31 ` [PATCH 0/3] btrfs-progs: mkfs: make sure we can clean up all " David Sterba

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.