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

[David Sterba <dsterba@suse.cz>]

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);
> +}