[PATCH] btrfs-progs: Drop the type check in init_alloc_chunk_ctl_policy_regular

* [PATCH] btrfs-progs: Drop the type check in init_alloc_chunk_ctl_policy_regular
@ 2021-08-09 18:26 Marcos Paulo de Souza
  2021-08-09 18:44 ` Nikolay Borisov
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Marcos Paulo de Souza @ 2021-08-09 18:26 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba, nborisov, Marcos Paulo de Souza

[PROBLEM]
Our documentation says that a DATA block group can have up to 1G of
size, or the at most 10% of the filesystem size. Currently, by default,
mkfs.btrfs is creating an initial DATA block group of 8M of size,
regardless of the filesystem size. It happens because we check for the
ctl->type in init_alloc_chunk_ctl_policy_regular to be one of the
BTRFS_BLOCK_GROUP_PROFILE_MASK bits, which is not the case for SINGLE
(default) DATA block group:

$ mkfs.btrfs -f /storage/btrfs.disk
btrfs-progs v4.19.1
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               39e3492f-41f2-4bd7-8c25-93032606b9fe
Node size:          16384
Sector size:        4096
Filesystem size:    55.00GiB
Block group profiles:
  Data:             single            8.00MiB
  Metadata:         DUP               1.00GiB
  System:           DUP               8.00MiB
SSD detected:       no
Incompat features:  extref, skinny-metadata
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1    55.00GiB  /storage/btrfs.disk

In this case, for single data profile, it should create a data block
group of 1G, since the filesystem if bigger than 50G.

[FIX]
Remove the check for BTRFS_BLOCK_GROUP_PROFILE_MASK in
init_alloc_chunk_ctl_policy_regular function. The ctl->stripe_size is
later on assigned to ctl.num_bytes, which is used by
btrfs_make_block_group to create the block group.

By removing the check we allow the code to always configure the correct
stripe_size regardless of the PROFILE, looking on the block group TYPE.

With the fix applied, it now created the BG correctly:

$ ./mkfs.btrfs -f /storage/btrfs.disk
btrfs-progs v5.10.1
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               5145e343-5639-462d-82ee-3eb75dc41c31
Node size:          16384
Sector size:        4096
Filesystem size:    55.00GiB
Block group profiles:
  Data:             single            1.00GiB
  Metadata:         DUP               1.00GiB
  System:           DUP               8.00MiB
SSD detected:       no
Zoned device:       no
Incompat features:  extref, skinny-metadata
Runtime features:
Checksum:           crc32c
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1    55.00GiB  /storage/btrfs.disk

Using a disk >50G it creates a 1G single data block group. Another
example:

$ ./mkfs.btrfs -f /storage/btrfs2.disk
btrfs-progs v5.10.1
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               c0910857-e512-4e76-9efa-50a475aafc87
Node size:          16384
Sector size:        4096
Filesystem size:    5.00GiB
Block group profiles:
  Data:             single          512.00MiB
  Metadata:         DUP             256.00MiB
  System:           DUP               8.00MiB
SSD detected:       no
Zoned device:       no
Incompat features:  extref, skinny-metadata
Runtime features:
Checksum:           crc32c
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1     5.00GiB  /storage/btrfs2.disk

The code now created a single data block group of 512M, which is exactly
10% of the size of the filesystem, which is 5G in this case.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---

 This change mimics what the kernel currently does, which is set the stripe_size
 regardless of the profile. Any thoughts on it? Thanks!

 kernel-shared/volumes.c | 40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index aeeb25fe..3677dd7c 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -1105,27 +1105,25 @@ static void init_alloc_chunk_ctl_policy_regular(struct btrfs_fs_info *info,
 	u64 type = ctl->type;
 	u64 percent_max;
 
-	if (type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
-		if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
-			ctl->stripe_size = SZ_8M;
-			ctl->max_chunk_size = ctl->stripe_size * 2;
-			ctl->min_stripe_size = SZ_1M;
-			ctl->max_stripes = BTRFS_MAX_DEVS_SYS_CHUNK;
-		} else if (type & BTRFS_BLOCK_GROUP_DATA) {
-			ctl->stripe_size = SZ_1G;
-			ctl->max_chunk_size = 10 * ctl->stripe_size;
-			ctl->min_stripe_size = SZ_64M;
-			ctl->max_stripes = BTRFS_MAX_DEVS(info);
-		} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
-			/* For larger filesystems, use larger metadata chunks */
-			if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
-				ctl->max_chunk_size = SZ_1G;
-			else
-				ctl->max_chunk_size = SZ_256M;
-			ctl->stripe_size = ctl->max_chunk_size;
-			ctl->min_stripe_size = SZ_32M;
-			ctl->max_stripes = BTRFS_MAX_DEVS(info);
-		}
+	if (type & BTRFS_BLOCK_GROUP_SYSTEM) {
+		ctl->stripe_size = SZ_8M;
+		ctl->max_chunk_size = ctl->stripe_size * 2;
+		ctl->min_stripe_size = SZ_1M;
+		ctl->max_stripes = BTRFS_MAX_DEVS_SYS_CHUNK;
+	} else if (type & BTRFS_BLOCK_GROUP_DATA) {
+		ctl->stripe_size = SZ_1G;
+		ctl->max_chunk_size = 10 * ctl->stripe_size;
+		ctl->min_stripe_size = SZ_64M;
+		ctl->max_stripes = BTRFS_MAX_DEVS(info);
+	} else if (type & BTRFS_BLOCK_GROUP_METADATA) {
+		/* For larger filesystems, use larger metadata chunks */
+		if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G)
+			ctl->max_chunk_size = SZ_1G;
+		else
+			ctl->max_chunk_size = SZ_256M;
+		ctl->stripe_size = ctl->max_chunk_size;
+		ctl->min_stripe_size = SZ_32M;
+		ctl->max_stripes = BTRFS_MAX_DEVS(info);
 	}
 
 	/* We don't want a chunk larger than 10% of the FS */
-- 
2.31.1


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