Re: [PATCH v2 11/18] btrfs: add reconfigure callback for fs_context

David Sterba
2023-11-14T18:30:30Z

On Wed, Nov 08, 2023 at 02:08:46PM -0500, Josef Bacik wrote:
> This is what is used to remount the file system with the new mount API.
> Because the mount options are parsed separately and one at a time I've
> added a helper to emit the mount options after the fact once the mount
> is configured, this matches the dmesg output for what happens with the
> old mount API.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> fs/btrfs/super.c | 243 +++++++++++++++++++++++++++++++++++++++++++----
> fs/btrfs/zoned.c | 16 ++--
> fs/btrfs/zoned.h | 6 +-
> 3 files changed, 236 insertions(+), 29 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index facea4632a8d..b5067cf637a2 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -734,10 +734,11 @@ static int btrfs_parse_param(struct fs_context *fc,
> return 0;
> }
>
> -static bool check_ro_option(struct btrfs_fs_info *fs_info, unsigned long opt,
> +static bool check_ro_option(struct btrfs_fs_info *fs_info,
> + unsigned long mount_opt, unsigned long opt,
> const char *opt_name)
> {
> - if (fs_info->mount_opt & opt) {
> + if (mount_opt & opt) {
> btrfs_err(fs_info, "%s must be used with ro mount option",
> opt_name);
> return true;
> @@ -745,33 +746,34 @@ static bool check_ro_option(struct btrfs_fs_info *fs_info, unsigned long opt,
> return false;
> }
>
> -static bool check_options(struct btrfs_fs_info *info, unsigned long flags)
> +static bool check_options(struct btrfs_fs_info *info, unsigned long *mount_opt,
> + unsigned long flags)
> {
> if (!(flags & SB_RDONLY) &&
> - (check_ro_option(info, BTRFS_MOUNT_NOLOGREPLAY, "nologreplay") ||
> - check_ro_option(info, BTRFS_MOUNT_IGNOREBADROOTS, "ignorebadroots") ||
> - check_ro_option(info, BTRFS_MOUNT_IGNOREDATACSUMS, "ignoredatacsums")))
> + (check_ro_option(info, *mount_opt, BTRFS_MOUNT_NOLOGREPLAY, "nologreplay") ||
> + check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREBADROOTS, "ignorebadroots") ||
> + check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREDATACSUMS, "ignoredatacsums")))
> return false;
>
> if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE) &&
> - !btrfs_test_opt(info, FREE_SPACE_TREE) &&
> - !btrfs_test_opt(info, CLEAR_CACHE)) {
> + !btrfs_raw_test_opt(*mount_opt, FREE_SPACE_TREE) &&
> + !btrfs_raw_test_opt(*mount_opt, CLEAR_CACHE)) {
> btrfs_err(info, "cannot disable free space tree");
> return false;
> }
> if (btrfs_fs_compat_ro(info, BLOCK_GROUP_TREE) &&
> - !btrfs_test_opt(info, FREE_SPACE_TREE)) {
> + !btrfs_raw_test_opt(*mount_opt, FREE_SPACE_TREE)) {
> btrfs_err(info, "cannot disable free space tree with block-group-tree feature");
> return false;
> }
>
> - if (btrfs_check_mountopts_zoned(info))
> + if (btrfs_check_mountopts_zoned(info, mount_opt))
> return false;
>
> if (!test_bit(BTRFS_FS_STATE_REMOUNTING, &info->fs_state)) {
> - if (btrfs_test_opt(info, SPACE_CACHE))
> + if (btrfs_raw_test_opt(*mount_opt, SPACE_CACHE))
> btrfs_info(info, "disk space caching is enabled");
> - if (btrfs_test_opt(info, FREE_SPACE_TREE))
> + if (btrfs_raw_test_opt(*mount_opt, FREE_SPACE_TREE))
> btrfs_info(info, "using free space tree");
> }
>
> @@ -1337,7 +1339,7 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> }
> }
> out:
> - if (!ret && !check_options(info, new_flags))
> + if (!ret && !check_options(info, &info->mount_opt, new_flags))
> ret = -EINVAL;
> return ret;
> }
> @@ -2377,6 +2379,203 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
> return ret;
> }
>
> +static void btrfs_ctx_to_info(struct btrfs_fs_info *fs_info,
> + struct btrfs_fs_context *ctx)
> +{
> + fs_info->max_inline = ctx->max_inline;
> + fs_info->commit_interval = ctx->commit_interval;
> + fs_info->metadata_ratio = ctx->metadata_ratio;
> + fs_info->thread_pool_size = ctx->thread_pool_size;
> + fs_info->mount_opt = ctx->mount_opt;
> + fs_info->compress_type = ctx->compress_type;
> + fs_info->compress_level = ctx->compress_level;
> +}
> +
> +static void btrfs_info_to_ctx(struct btrfs_fs_info *fs_info,
> + struct btrfs_fs_context *ctx)
> +{
> + ctx->max_inline = fs_info->max_inline;
> + ctx->commit_interval = fs_info->commit_interval;
> + ctx->metadata_ratio = fs_info->metadata_ratio;
> + ctx->thread_pool_size = fs_info->thread_pool_size;
> + ctx->mount_opt = fs_info->mount_opt;
> + ctx->compress_type = fs_info->compress_type;
> + ctx->compress_level = fs_info->compress_level;
> +}
> +
> +#define btrfs_info_if_set(fs_info, old_ctx, opt, fmt, args...) \
> +do { \
> + if ((!old_ctx || !btrfs_raw_test_opt(old_ctx->mount_opt, opt)) && \
> + btrfs_raw_test_opt(fs_info->mount_opt, opt)) \
> + btrfs_info(fs_info, fmt, ##args); \
> +} while (0)
> +
> +#define btrfs_info_if_unset(fs_info, old_ctx, opt, fmt, args...) \
> +do { \
> + if ((old_ctx && btrfs_raw_test_opt(old_ctx->mount_opt, opt)) && \
> + !btrfs_raw_test_opt(fs_info->mount_opt, opt)) \
> + btrfs_info(fs_info, fmt, ##args); \
> +} while (0)
> +
> +static void btrfs_emit_options(struct btrfs_fs_info *fs_info,
> + struct btrfs_fs_context *old_ctx)
> +{
> + btrfs_info_if_set(fs_info, old_ctx, NODATASUM, "setting nodatasum");
> + btrfs_info_if_set(fs_info, old_ctx, DEGRADED,
> + "allowing degraded mounts");
> + btrfs_info_if_set(fs_info, old_ctx, NODATASUM, "setting nodatasum");
> + btrfs_info_if_set(fs_info, old_ctx, SSD, "enabling ssd optimizations");
> + btrfs_info_if_set(fs_info, old_ctx, SSD_SPREAD,
> + "using spread ssd allocation scheme");
> + btrfs_info_if_set(fs_info, old_ctx, NOBARRIER, "turning off barriers");
> + btrfs_info_if_set(fs_info, old_ctx, NOTREELOG, "disabling tree log");
> + btrfs_info_if_set(fs_info, old_ctx, NOLOGREPLAY,
> + "disabling log replay at mount time");
> + btrfs_info_if_set(fs_info, old_ctx, FLUSHONCOMMIT,
> + "turning on flush-on-commit");
> + btrfs_info_if_set(fs_info, old_ctx, DISCARD_SYNC,
> + "turning on sync discard");
> + btrfs_info_if_set(fs_info, old_ctx, DISCARD_ASYNC,
> + "turning on async discard");
> + btrfs_info_if_set(fs_info, old_ctx, FREE_SPACE_TREE,
> + "enabling free space tree");
> + btrfs_info_if_set(fs_info, old_ctx, SPACE_CACHE,
> + "enabling disk space caching");
> + btrfs_info_if_set(fs_info, old_ctx, CLEAR_CACHE,
> + "force clearing of disk cache");
> + btrfs_info_if_set(fs_info, old_ctx, AUTO_DEFRAG,
> + "enabling auto defrag");
> + btrfs_info_if_set(fs_info, old_ctx, FRAGMENT_DATA,
> + "fragmenting data");
> + btrfs_info_if_set(fs_info, old_ctx, FRAGMENT_METADATA,
> + "fragmenting metadata");
> + btrfs_info_if_set(fs_info, old_ctx, REF_VERIFY,
> + "doing ref verification");
> + btrfs_info_if_set(fs_info, old_ctx, USEBACKUPROOT,
> + "trying to use backup root at mount time");
> + btrfs_info_if_set(fs_info, old_ctx, IGNOREBADROOTS,
> + "ignoring bad roots");
> + btrfs_info_if_set(fs_info, old_ctx, IGNOREDATACSUMS,
> + "ignoring data csums");

I think we can format this on >80 char lines, it would look better IMHO.
If fs_info and old_ctx are named shorter then more of the string fits.

> + btrfs_info_if_unset(fs_info, old_ctx, NODATACOW, "setting datacow");
> + btrfs_info_if_unset(fs_info, old_ctx, SSD, "not using ssd optimizations");
> + btrfs_info_if_unset(fs_info, old_ctx, SSD_SPREAD,
> + "not using spread ssd allocation scheme");
> + btrfs_info_if_unset(fs_info, old_ctx, NOBARRIER,
> + "turning off barriers");
> + btrfs_info_if_unset(fs_info, old_ctx, NOTREELOG, "enabling tree log");
> + btrfs_info_if_unset(fs_info, old_ctx, SPACE_CACHE,
> + "disabling disk space caching");
> + btrfs_info_if_unset(fs_info, old_ctx, FREE_SPACE_TREE,
> + "disabling free space tree");
> + btrfs_info_if_unset(fs_info, old_ctx, AUTO_DEFRAG,
> + "disabling auto defrag");
> + btrfs_info_if_unset(fs_info, old_ctx, COMPRESS,
> + "use no compression");

Same

> +
> + /* Did the compression settings change? */
> + if (btrfs_test_opt(fs_info, COMPRESS) &&
> + (!old_ctx ||
> + old_ctx->compress_type != fs_info->compress_type ||
> + old_ctx->compress_level != fs_info->compress_level ||
> + (!btrfs_raw_test_opt(old_ctx->mount_opt, FORCE_COMPRESS) &&
> + btrfs_raw_test_opt(fs_info->mount_opt, FORCE_COMPRESS)))) {
> + char *compress_type = "none";
> +
> + switch (fs_info->compress_type) {
> + case BTRFS_COMPRESS_ZLIB:
> + compress_type = "zlib";
> + break;
> + case BTRFS_COMPRESS_LZO:
> + compress_type = "lzo";
> + break;
> + case BTRFS_COMPRESS_ZSTD:
> + compress_type = "zstd";
> + break;
> + }

We have btrfs_compress_type2str()

> +
> + btrfs_info(fs_info, "%s %s compression, level %d",
> + btrfs_test_opt(fs_info, FORCE_COMPRESS) ? "force" : "use",
> + compress_type, fs_info->compress_level);
> + }
> +
> + if (fs_info->max_inline != BTRFS_DEFAULT_MAX_INLINE)
> + btrfs_info(fs_info, "max_inline at %llu",
> + fs_info->max_inline);
> +}