Re: [PATCH] btrfs: Add nossd_spread mount option

[Hans van Kranenburg <hans.van.kranenburg@mendix.com>]

On 02/22/2018 12:31 AM, Howard McLauchlan wrote:
> Btrfs has two mount options for SSD optimizations: ssd and ssd_spread.
> Presently there is an option to disable all SSD optimizations, but there
> isn't an option to disable just ssd_spread.
> 
> This patch adds a mount option nossd_spread that disables ssd_spread
> only.
> 
> Signed-off-by: Howard McLauchlan <hmclauchlan@fb.com>
> ---
>  fs/btrfs/super.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 6e71a2a78363..4c0fcf5b3e7e 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -310,10 +310,10 @@ static void btrfs_put_super(struct super_block *sb)
>  enum {
>  	Opt_degraded, Opt_subvol, Opt_subvolid, Opt_device, Opt_nodatasum,
>  	Opt_nodatacow, Opt_max_inline, Opt_alloc_start, Opt_nobarrier, Opt_ssd,
> -	Opt_nossd, Opt_ssd_spread, Opt_thread_pool, Opt_noacl, Opt_compress,
> -	Opt_compress_type, Opt_compress_force, Opt_compress_force_type,
> -	Opt_notreelog, Opt_ratio, Opt_flushoncommit, Opt_discard,
> -	Opt_space_cache, Opt_space_cache_version, Opt_clear_cache,
> +	Opt_nossd, Opt_ssd_spread, Opt_nossd_spread, Opt_thread_pool, Opt_noacl,
> +	Opt_compress, Opt_compress_type, Opt_compress_force,
> +	Opt_compress_force_type, Opt_notreelog, Opt_ratio, Opt_flushoncommit,
> +	Opt_discard, Opt_space_cache, Opt_space_cache_version, Opt_clear_cache,
>  	Opt_user_subvol_rm_allowed, Opt_enospc_debug, Opt_subvolrootid,
>  	Opt_defrag, Opt_inode_cache, Opt_no_space_cache, Opt_recovery,
>  	Opt_skip_balance, Opt_check_integrity,
> @@ -353,6 +353,7 @@ static const match_table_t tokens = {
>  	{Opt_ssd, "ssd"},
>  	{Opt_ssd_spread, "ssd_spread"},
>  	{Opt_nossd, "nossd"},
> +	{Opt_nossd_spread, "nossd_spread"},
>  	{Opt_acl, "acl"},
>  	{Opt_noacl, "noacl"},
>  	{Opt_notreelog, "notreelog"},

.oO(Why doesn't the enum just have one option per line, so the changelog
is less invasive?)

> @@ -582,6 +583,10 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
>  			btrfs_clear_and_info(info, SSD_SPREAD,
>  					     "not using spread ssd allocation scheme");
>  			break;
> +		case Opt_nossd_spread:
> +			btrfs_clear_and_info(info, SSD_SPREAD,
> +					     "not using spread ssd allocation scheme");
> +			break;
>  		case Opt_barrier:
>  			btrfs_clear_and_info(info, NOBARRIER,
>  					     "turning on barriers");
> 

Related:
* https://www.spinics.net/lists/linux-btrfs/msg64247.html
* https://www.spinics.net/lists/linux-btrfs/msg64277.html
* https://www.spinics.net/lists/linux-btrfs/msg64499.html

Apparently that discussion never resulted in actual changes, so thanks
for continuing it now.

The mount options are a bit weird, because ssd_spread also includes ssd,
but doesn't show it.

I personally don't like all of them at all, and I should really finish
and send my proposal to get them replaced by options that can choose
extent allocator for data and metadata individually (instead of some
setting that changes them both at the same time) because there are
proper real life situations that e.g. benefit from nossd style 'tetris'
data allocator with ssd_spread style 'contiguous' metadata extent allocator.

-- 
Hans van Kranenburg