Re: [PATCH] btrfs: auto enable discard=async when possible

[Wang Yugui <wangyugui@e16-tech.com>]

Hi,

> There's a request to automatically enable async discard for capable
> devices. We can do that, the async mode is designed to wait for larger
> freed extents and is not intrusive, with limits to iops, kbps or latency.
> 
> The status and tunables will be exported in /sys/fs/btrfs/FSID/discard .
> 
> The automatic selection is done if there's at least one discard capable
> device in the filesystem (not capable devices are skipped). Mounting
> with any other discard option will honor that option, notably mounting
> with nodiscard will keep it disabled.
> 
> Link: https://lore.kernel.org/linux-btrfs/CAEg-Je_b1YtdsCR0zS5XZ_SbvJgN70ezwvRwLiCZgDGLbeMB=w@mail.gmail.com/
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h   |  1 +
>  fs/btrfs/disk-io.c | 14 ++++++++++++++
>  fs/btrfs/super.c   |  2 ++
>  fs/btrfs/volumes.c |  3 +++
>  fs/btrfs/volumes.h |  2 ++
>  5 files changed, 22 insertions(+)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 4db85b9dc7ed..0a338311f8e2 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1503,6 +1503,7 @@ enum {
>  	BTRFS_MOUNT_DISCARD_ASYNC		= (1UL << 28),
>  	BTRFS_MOUNT_IGNOREBADROOTS		= (1UL << 29),
>  	BTRFS_MOUNT_IGNOREDATACSUMS		= (1UL << 30),
> +	BTRFS_MOUNT_NODISCARD			= (1UL << 31),
>  };
>  
>  #define BTRFS_DEFAULT_COMMIT_INTERVAL	(30)
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 3fac429cf8a4..8f8e33219d4d 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3767,6 +3767,20 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  		btrfs_set_and_info(fs_info, SSD, "enabling ssd optimizations");
>  	}
>  
> +	/*
> +	 * For devices supporting discard turn on discard=async automatically,
> +	 * unless it's already set or disabled. This could be turned off by
> +	 * nodiscard for the same mount.
> +	 */
> +	if (!(btrfs_test_opt(fs_info, DISCARD_SYNC) ||
> +	      btrfs_test_opt(fs_info, DISCARD_ASYNC) ||
> +	      btrfs_test_opt(fs_info, NODISCARD)) &&
> +	    fs_info->fs_devices->discardable) {
> +		btrfs_set_and_info(fs_info, DISCARD_ASYNC,
> +				"auto enabling discard=async");
> +	      btrfs_clear_opt(fs_info->mount_opt, NODISCARD);

We do not need 'btrfs_clear_opt(fs_info->mount_opt, NODISCARD);' here?

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/11/06

> +	}
> +
>  	/*
>  	 * Mount does not set all options immediately, we can do it now and do
>  	 * not have to wait for transaction commit
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 4c7089b1681b..1032aaa2c2f4 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -915,12 +915,14 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
>  				ret = -EINVAL;
>  				goto out;
>  			}
> +			btrfs_clear_opt(info->mount_opt, NODISCARD);
>  			break;
>  		case Opt_nodiscard:
>  			btrfs_clear_and_info(info, DISCARD_SYNC,
>  					     "turning off discard");
>  			btrfs_clear_and_info(info, DISCARD_ASYNC,
>  					     "turning off async discard");
> +			btrfs_set_opt(info->mount_opt, NODISCARD);
>  			break;
>  		case Opt_space_cache:
>  		case Opt_space_cache_version:
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 272901514b0c..22bfc7806ccb 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -639,6 +639,9 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
>  	if (!bdev_nonrot(bdev))
>  		fs_devices->rotating = true;
>  
> +	if (bdev_max_discard_sectors(bdev))
> +		fs_devices->discardable = true;
> +
>  	device->bdev = bdev;
>  	clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
>  	device->mode = flags;
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 5639961b3626..4c716603449d 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -329,6 +329,8 @@ struct btrfs_fs_devices {
>  	 * nonrot flag set
>  	 */
>  	bool rotating;
> +	/* Devices support TRIM/discard commands */
> +	bool discardable;
>  
>  	struct btrfs_fs_info *fs_info;
>  	/* sysfs kobjects */
> -- 
> 2.36.1