[PATCH 0/2] zram: support algorithm-specific parameters

[PATCH 0/2] zram: support algorithm-specific parameters

[Sergey Senozhatsky]

This patch set adds support for algorithm-specific parameters.
For now, only deflate-specific winbits can be configured, which
fixes deflate support on some s390 setups.

Sergey Senozhatsky (2):
  zram: rename ZCOMP_PARAM_NO_LEVEL
  zram: support deflate-specific params

 drivers/block/zram/backend_deflate.c | 12 +++++++-----
 drivers/block/zram/backend_lz4.c     |  2 +-
 drivers/block/zram/backend_lz4hc.c   |  2 +-
 drivers/block/zram/backend_zstd.c    |  2 +-
 drivers/block/zram/zcomp.h           |  9 ++++++++-
 drivers/block/zram/zram_drv.c        | 21 +++++++++++++++++----
 6 files changed, 35 insertions(+), 13 deletions(-)

-- 
2.49.0.1045.g170613ef41-goog

[PATCH 1/2] zram: rename ZCOMP_PARAM_NO_LEVEL

[Sergey Senozhatsky]

Use more generic name because this will be default "un-set"
value for more params in the future.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/block/zram/backend_deflate.c | 2 +-
 drivers/block/zram/backend_lz4.c     | 2 +-
 drivers/block/zram/backend_lz4hc.c   | 2 +-
 drivers/block/zram/backend_zstd.c    | 2 +-
 drivers/block/zram/zcomp.h           | 2 +-
 drivers/block/zram/zram_drv.c        | 4 ++--
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index 0f7f252c12f4..4c00b5b6739f 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -22,7 +22,7 @@ static void deflate_release_params(struct zcomp_params *params)
 
 static int deflate_setup_params(struct zcomp_params *params)
 {
-	if (params->level == ZCOMP_PARAM_NO_LEVEL)
+	if (params->level == ZCOMP_PARAM_NOT_SET)
 		params->level = Z_DEFAULT_COMPRESSION;
 
 	return 0;
diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
index 847f3334eb38..daccd60857eb 100644
--- a/drivers/block/zram/backend_lz4.c
+++ b/drivers/block/zram/backend_lz4.c
@@ -18,7 +18,7 @@ static void lz4_release_params(struct zcomp_params *params)
 
 static int lz4_setup_params(struct zcomp_params *params)
 {
-	if (params->level == ZCOMP_PARAM_NO_LEVEL)
+	if (params->level == ZCOMP_PARAM_NOT_SET)
 		params->level = LZ4_ACCELERATION_DEFAULT;
 
 	return 0;
diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
index 5f37d5abcaeb..9e8a35dfa56d 100644
--- a/drivers/block/zram/backend_lz4hc.c
+++ b/drivers/block/zram/backend_lz4hc.c
@@ -18,7 +18,7 @@ static void lz4hc_release_params(struct zcomp_params *params)
 
 static int lz4hc_setup_params(struct zcomp_params *params)
 {
-	if (params->level == ZCOMP_PARAM_NO_LEVEL)
+	if (params->level == ZCOMP_PARAM_NOT_SET)
 		params->level = LZ4HC_DEFAULT_CLEVEL;
 
 	return 0;
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 22c8067536f3..81defb98ed09 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -58,7 +58,7 @@ static int zstd_setup_params(struct zcomp_params *params)
 		return -ENOMEM;
 
 	params->drv_data = zp;
-	if (params->level == ZCOMP_PARAM_NO_LEVEL)
+	if (params->level == ZCOMP_PARAM_NOT_SET)
 		params->level = zstd_default_clevel();
 
 	zp->cprm = zstd_get_params(params->level, PAGE_SIZE);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 25339ed1e07e..cfacdfe9044c 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -5,7 +5,7 @@
 
 #include <linux/mutex.h>
 
-#define ZCOMP_PARAM_NO_LEVEL	INT_MIN
+#define ZCOMP_PARAM_NOT_SET	INT_MIN
 
 /*
  * Immutable driver (backend) parameters. The driver may attach private
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 94e6e9b80bf0..a11b7a6e35f4 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1276,7 +1276,7 @@ static void comp_params_reset(struct zram *zram, u32 prio)
 	struct zcomp_params *params = &zram->params[prio];
 
 	vfree(params->dict);
-	params->level = ZCOMP_PARAM_NO_LEVEL;
+	params->level = ZCOMP_PARAM_NOT_SET;
 	params->dict_sz = 0;
 	params->dict = NULL;
 }
@@ -1308,7 +1308,7 @@ static ssize_t algorithm_params_store(struct device *dev,
 				      const char *buf,
 				      size_t len)
 {
-	s32 prio = ZRAM_PRIMARY_COMP, level = ZCOMP_PARAM_NO_LEVEL;
+	s32 prio = ZRAM_PRIMARY_COMP, level = ZCOMP_PARAM_NOT_SET;
 	char *args, *param, *val, *algo = NULL, *dict_path = NULL;
 	struct zram *zram = dev_to_zram(dev);
 	int ret;
-- 
2.49.0.1045.g170613ef41-goog

Re: [PATCH 1/2] zram: rename ZCOMP_PARAM_NO_LEVEL

[Zaslonko Mikhail]

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

On 14.05.2025 04:47, Sergey Senozhatsky wrote:
> Use more generic name because this will be default "un-set"
> value for more params in the future.
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  drivers/block/zram/backend_deflate.c | 2 +-
>  drivers/block/zram/backend_lz4.c     | 2 +-
>  drivers/block/zram/backend_lz4hc.c   | 2 +-
>  drivers/block/zram/backend_zstd.c    | 2 +-
>  drivers/block/zram/zcomp.h           | 2 +-
>  drivers/block/zram/zram_drv.c        | 4 ++--
>  6 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
> index 0f7f252c12f4..4c00b5b6739f 100644
> --- a/drivers/block/zram/backend_deflate.c
> +++ b/drivers/block/zram/backend_deflate.c
> @@ -22,7 +22,7 @@ static void deflate_release_params(struct zcomp_params *params)
>  
>  static int deflate_setup_params(struct zcomp_params *params)
>  {
> -	if (params->level == ZCOMP_PARAM_NO_LEVEL)
> +	if (params->level == ZCOMP_PARAM_NOT_SET)
>  		params->level = Z_DEFAULT_COMPRESSION;
>  
>  	return 0;
> diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
> index 847f3334eb38..daccd60857eb 100644
> --- a/drivers/block/zram/backend_lz4.c
> +++ b/drivers/block/zram/backend_lz4.c
> @@ -18,7 +18,7 @@ static void lz4_release_params(struct zcomp_params *params)
>  
>  static int lz4_setup_params(struct zcomp_params *params)
>  {
> -	if (params->level == ZCOMP_PARAM_NO_LEVEL)
> +	if (params->level == ZCOMP_PARAM_NOT_SET)
>  		params->level = LZ4_ACCELERATION_DEFAULT;
>  
>  	return 0;
> diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
> index 5f37d5abcaeb..9e8a35dfa56d 100644
> --- a/drivers/block/zram/backend_lz4hc.c
> +++ b/drivers/block/zram/backend_lz4hc.c
> @@ -18,7 +18,7 @@ static void lz4hc_release_params(struct zcomp_params *params)
>  
>  static int lz4hc_setup_params(struct zcomp_params *params)
>  {
> -	if (params->level == ZCOMP_PARAM_NO_LEVEL)
> +	if (params->level == ZCOMP_PARAM_NOT_SET)
>  		params->level = LZ4HC_DEFAULT_CLEVEL;
>  
>  	return 0;
> diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
> index 22c8067536f3..81defb98ed09 100644
> --- a/drivers/block/zram/backend_zstd.c
> +++ b/drivers/block/zram/backend_zstd.c
> @@ -58,7 +58,7 @@ static int zstd_setup_params(struct zcomp_params *params)
>  		return -ENOMEM;
>  
>  	params->drv_data = zp;
> -	if (params->level == ZCOMP_PARAM_NO_LEVEL)
> +	if (params->level == ZCOMP_PARAM_NOT_SET)
>  		params->level = zstd_default_clevel();
>  
>  	zp->cprm = zstd_get_params(params->level, PAGE_SIZE);
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 25339ed1e07e..cfacdfe9044c 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -5,7 +5,7 @@
>  
>  #include <linux/mutex.h>
>  
> -#define ZCOMP_PARAM_NO_LEVEL	INT_MIN
> +#define ZCOMP_PARAM_NOT_SET	INT_MIN
>  
>  /*
>   * Immutable driver (backend) parameters. The driver may attach private
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 94e6e9b80bf0..a11b7a6e35f4 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1276,7 +1276,7 @@ static void comp_params_reset(struct zram *zram, u32 prio)
>  	struct zcomp_params *params = &zram->params[prio];
>  
>  	vfree(params->dict);
> -	params->level = ZCOMP_PARAM_NO_LEVEL;
> +	params->level = ZCOMP_PARAM_NOT_SET;
>  	params->dict_sz = 0;
>  	params->dict = NULL;
>  }
> @@ -1308,7 +1308,7 @@ static ssize_t algorithm_params_store(struct device *dev,
>  				      const char *buf,
>  				      size_t len)
>  {
> -	s32 prio = ZRAM_PRIMARY_COMP, level = ZCOMP_PARAM_NO_LEVEL;
> +	s32 prio = ZRAM_PRIMARY_COMP, level = ZCOMP_PARAM_NOT_SET;
>  	char *args, *param, *val, *algo = NULL, *dict_path = NULL;
>  	struct zram *zram = dev_to_zram(dev);
>  	int ret;

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

[Sergey Senozhatsky]

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;
 }
 
-- 
2.49.0.1045.g170613ef41-goog

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

[Zaslonko Mikhail]

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

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

[Sergey Senozhatsky]

Cc-ing Herbert

On (25/05/14 12:58), Zaslonko Mikhail wrote:
> 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().

Yeah, so in zram we can use only raw deflate (we decompress only what we
have compressed earlier and the data never leaves the device.)  But in case
of Crypto API I actually don't know, added Herbert to the Cc.

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

I'm not sure if we want this much of s390 specific code in the generic
zram/Crypto API code.  Both of these params can be configured by user-space
via the algorithm_params device attribute.

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

[Herbert Xu]

On Thu, May 15, 2025 at 12:14:38PM +0900, Sergey Senozhatsky wrote:
>
> Yeah, so in zram we can use only raw deflate (we decompress only what we
> have compressed earlier and the data never leaves the device.)  But in case
> of Crypto API I actually don't know, added Herbert to the Cc.

The Crypto API has only ever used the raw deflate format.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Sergey Senozhatsky]

On (25/05/15 11:17), Herbert Xu wrote:
> On Thu, May 15, 2025 at 12:14:38PM +0900, Sergey Senozhatsky wrote:
> >
> > Yeah, so in zram we can use only raw deflate (we decompress only what we
> > have compressed earlier and the data never leaves the device.)  But in case
> > of Crypto API I actually don't know, added Herbert to the Cc.
> 
> The Crypto API has only ever used the raw deflate format.

OK, so do we want to limit user-space and permit only "raw deflate"
winbits values. that is only negative ones (either explicitly or
implicitly (by negating the value before zlib API calls))?

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

[Herbert Xu]

On Thu, May 15, 2025 at 12:19:25PM +0900, Sergey Senozhatsky wrote:
>
> OK, so do we want to limit user-space and permit only "raw deflate"
> winbits values. that is only negative ones (either explicitly or
> implicitly (by negating the value before zlib API calls))?

I would suggest that we stick with the zlib values, but filter
out the ones that we don't support/use currently.  If you've already
exported this to user-space then obviously you'll need to decide
on how to maintain compatibility but that should be specific to
zram.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Sergey Senozhatsky]

On (25/05/15 11:24), Herbert Xu wrote:
> On Thu, May 15, 2025 at 12:19:25PM +0900, Sergey Senozhatsky wrote:
> >
> > OK, so do we want to limit user-space and permit only "raw deflate"
> > winbits values. that is only negative ones (either explicitly or
> > implicitly (by negating the value before zlib API calls))?
> 
> I would suggest that we stick with the zlib values, but filter
> out the ones that we don't support/use currently.  If you've already
> exported this to user-space then obviously you'll need to decide
> on how to maintain compatibility but that should be specific to
> zram.

This is not exported yet.

I lean toward not filtering/limiting anything and just permit
what include/linux/zlib.h promises [1].  Would that be OK for
Crypto API?

[1]
----

     The windowBits parameter is the base two logarithm of the maximum window
   size (the size of the history buffer).  It should be in the range 8..15 for
   this version of the library. The default value is 15 if inflateInit is used
   instead. windowBits must be greater than or equal to the windowBits value
   provided to deflateInit2() while compressing, or it must be equal to 15 if
   deflateInit2() was not used. If a compressed stream with a larger window
   size is given as input, inflate() will return with the error code
   Z_DATA_ERROR instead of trying to allocate a larger window.

     windowBits can also be -8..-15 for raw inflate. In this case, -windowBits
   determines the window size. inflate() will then process raw deflate data,
   not looking for a zlib or gzip header, not generating a check value, and not
   looking for any check values for comparison at the end of the stream. This
   is for use with other formats that use the deflate compressed data format
   such as zip.  Those formats provide their own check values. If a custom
   format is developed using the raw deflate format for compressed data, it is
   recommended that a check value such as an adler32 or a crc32 be applied to
   the uncompressed data as is done in the zlib, gzip, and zip formats.  For
   most applications, the zlib format should be used as is. Note that comments
   above on the use in deflateInit2() applies to the magnitude of windowBits.

     windowBits can also be greater than 15 for optional gzip decoding. Add
   32 to windowBits to enable zlib and gzip decoding with automatic header
   detection, or add 16 to decode only the gzip format (the zlib format will
   return a Z_DATA_ERROR).  If a gzip stream is being decoded, strm->adler is
   a crc32 instead of an adler32.

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

[Herbert Xu]

On Thu, May 15, 2025 at 12:32:39PM +0900, Sergey Senozhatsky wrote:
>
> This is not exported yet.
> 
> I lean toward not filtering/limiting anything and just permit
> what include/linux/zlib.h promises [1].  Would that be OK for
> Crypto API?

I don't have a problem with that.

It just makes the hardware implementor's job a little bit harder,
because the Crypto API requires every implementation of a given
algorithm to be equivalent.  So if the software zlib supports
a full level specification, then so must the s390 version of zlib.
If it cannot support a parameter, then it must provide a software
fallback.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Zaslonko Mikhail]

Hello,

On 15.05.2025 05:38, Herbert Xu wrote:
> On Thu, May 15, 2025 at 12:32:39PM +0900, Sergey Senozhatsky wrote:
>>
>> This is not exported yet.
>>
>> I lean toward not filtering/limiting anything and just permit
>> what include/linux/zlib.h promises [1].  Would that be OK for
>> Crypto API?
> 
> I don't have a problem with that.
> 
> It just makes the hardware implementor's job a little bit harder,
> because the Crypto API requires every implementation of a given
> algorithm to be equivalent.  So if the software zlib supports
> a full level specification, then so must the s390 version of zlib.
> If it cannot support a parameter, then it must provide a software
> fallback.

That's exactly how s390 zlib hardware support works, including
fallback to software. 
My intention was to use specific zlib compression parameters
(window size and compression level), as default values for zram
delflate on s390 to benefit from hardware acceleration.

> 
> Cheers,

Thanks,
Mikhail

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

[Zaslonko Mikhail]

Hello,

On 15.05.2025 05:14, Sergey Senozhatsky wrote:
> Cc-ing Herbert
> 
> On (25/05/14 12:58), Zaslonko Mikhail wrote:
>> Looks good to me.
>>

>> 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;
>> +       }
> 
> I'm not sure if we want this much of s390 specific code in the generic
> zram/Crypto API code.  Both of these params can be configured by user-space
> via the algorithm_params device attribute.

I understand the concern. My intention was to use special defaults for s390
when no algorithm_params configured by the user (which is the common case,
I assume). Do you see other way of doing so without touching zram generic
code?

Thanks,
Mikhail