Re: [PATCH 2/2] zram: support deflate-specific params

[Zaslonko Mikhail <zaslonko@linux.ibm.com>]

Looks good to me.

Just a minor comment. If we intend to use raw deflate only, like we do now (no zlib header or
trailer for the compressed data), we should probably change deflate.winbits to unsigned and
pass '-deflate.winbits' to zlib_deflateInit2().


Also, here is another patch suggestion from my side on top of this one. 
Let me know what you think.

---8<---

zram: Utilize s390 hardware deflate acceleration for zram

Utilize s390 hardware deflate acceleration for zram deflate compression
by default when the facility is available.

Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>

diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index b75016e0e654..5bfc57522e3a 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -22,10 +22,23 @@ static void deflate_release_params(struct zcomp_params *params)

 static int deflate_setup_params(struct zcomp_params *params)
 {
-       if (params->level == ZCOMP_PARAM_NOT_SET)
-               params->level = Z_DEFAULT_COMPRESSION;
-       if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET)
-               params->deflate.winbits = DEFLATE_DEF_WINBITS;
+       /*
+        * In case of s390 zlib hardware support available,
+        * use maximum window size and level one as default compression
+        * parameters in order to utilize hardware deflate acceleration.
+        */
+       if (params->level == ZCOMP_PARAM_NOT_SET) {
+               if (zlib_deflate_dfltcc_enabled())
+                       params->level = Z_BEST_SPEED;
+               else
+                       params->level = Z_DEFAULT_COMPRESSION;
+       }
+       if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET) {
+               if (zlib_deflate_dfltcc_enabled())
+                       params->deflate.winbits = -MAX_WBITS;
+               else
+                       params->deflate.winbits = DEFLATE_DEF_WINBITS;
+       }

        return 0;
 }



On 14.05.2025 04:47, Sergey Senozhatsky wrote:
> Introduce support of algorithm specific parameters in
> algorithm_params device attribute.  The expected format
> is algorithm.param=value.
> 
> For starters, add support for deflate.winbits parameter.
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  drivers/block/zram/backend_deflate.c | 10 ++++++----
>  drivers/block/zram/zcomp.h           |  7 +++++++
>  drivers/block/zram/zram_drv.c        | 17 +++++++++++++++--
>  3 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
> index 4c00b5b6739f..b75016e0e654 100644
> --- a/drivers/block/zram/backend_deflate.c
> +++ b/drivers/block/zram/backend_deflate.c
> @@ -8,7 +8,7 @@
>  #include "backend_deflate.h"
>  
>  /* Use the same value as crypto API */
> -#define DEFLATE_DEF_WINBITS		11
> +#define DEFLATE_DEF_WINBITS		(-11)
>  #define DEFLATE_DEF_MEMLEVEL		MAX_MEM_LEVEL
>  
>  struct deflate_ctx {
> @@ -24,6 +24,8 @@ static int deflate_setup_params(struct zcomp_params *params)
>  {
>  	if (params->level == ZCOMP_PARAM_NOT_SET)
>  		params->level = Z_DEFAULT_COMPRESSION;
> +	if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET)
> +		params->deflate.winbits = DEFLATE_DEF_WINBITS;
>  
>  	return 0;
>  }
> @@ -57,13 +59,13 @@ static int deflate_create(struct zcomp_params *params, struct zcomp_ctx *ctx)
>  		return -ENOMEM;
>  
>  	ctx->context = zctx;
> -	sz = zlib_deflate_workspacesize(-DEFLATE_DEF_WINBITS, MAX_MEM_LEVEL);
> +	sz = zlib_deflate_workspacesize(params->deflate.winbits, MAX_MEM_LEVEL);
>  	zctx->cctx.workspace = vzalloc(sz);
>  	if (!zctx->cctx.workspace)
>  		goto error;
>  
>  	ret = zlib_deflateInit2(&zctx->cctx, params->level, Z_DEFLATED,
> -				-DEFLATE_DEF_WINBITS, DEFLATE_DEF_MEMLEVEL,
> +				params->deflate.winbits, DEFLATE_DEF_MEMLEVEL,
>  				Z_DEFAULT_STRATEGY);
>  	if (ret != Z_OK)
>  		goto error;
> @@ -73,7 +75,7 @@ static int deflate_create(struct zcomp_params *params, struct zcomp_ctx *ctx)
>  	if (!zctx->dctx.workspace)
>  		goto error;
>  
> -	ret = zlib_inflateInit2(&zctx->dctx, -DEFLATE_DEF_WINBITS);
> +	ret = zlib_inflateInit2(&zctx->dctx, params->deflate.winbits);
>  	if (ret != Z_OK)
>  		goto error;
>  
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index cfacdfe9044c..4acffe671a5e 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -7,6 +7,10 @@
>  
>  #define ZCOMP_PARAM_NOT_SET	INT_MIN
>  
> +struct deflate_params {
> +	s32 winbits;
> +};
> +
>  /*
>   * Immutable driver (backend) parameters. The driver may attach private
>   * data to it (e.g. driver representation of the dictionary, etc.).
> @@ -17,6 +21,9 @@ struct zcomp_params {
>  	void *dict;
>  	size_t dict_sz;
>  	s32 level;
> +	union {
> +		struct deflate_params deflate;
> +	};
>  
>  	void *drv_data;
>  };
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index a11b7a6e35f4..54c57103715f 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1277,12 +1277,14 @@ static void comp_params_reset(struct zram *zram, u32 prio)
>  
>  	vfree(params->dict);
>  	params->level = ZCOMP_PARAM_NOT_SET;
> +	params->deflate.winbits = ZCOMP_PARAM_NOT_SET;
>  	params->dict_sz = 0;
>  	params->dict = NULL;
>  }
>  
>  static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> -			     const char *dict_path)
> +			     const char *dict_path,
> +			     struct deflate_params *deflate_params)
>  {
>  	ssize_t sz = 0;
>  
> @@ -1300,6 +1302,7 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
>  
>  	zram->params[prio].dict_sz = sz;
>  	zram->params[prio].level = level;
> +	zram->params[prio].deflate.winbits = deflate_params->winbits;
>  	return 0;
>  }
>  
> @@ -1310,9 +1313,12 @@ static ssize_t algorithm_params_store(struct device *dev,
>  {
>  	s32 prio = ZRAM_PRIMARY_COMP, level = ZCOMP_PARAM_NOT_SET;
>  	char *args, *param, *val, *algo = NULL, *dict_path = NULL;
> +	struct deflate_params deflate_params;
>  	struct zram *zram = dev_to_zram(dev);
>  	int ret;
>  
> +	deflate_params.winbits = ZCOMP_PARAM_NOT_SET;
> +
>  	args = skip_spaces(buf);
>  	while (*args) {
>  		args = next_arg(args, &param, &val);
> @@ -1343,6 +1349,13 @@ static ssize_t algorithm_params_store(struct device *dev,
>  			dict_path = val;
>  			continue;
>  		}
> +
> +		if (!strcmp(param, "deflate.winbits")) {
> +			ret = kstrtoint(val, 10, &deflate_params.winbits);
> +			if (ret)
> +				return ret;
> +			continue;
> +		}
>  	}
>  
>  	/* Lookup priority by algorithm name */
> @@ -1364,7 +1377,7 @@ static ssize_t algorithm_params_store(struct device *dev,
>  	if (prio < ZRAM_PRIMARY_COMP || prio >= ZRAM_MAX_COMPS)
>  		return -EINVAL;
>  
> -	ret = comp_params_store(zram, prio, level, dict_path);
> +	ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
>  	return ret ? ret : len;
>  }
>