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

On 2021/8/17 下午9:24, David Sterba wrote:

On Mon, Aug 09, 2021 at 03:26:13PM -0300, Marcos Paulo de Souza wrote:

quoted

[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 <redacted>
---

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

Makes sense to unify that, it works well for the large sizes. Please
write tests that verify that the chunk sizes are correct after mkfs on
various device sizes. Patch added to devel, thanks.

It in fact makes fsck/025 to fail, bisection points to this patch
surprisingly.

Now "mkfs.btrfs -f" on a 128M file will just fail.

This looks like a big problem to me though...

Thanks,
Qu