[PATCH 0/2] btrfs-progs: mkfs: fix mkfs --rootdir size problem

[PATCH 0/2] btrfs-progs: mkfs: fix mkfs --rootdir size problem

[Qu Wenruo]

There is a bug report that mkfs.btrfs with -b limit, -d dup, and
--rootdir with some contents filled, leads to unexpected ENOSPC.

It turns out that, the new chunk allocation code is broken quite some
time ago, and by coincident we're using SINGLE data profile by default,
it doesn't get exposed through existing test cases.

The patchset will fix it and add corresponding test case for it.

Qu Wenruo (2):
  btrfs-progs: fix wrong chunk profile for do_chunk_alloc()
  btrfs-progs: mkfs-tests: Add test case to verify the --rootdir size
    limit

 extent-tree.c                             | 14 ++++++++++----
 tests/mkfs-tests/021-rootdir-size/test.sh | 22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)
 create mode 100755 tests/mkfs-tests/021-rootdir-size/test.sh

-- 
2.27.0

[PATCH 1/2] btrfs-progs: fix wrong chunk profile for do_chunk_alloc()

[Qu Wenruo]

[BUG]
There is a bug report that using DUP data profile, with a 400MiB source
dir, on a 7G disk leads to mkfs failure caused by ENOSPC.

[CAUSE]
After some debugging, it turns out that do_chunk_alloc() is always
passing SINGLE profile for new chunks.

The offending code looks like: extent-tree.c:: do_chunk_alloc()

	ret = btrfs_alloc_chunk(trans, fs_info, &start, &num_bytes,
	                        space_info->flags);

However since commit bce7dbba2859 ("Btrfs-progs: only build space info's
for the main flags"), we no longer store the profile bits in space_info
anymore.

This makes space_info never get updated properly, and causing us to
creating more and more chunks to eat up most of the disk with unused
SINGLE chunks, and finally leads to ENOSPC.

[FIX]
Fix the bug by passing the proper flags to btrfs_alloc_chunk().
Also, to address the original problem commit 2689259501c1 ("btrfs progs:
fix extra metadata chunk allocation in --mixed case") tries to fix, here
we do extra bit OR to ensure we get the proper flags.

Issue: #258
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 extent-tree.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/extent-tree.c b/extent-tree.c
index 4af8f4ba8a47..1cb956382c3b 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1721,8 +1721,14 @@ static int do_chunk_alloc(struct btrfs_trans_handle *trans,
 		return 0;
 	trans->allocating_chunk = 1;
 
-	ret = btrfs_alloc_chunk(trans, fs_info, &start, &num_bytes,
-	                        space_info->flags);
+	/*
+	 * The space_info only has block group type (data/meta/sys), doesn't
+	 * have the proper profile.
+	 * While we still want to handle mixed block groups properly.
+	 * So here add the extra bits for mixed profile.
+	 */
+	flags |= space_info->flags;
+	ret = btrfs_alloc_chunk(trans, fs_info, &start, &num_bytes, flags);
 	if (ret == -ENOSPC) {
 		space_info->full = 1;
 		trans->allocating_chunk = 0;
@@ -1731,8 +1737,8 @@ static int do_chunk_alloc(struct btrfs_trans_handle *trans,
 
 	BUG_ON(ret);
 
-	ret = btrfs_make_block_group(trans, fs_info, 0, space_info->flags,
-				     start, num_bytes);
+	ret = btrfs_make_block_group(trans, fs_info, 0, flags, start,
+				     num_bytes);
 	BUG_ON(ret);
 	trans->allocating_chunk = 0;
 	return 0;
-- 
2.27.0

[PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to verify the --rootdir size limit

[Qu Wenruo]

Add a test case to ensure we can create a 350M fs with 128M rootdir.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/mkfs-tests/021-rootdir-size/test.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100755 tests/mkfs-tests/021-rootdir-size/test.sh

diff --git a/tests/mkfs-tests/021-rootdir-size/test.sh b/tests/mkfs-tests/021-rootdir-size/test.sh
new file mode 100755
index 000000000000..064476fdc847
--- /dev/null
+++ b/tests/mkfs-tests/021-rootdir-size/test.sh
@@ -0,0 +1,22 @@
+#!/bin/bash
+# Regression test for mkfs.btrfs --rootdir with DUP data profile and rootdir
+# size near the limit of the device.
+#
+# There is a bug that makes mkfs.btrfs always to create unnecessary SINGLE
+# chunks, which eats up a lot of space and leads to unexpected ENOSPC bugs.
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+prepare_test_dev
+
+tmp=$(mktemp -d --tmpdir btrfs-progs-mkfs.rootdirXXXXXXX)
+
+fallocate -l 128M $tmp/large_file
+
+# We should be able to create the fs with size limit to 2 * (128 + 32 + 8)
+# which is 336M. Here we round it up to 350M.
+run_check "$TOP/mkfs.btrfs" -f --rootdir "$tmp" -d dup -b 350M "$TEST_DEV"
+run_check "$TOP/btrfs" check "$TEST_DEV"
+
+rm -rf -- "$tmp"
-- 
2.27.0

Re: [PATCH 0/2] btrfs-progs: mkfs: fix mkfs --rootdir size problem

[David Sterba]

On Tue, Jun 16, 2020 at 02:32:28PM +0800, Qu Wenruo wrote:
> There is a bug report that mkfs.btrfs with -b limit, -d dup, and
> --rootdir with some contents filled, leads to unexpected ENOSPC.
> 
> It turns out that, the new chunk allocation code is broken quite some
> time ago, and by coincident we're using SINGLE data profile by default,
> it doesn't get exposed through existing test cases.
> 
> The patchset will fix it and add corresponding test case for it.
> 
> Qu Wenruo (2):
>   btrfs-progs: fix wrong chunk profile for do_chunk_alloc()
>   btrfs-progs: mkfs-tests: Add test case to verify the --rootdir size
>     limit

Thanks, added to devel.

Re: [PATCH 1/2] btrfs-progs: fix wrong chunk profile for do_chunk_alloc()

[Anand Jain]

looks good.
Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 2/2] btrfs-progs: mkfs-tests: Add test case to verify the --rootdir size limit

[Anand Jain]

looks good.
Reviewed-by: Anand Jain <anand.jain@oracle.com>