Re: [PATCH v4 1/4] btrfs: store chunk size in space-info struct.
From: Nikolay Borisov <hidden>
Date: 2021-11-05 08:52:14
On 29.10.21 г. 21:39, Stefan Roesch wrote:
The chunk size is stored in the btrfs_space_info structure. It is initialized at the start and is then used. Two api's are added to get the current value and one to update the value.
There is just a single API added to update the size, there is no api to get the value, one has to directly read default_chunk_size. Additionally if it's going to be changed then does the "default_" prefix really mean anything?
quoted hunk ↗ jump to hunk
These api's will be used to be able to expose the chunk_size as a sysfs setting. Signed-off-by: Stefan Roesch <redacted> --- fs/btrfs/space-info.c | 51 +++++++++++++++++++++++++++++++++++++++++++ fs/btrfs/space-info.h | 3 +++ fs/btrfs/volumes.c | 28 ++++++++---------------- 3 files changed, 63 insertions(+), 19 deletions(-)diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c index 48d77f360a24..7370c152ce8a 100644 --- a/fs/btrfs/space-info.c +++ b/fs/btrfs/space-info.c@@ -181,6 +181,56 @@ void btrfs_clear_space_info_full(struct btrfs_fs_info *info) found->full = 0; } +/* + * Compute chunk size depending on block type for regular volumes. + */ +static u64 compute_chunk_size_regular(struct btrfs_fs_info *info, u64 flags) +{ + ASSERT(flags & BTRFS_BLOCK_GROUP_TYPE_MASK); + + if (flags & BTRFS_BLOCK_GROUP_DATA) + return SZ_1G; + else if (flags & BTRFS_BLOCK_GROUP_SYSTEM) + return SZ_32M; + + /* Handle BTRFS_BLOCK_GROUP_METADATA */ + if (info->fs_devices->total_rw_bytes > 50ULL * SZ_1G) + return SZ_1G; + + return SZ_256M; +} + +/* + * Compute chunk size for zoned volumes. + */ +static u64 compute_chunk_size_zoned(struct btrfs_fs_info *info) +{ + return info->zone_size; +}
nit: This is trivial and so can simply be open-coded in compute_chunk_size, yes it's static and will likely be compiled out but it adds a bit of cognitive load when reading the code. In any case I'd leave this to David to decide whether to leave the function or not.
+ +/* + * Compute chunk size depending on volume type. + */
<snip>
quoted hunk ↗ jump to hunk
static int create_space_info(struct btrfs_fs_info *info, u64 flags) {@@ -202,6 +252,7 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags) INIT_LIST_HEAD(&space_info->tickets); INIT_LIST_HEAD(&space_info->priority_tickets); space_info->clamp = 1; + space_info->default_chunk_size = compute_chunk_size(info, flags); ret = btrfs_sysfs_add_space_info_type(info, space_info); if (ret)
<snip>
quoted hunk ↗ jump to hunk
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 546bf1146b2d..563e5b30060d 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c@@ -5063,26 +5063,16 @@ static void init_alloc_chunk_ctl_policy_regular( struct btrfs_fs_devices *fs_devices, struct alloc_chunk_ctl *ctl) { - u64 type = ctl->type; + struct btrfs_space_info *space_info; - if (type & BTRFS_BLOCK_GROUP_DATA) { - ctl->max_stripe_size = SZ_1G; - ctl->max_chunk_size = BTRFS_MAX_DATA_CHUNK_SIZE; - } else if (type & BTRFS_BLOCK_GROUP_METADATA) { - /* For larger filesystems, use larger metadata chunks */ - if (fs_devices->total_rw_bytes > 50ULL * SZ_1G) - ctl->max_stripe_size = SZ_1G; - else - ctl->max_stripe_size = SZ_256M; - ctl->max_chunk_size = ctl->max_stripe_size; - } else if (type & BTRFS_BLOCK_GROUP_SYSTEM) { - ctl->max_stripe_size = SZ_32M; - ctl->max_chunk_size = 2 * ctl->max_stripe_size; - ctl->devs_max = min_t(int, ctl->devs_max, - BTRFS_MAX_DEVS_SYS_CHUNK); - } else { - BUG(); - } + space_info = btrfs_find_space_info(fs_devices->fs_info, ctl->type); + ASSERT(space_info); + + ctl->max_chunk_size = space_info->default_chunk_size; + ctl->max_stripe_size = space_info->default_chunk_size;
Those are racy accesses, no ? Chunk allocation happens under chunk_mutex, not the space_info lock ? Perhaps it could be turned into an atomic?
+ + if (ctl->type & BTRFS_BLOCK_GROUP_SYSTEM) + ctl->devs_max = min_t(int, ctl->devs_max, BTRFS_MAX_DEVS_SYS_CHUNK); /* We don't want a chunk larger than 10% of writable space */ ctl->max_chunk_size = min(div_factor(fs_devices->total_rw_bytes, 1),