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

[David Sterba <dsterba@suse.cz>]

On Mon, Aug 09, 2021 at 03:26:13PM -0300, Marcos Paulo de Souza wrote:
> [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!

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.