[PATCH v6 3/3] erofs: add sysfs node to control sync decompression strategy

[Gao Xiang]

Hi Jianan,

On Sat, Nov 13, 2021 at 12:09:35AM +0800, Huang Jianan wrote:
> From: Huang Jianan <huangjianan at oppo.com>
> 
> Although readpage is a synchronous path, there will be no additional
> kworker scheduling overhead in non-atomic contexts. So add a sysfs
> node to allow disable sync decompression.

Sorry for the delay. Please help update the following so I could apply
to -next.

Although readpage is a synchronous path, there will be no additional
kworker scheduling overhead in non-atomic contexts together with
dm-verity.

Let's add a sysfs node to disable sync decompression as an option.

> 
> Signed-off-by: Huang Jianan <huangjianan at oppo.com>
> Reviewed-by: Chao Yu <chao at kernel.org>
> ---
> since v4:
> - Resend in a clean chain.
> 
> since v3:
> - Clean up the sync decompressstrategy into a separate function.
> 
> since v2:
> - Use enum to indicate sync decompression strategy.
> - Add missing CONFIG_EROFS_FS_ZIP ifdef.
> 
> since v1:
> - Leave auto default.
> - Add a disable strategy for sync_decompress.
> 
>  Documentation/ABI/testing/sysfs-fs-erofs |  9 ++++++++
>  fs/erofs/internal.h                      | 10 ++++++--
>  fs/erofs/super.c                         |  2 +-
>  fs/erofs/sysfs.c                         | 15 ++++++++++++
>  fs/erofs/zdata.c                         | 29 ++++++++++++++++++++----
>  5 files changed, 58 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
> index a9512594dc4c..8b620fb49962 100644
> --- a/Documentation/ABI/testing/sysfs-fs-erofs
> +++ b/Documentation/ABI/testing/sysfs-fs-erofs
> @@ -5,3 +5,12 @@ Description:	Shows all enabled kernel features.
>  		Supported features:
>  		zero_padding, compr_cfgs, big_pcluster, chunked_file,
>  		device_table, compr_head2, sb_chksum.
> +
> +What:		/sys/fs/erofs/<disk>/sync_decompress
> +Date:		November 2021
> +Contact:	"Huang Jianan" <huangjianan at oppo.com>
> +Description:	Control strategy of sync decompression
> +		- 0 (defalut, auto): enable for readpage, and enable for

                     ^ default

> +				     readahead on atomic contexts only,
> +		- 1 (force on): enable for readpage and readahead.
> +		- 2 (force off): disable for all situations.
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 43f0332fa489..8e70435629e5 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -56,12 +56,18 @@ struct erofs_device_info {
>  	u32 mapped_blkaddr;
>  };
>  
> +enum {
> +	EROFS_SYNC_DECOMPRESS_AUTO,
> +	EROFS_SYNC_DECOMPRESS_FORCE_ON,
> +	EROFS_SYNC_DECOMPRESS_FORCE_OFF
> +};
> +
>  struct erofs_mount_opts {
>  #ifdef CONFIG_EROFS_FS_ZIP
>  	/* current strategy of how to use managed cache */
>  	unsigned char cache_strategy;
> -	/* strategy of sync decompression (false - auto, true - force on) */
> -	bool readahead_sync_decompress;
> +	/* strategy of sync decompression (0 - auto, 1 - force on, 2 - force off) */
> +	unsigned int sync_decompress;
>  
>  	/* threshold for decompression synchronously */
>  	unsigned int max_sync_decompress_pages;
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index abc1da5d1719..58f381f80205 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
>  #ifdef CONFIG_EROFS_FS_ZIP
>  	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
>  	ctx->opt.max_sync_decompress_pages = 3;
> -	ctx->opt.readahead_sync_decompress = false;
> +	ctx->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
>  #endif
>  #ifdef CONFIG_EROFS_FS_XATTR
>  	set_opt(&ctx->opt, XATTR_USER);
> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
> index 80ca5ed59bb4..8f0a71e880ea 100644
> --- a/fs/erofs/sysfs.c
> +++ b/fs/erofs/sysfs.c
> @@ -16,6 +16,7 @@ enum {
>  
>  enum {
>  	struct_erofs_sb_info,
> +	struct_erofs_mount_opts,
>  };
>  
>  struct erofs_attr {
> @@ -55,7 +56,14 @@ static struct erofs_attr erofs_attr_##_name = {			\
>  
>  #define ATTR_LIST(name) (&erofs_attr_##name.attr)
>  
> +#ifdef CONFIG_EROFS_FS_ZIP
> +EROFS_ATTR_RW_UI(sync_decompress, erofs_mount_opts);
> +#endif
> +
>  static struct attribute *erofs_attrs[] = {
> +#ifdef CONFIG_EROFS_FS_ZIP
> +	ATTR_LIST(sync_decompress),
> +#endif
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(erofs);
> @@ -86,6 +94,8 @@ static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
>  {
>  	if (struct_type == struct_erofs_sb_info)
>  		return (unsigned char *)sbi + offset;
> +	if (struct_type == struct_erofs_mount_opts)
> +		return (unsigned char *)&sbi->opt + offset;
>  	return NULL;
>  }
>  
> @@ -132,6 +142,11 @@ static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
>  			return ret;
>  		if (t > UINT_MAX)
>  			return -EINVAL;
> +#ifdef CONFIG_EROFS_FS_ZIP
> +		if (!strcmp(a->attr.name, "sync_decompress") &&
> +		    (t > EROFS_SYNC_DECOMPRESS_FORCE_OFF))
> +			return -EINVAL;
> +#endif
>  		*(unsigned int *)ptr = t;
>  		return len;
>  	case attr_pointer_bool:
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index bcb1b91b234f..233c8a047c53 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -772,6 +772,26 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
>  	goto out;
>  }
>  
> +static void set_sync_decompress_policy(struct erofs_sb_info *sbi)
> +{
> +	/* enable sync decompression in readahead for atomic contexts */
> +	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO)
> +		sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_FORCE_ON;
> +}
> +
> +static bool get_sync_decompress_policy(struct erofs_sb_info *sbi,
> +				       unsigned int readahead_pages)

static bool z_erofs_get_sync_decompress_policy ...

> +{
> +	/* auto: enable for readpage, disable for readahead */
> +	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO)
> +		return readahead_pages == 0;

	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO &&
	    !readahead_pages)
		return true;

> +
> +	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_FORCE_ON)
> +		return readahead_pages <= sbi->opt.max_sync_decompress_pages;

Same here.

> +
> +	return false;
> +}
> +
>  static void z_erofs_decompressqueue_work(struct work_struct *work);
>  static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
>  				       bool sync, int bios)
> @@ -794,7 +814,7 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
>  	/* Use workqueue and sync decompression for atomic contexts only */
>  	if (in_atomic() || irqs_disabled()) {
>  		queue_work(z_erofs_workqueue, &io->u.work);
> -		sbi->opt.readahead_sync_decompress = true;
> +		set_sync_decompress_policy(sbi);

Please help inline this since this is the only one caller.

Thanks,
Gao Xiang