Re: [PATCH v2 04/18] btrfs: move space cache settings into open_ctree

Anand Jain
2023-11-15T00:06:18Z

> @@ -3287,6 +3287,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> fs_info->csums_per_leaf = BTRFS_MAX_ITEM_SIZE(fs_info) / fs_info->csum_size;
> fs_info->stripesize = stripesize;
>
> + /*
> + * Handle the space caching options appropriately now that we have the
> + * super loaded and validated.
> + */
> + btrfs_set_free_space_cache_settings(fs_info);
> +
> ret = btrfs_parse_options(fs_info, options, sb->s_flags);
> if (ret)
> goto fail_alloc;
> @@ -3298,17 +3304,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> if (sectorsize < PAGE_SIZE) {
> struct btrfs_subpage_info *subpage_info;
>
> - /*
> - * V1 space cache has some hardcoded PAGE_SIZE usage, and is
> - * going to be deprecated.
> - *
> - * Force to use v2 cache for subpage case.
> - */
> - btrfs_clear_opt(fs_info->mount_opt, SPACE_CACHE);
> - btrfs_set_and_info(fs_info, FREE_SPACE_TREE,
> - "forcing free space tree for sector size %u with page size %lu",
> - sectorsize, PAGE_SIZE);
> -
> btrfs_warn(fs_info,
> "read-write for sector size %u with page size %lu is experimental",
> sectorsize, PAGE_SIZE);
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 639601d346d0..aef7e67538a3 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -266,6 +266,31 @@ static bool check_options(struct btrfs_fs_info *info, unsigned long flags)
> return true;
> }
>
> +void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
> +{
> + if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE))
> + btrfs_set_opt(fs_info->mount_opt, FREE_SPACE_TREE);
> + else if (btrfs_free_space_cache_v1_active(fs_info)) {
> + if (btrfs_is_zoned(fs_info)) {
> + btrfs_info(fs_info,
> + "zoned: clearing existing space cache");
> + btrfs_set_super_cache_generation(fs_info->super_copy, 0);
> + } else {
> + btrfs_set_opt(fs_info->mount_opt, SPACE_CACHE);
> + }
> + }
> +
> + if (fs_info->sectorsize < PAGE_SIZE) {
> + btrfs_clear_opt(fs_info->mount_opt, SPACE_CACHE);
> + if (!btrfs_test_opt(fs_info, FREE_SPACE_TREE)) {
> + btrfs_info(fs_info,
> + "forcing free space tree for sector size %u with page size %lu",
> + fs_info->sectorsize, PAGE_SIZE);
> + btrfs_set_opt(fs_info->mount_opt, FREE_SPACE_TREE);
> + }
> + }
> +}
> +
> static int parse_rescue_options(struct btrfs_fs_info *info, const char *options)
> {
> char *opts;
> @@ -345,18 +370,6 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> bool saved_compress_force;
> int no_compress = 0;
>
> - if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE))
> - btrfs_set_opt(info->mount_opt, FREE_SPACE_TREE);
> - else if (btrfs_free_space_cache_v1_active(info)) {
> - if (btrfs_is_zoned(info)) {
> - btrfs_info(info,
> - "zoned: clearing existing space cache");
> - btrfs_set_super_cache_generation(info->super_copy, 0);
> - } else {
> - btrfs_set_opt(info->mount_opt, SPACE_CACHE);
> - }
> - }
> -
> /*
> * Even the options are empty, we still need to do extra check
> * against new flags

btrfs_remount() calls btrfs_parse_options(), which previously handled
space cache/tree flags set/reset. However, with the move of this
functionality to a separate function, btrfs_set_free_space_cache_settings(),
the btrfs_remount() thread no longer has the space cache/tree flags
set/reset. The changelog provides no explanation for this change.
Could this be a bug?

Thanks, Anand