* [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
@ 2026-07-28 9:29 ` Haoqin Huang
2026-07-29 2:30 ` Sergey Senozhatsky
2026-07-28 9:29 ` [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h Haoqin Huang
` (6 subsequent siblings)
7 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-07-28 9:29 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
comp_params_store() resets old parameters before reading a new dict,
so if kernel_read_file_from_path() fails the params are left broken
and the actual error is swallowed. Fix by reading into a temporary
buffer first, swapping only on success. Use sz <= 0 to also reject
zero-size dicts.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/zram_drv.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index ace65c586072..9ea7ba9d1ed0 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1699,21 +1699,23 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
const char *dict_path,
struct deflate_params *deflate_params)
{
+ void *new_dict = NULL;
ssize_t sz = 0;
- comp_params_reset(zram, prio);
-
if (dict_path) {
- sz = kernel_read_file_from_path(dict_path, 0,
- &zram->params[prio].dict,
- INT_MAX,
- NULL,
- READING_POLICY);
- if (sz < 0)
- return -EINVAL;
+ sz = kernel_read_file_from_path(dict_path, 0, &new_dict,
+ INT_MAX, NULL, READING_POLICY);
+ if (sz <= 0) {
+ vfree(new_dict);
+ if (sz == 0)
+ return -EINVAL;
+ return sz;
+ }
}
+ comp_params_reset(zram, prio);
zram->params[prio].dict_sz = sz;
+ zram->params[prio].dict = new_dict;
zram->params[prio].level = level;
zram->params[prio].deflate.winbits = deflate_params->winbits;
return 0;
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic
2026-07-28 9:29 ` [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic Haoqin Huang
@ 2026-07-29 2:30 ` Sergey Senozhatsky
2026-07-29 4:06 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 2:30 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/28 17:29), Haoqin Huang wrote:
> comp_params_store() resets old parameters before reading a new dict,
> so if kernel_read_file_from_path() fails the params are left broken
> and the actual error is swallowed. Fix by reading into a temporary
> buffer first, swapping only on success. Use sz <= 0 to also reject
> zero-size dicts.
>
> Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
> Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
> ---
> drivers/block/zram/zram_drv.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index ace65c586072..9ea7ba9d1ed0 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1699,21 +1699,23 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> const char *dict_path,
> struct deflate_params *deflate_params)
> {
> + void *new_dict = NULL;
> ssize_t sz = 0;
>
> - comp_params_reset(zram, prio);
I don't see why is that a problem. All you wanted to do here is to
handle zero i_size. Why do we need dict setting to be atomic?
> if (dict_path) {
> - sz = kernel_read_file_from_path(dict_path, 0,
> - &zram->params[prio].dict,
> - INT_MAX,
> - NULL,
> - READING_POLICY);
> - if (sz < 0)
> - return -EINVAL;
> + sz = kernel_read_file_from_path(dict_path, 0, &new_dict,
> + INT_MAX, NULL, READING_POLICY);
> + if (sz <= 0) {
> + vfree(new_dict);
> + if (sz == 0)
> + return -EINVAL;
> + return sz;
> + }
> }
>
> + comp_params_reset(zram, prio);
> zram->params[prio].dict_sz = sz;
> + zram->params[prio].dict = new_dict;
> zram->params[prio].level = level;
> zram->params[prio].deflate.winbits = deflate_params->winbits;
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic
2026-07-29 2:30 ` Sergey Senozhatsky
@ 2026-07-29 4:06 ` haoqin huang
2026-07-29 4:16 ` Sergey Senozhatsky
0 siblings, 1 reply; 81+ messages in thread
From: haoqin huang @ 2026-07-29 4:06 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 10:30 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/28 17:29), Haoqin Huang wrote:
> > comp_params_store() resets old parameters before reading a new dict,
> > so if kernel_read_file_from_path() fails the params are left broken
> > and the actual error is swallowed. Fix by reading into a temporary
> > buffer first, swapping only on success. Use sz <= 0 to also reject
> > zero-size dicts.
> >
> > Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
> > Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
> > ---
> > drivers/block/zram/zram_drv.c | 20 +++++++++++---------
> > 1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index ace65c586072..9ea7ba9d1ed0 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -1699,21 +1699,23 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> > const char *dict_path,
> > struct deflate_params *deflate_params)
> > {
> > + void *new_dict = NULL;
> > ssize_t sz = 0;
> >
> > - comp_params_reset(zram, prio);
>
> I don't see why is that a problem. All you wanted to do here is to
> handle zero i_size. Why do we need dict setting to be atomic?
>
comp_params_reset() calls vfree() on the old dict and resets level/
winbits to NOT_SET before reading the new dict. If
kernel_read_file_from_path() then fails for any reason, not just
zero-size, but also ENOENT, ENOMEM, etc. the old dict is already
freed and unrecoverable, and the params are left in a broken,
half-reset state.
The "atomic" in the subject is about all-or-nothing semantics: don't
destroy valid state until the replacement is confirmed good. Reading
into a temp buffer first, then swapping only on success, is the natural
way to do that. The sz <= 0 check falls out naturally.
That said, if you prefer a more minimal fix, I can drop the temp-buffer
approach and just change sz < 0 to sz <= 0. But since reading into
a temp buffer first protects against all failure paths, not just zero-size,
it seemed worth doing in one step.
> > if (dict_path) {
> > - sz = kernel_read_file_from_path(dict_path, 0,
> > - &zram->params[prio].dict,
> > - INT_MAX,
> > - NULL,
> > - READING_POLICY);
> > - if (sz < 0)
> > - return -EINVAL;
> > + sz = kernel_read_file_from_path(dict_path, 0, &new_dict,
> > + INT_MAX, NULL, READING_POLICY);
> > + if (sz <= 0) {
> > + vfree(new_dict);
> > + if (sz == 0)
> > + return -EINVAL;
> > + return sz;
> > + }
> > }
> >
> > + comp_params_reset(zram, prio);
> > zram->params[prio].dict_sz = sz;
> > + zram->params[prio].dict = new_dict;
> > zram->params[prio].level = level;
> > zram->params[prio].deflate.winbits = deflate_params->winbits;
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic
2026-07-29 4:06 ` haoqin huang
@ 2026-07-29 4:16 ` Sergey Senozhatsky
2026-07-29 4:25 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 4:16 UTC (permalink / raw)
To: haoqin huang
Cc: Sergey Senozhatsky, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/29 12:06), haoqin huang wrote:
> > On (26/07/28 17:29), Haoqin Huang wrote:
[..]
> > > @@ -1699,21 +1699,23 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> > > const char *dict_path,
> > > struct deflate_params *deflate_params)
> > > {
> > > + void *new_dict = NULL;
> > > ssize_t sz = 0;
> > >
> > > - comp_params_reset(zram, prio);
> >
> > I don't see why is that a problem. All you wanted to do here is to
> > handle zero i_size. Why do we need dict setting to be atomic?
> >
>
> comp_params_reset() calls vfree() on the old dict and resets level/
> winbits to NOT_SET before reading the new dict
But what is the scenario here? Who would have several dicts?
echo "dict=/etc/dict.foo prio=1" > algorithm_params
and if that fails then
echo "dict=/etc/dict.bar prio=1" > algorithm_params
I don't think this is something that we need to consider. What am I
missing?
[..]
> The "atomic" in the subject is about all-or-nothing semantics: don't
> destroy valid state until the replacement is confirmed good.
I don't think that "valid state configuration replacement" ever happens.
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic
2026-07-29 4:16 ` Sergey Senozhatsky
@ 2026-07-29 4:25 ` haoqin huang
0 siblings, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-07-29 4:25 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 12:16 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/29 12:06), haoqin huang wrote:
> > > On (26/07/28 17:29), Haoqin Huang wrote:
> [..]
> > > > @@ -1699,21 +1699,23 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> > > > const char *dict_path,
> > > > struct deflate_params *deflate_params)
> > > > {
> > > > + void *new_dict = NULL;
> > > > ssize_t sz = 0;
> > > >
> > > > - comp_params_reset(zram, prio);
> > >
> > > I don't see why is that a problem. All you wanted to do here is to
> > > handle zero i_size. Why do we need dict setting to be atomic?
> > >
> >
> > comp_params_reset() calls vfree() on the old dict and resets level/
> > winbits to NOT_SET before reading the new dict
>
> But what is the scenario here? Who would have several dicts?
>
> echo "dict=/etc/dict.foo prio=1" > algorithm_params
>
> and if that fails then
>
> echo "dict=/etc/dict.bar prio=1" > algorithm_params
>
> I don't think this is something that we need to consider. What am I
> missing?
>
> [..]
> > The "atomic" in the subject is about all-or-nothing semantics: don't
> > destroy valid state until the replacement is confirmed good.
>
> I don't think that "valid state configuration replacement" ever happens.
You're right, the scenario I had in mind was a bit far-fetched. It requires
loading dict_a first, then trying to replace it with dict_b before init, with
the second read failing:
echo "algo=zstd dict=/path/dict_a" > algorithm_params
echo "algo=zstd dict=/path/dict_b" > algorithm_params # fails
comp_params_reset() would free dict_a before dict_b fails to load. But in
practice params are set once before init and never change.
I'll simplify this to just the sz <= 0 change in v3.
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
2026-07-28 9:29 ` [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic Haoqin Huang
@ 2026-07-28 9:29 ` Haoqin Huang
2026-07-29 3:03 ` Sergey Senozhatsky
2026-07-28 9:29 ` [PATCH v2 4/5] zram: add per-backend caps and validate parameters early Haoqin Huang
` (5 subsequent siblings)
7 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-07-28 9:29 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Move ZSTD_MAX_CLEVEL from lib/zstd/compress/clevels.h to
include/linux/zstd_lib.h so that external users (e.g. the zram
zstd backend) can reference it alongside ZSTD_TARGETLENGTH_MAX
to define a valid level range.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
include/linux/zstd_lib.h | 1 +
lib/zstd/compress/clevels.h | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/zstd_lib.h b/include/linux/zstd_lib.h
index e295d4125dde..f4b26844e53a 100644
--- a/include/linux/zstd_lib.h
+++ b/include/linux/zstd_lib.h
@@ -1241,6 +1241,7 @@ ZSTDLIB_API size_t ZSTD_sizeof_DDict(const ZSTD_DDict* ddict);
#define ZSTD_SEARCHLOG_MIN 1
#define ZSTD_MINMATCH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */
#define ZSTD_MINMATCH_MIN 3 /* only for ZSTD_btopt+, faster strategies are limited to 4 */
+#define ZSTD_MAX_CLEVEL 22
#define ZSTD_TARGETLENGTH_MAX ZSTD_BLOCKSIZE_MAX
#define ZSTD_TARGETLENGTH_MIN 0 /* note : comparing this constant to an unsigned results in a tautological test */
#define ZSTD_STRATEGY_MIN ZSTD_fast
diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
index 6ab8be6532ef..06565e064456 100644
--- a/lib/zstd/compress/clevels.h
+++ b/lib/zstd/compress/clevels.h
@@ -17,8 +17,6 @@
/*-===== Pre-defined compression levels =====-*/
-#define ZSTD_MAX_CLEVEL 22
-
__attribute__((__unused__))
static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEVEL+1] = {
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
2026-07-28 9:29 ` [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h Haoqin Huang
@ 2026-07-29 3:03 ` Sergey Senozhatsky
2026-07-29 4:16 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 3:03 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/28 17:29), Haoqin Huang wrote:
[..]
> diff --git a/include/linux/zstd_lib.h b/include/linux/zstd_lib.h
> index e295d4125dde..f4b26844e53a 100644
> --- a/include/linux/zstd_lib.h
> +++ b/include/linux/zstd_lib.h
> @@ -1241,6 +1241,7 @@ ZSTDLIB_API size_t ZSTD_sizeof_DDict(const ZSTD_DDict* ddict);
> #define ZSTD_SEARCHLOG_MIN 1
> #define ZSTD_MINMATCH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */
> #define ZSTD_MINMATCH_MIN 3 /* only for ZSTD_btopt+, faster strategies are limited to 4 */
> +#define ZSTD_MAX_CLEVEL 22
> #define ZSTD_TARGETLENGTH_MAX ZSTD_BLOCKSIZE_MAX
> #define ZSTD_TARGETLENGTH_MIN 0 /* note : comparing this constant to an unsigned results in a tautological test */
> #define ZSTD_STRATEGY_MIN ZSTD_fast
> diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
> index 6ab8be6532ef..06565e064456 100644
> --- a/lib/zstd/compress/clevels.h
> +++ b/lib/zstd/compress/clevels.h
> @@ -17,8 +17,6 @@
>
> /*-===== Pre-defined compression levels =====-*/
>
> -#define ZSTD_MAX_CLEVEL 22
> -
> __attribute__((__unused__))
Sashiko made a good point. Can we use zstd_max_clevel() instead?
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
2026-07-29 3:03 ` Sergey Senozhatsky
@ 2026-07-29 4:16 ` haoqin huang
2026-07-29 4:22 ` Sergey Senozhatsky
0 siblings, 1 reply; 81+ messages in thread
From: haoqin huang @ 2026-07-29 4:16 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 11:03 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/28 17:29), Haoqin Huang wrote:
> [..]
> > diff --git a/include/linux/zstd_lib.h b/include/linux/zstd_lib.h
> > index e295d4125dde..f4b26844e53a 100644
> > --- a/include/linux/zstd_lib.h
> > +++ b/include/linux/zstd_lib.h
> > @@ -1241,6 +1241,7 @@ ZSTDLIB_API size_t ZSTD_sizeof_DDict(const ZSTD_DDict* ddict);
> > #define ZSTD_SEARCHLOG_MIN 1
> > #define ZSTD_MINMATCH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */
> > #define ZSTD_MINMATCH_MIN 3 /* only for ZSTD_btopt+, faster strategies are limited to 4 */
> > +#define ZSTD_MAX_CLEVEL 22
> > #define ZSTD_TARGETLENGTH_MAX ZSTD_BLOCKSIZE_MAX
> > #define ZSTD_TARGETLENGTH_MIN 0 /* note : comparing this constant to an unsigned results in a tautological test */
> > #define ZSTD_STRATEGY_MIN ZSTD_fast
> > diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
> > index 6ab8be6532ef..06565e064456 100644
> > --- a/lib/zstd/compress/clevels.h
> > +++ b/lib/zstd/compress/clevels.h
> > @@ -17,8 +17,6 @@
> >
> > /*-===== Pre-defined compression levels =====-*/
> >
> > -#define ZSTD_MAX_CLEVEL 22
> > -
> > __attribute__((__unused__))
>
> Sashiko made a good point. Can we use zstd_max_clevel() instead?
Good point, I'll drop this patch and use zstd_max_clevel() instead
in v3.
Since it's a runtime function and can't be used for static struct
initialization, I plan to set backend_zstd's level_max to -1 as a
sentinel value, and query the actual maximum in
zcomp_validate_params():
s32 max = backend->level_max;
if (max < 0)
max = zstd_max_clevel();
Do you think this approach is feasible?
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
2026-07-29 4:16 ` haoqin huang
@ 2026-07-29 4:22 ` Sergey Senozhatsky
2026-07-29 4:32 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 4:22 UTC (permalink / raw)
To: haoqin huang
Cc: Sergey Senozhatsky, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/29 12:16), haoqin huang wrote:
> > On (26/07/28 17:29), Haoqin Huang wrote:
> > [..]
> > > #define ZSTD_MINMATCH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */
> > > #define ZSTD_MINMATCH_MIN 3 /* only for ZSTD_btopt+, faster strategies are limited to 4 */
> > > +#define ZSTD_MAX_CLEVEL 22
> > > #define ZSTD_TARGETLENGTH_MAX ZSTD_BLOCKSIZE_MAX
> > > #define ZSTD_TARGETLENGTH_MIN 0 /* note : comparing this constant to an unsigned results in a tautological test */
> > > #define ZSTD_STRATEGY_MIN ZSTD_fast
> > > diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
> > > index 6ab8be6532ef..06565e064456 100644
> > > --- a/lib/zstd/compress/clevels.h
> > > +++ b/lib/zstd/compress/clevels.h
> > > @@ -17,8 +17,6 @@
> > >
> > > /*-===== Pre-defined compression levels =====-*/
> > >
> > > -#define ZSTD_MAX_CLEVEL 22
> > > -
> > > __attribute__((__unused__))
> >
> > Sashiko made a good point. Can we use zstd_max_clevel() instead?
>
> Good point, I'll drop this patch and use zstd_max_clevel() instead
> in v3.
>
> Since it's a runtime function and can't be used for static struct
> initialization, I plan to set backend_zstd's level_max to -1 as a
> sentinel value, and query the actual maximum in
> zcomp_validate_params():
>
> s32 max = backend->level_max;
> if (max < 0)
> max = zstd_max_clevel();
>
> Do you think this approach is feasible?
Hmm, no, that doesn't look good. zcomp should not include
zstd.h or any other libs directly. Should params validation
be a per-backend callback then?
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
2026-07-29 4:22 ` Sergey Senozhatsky
@ 2026-07-29 4:32 ` haoqin huang
2026-07-29 4:49 ` Sergey Senozhatsky
0 siblings, 1 reply; 81+ messages in thread
From: haoqin huang @ 2026-07-29 4:32 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 12:23 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/29 12:16), haoqin huang wrote:
> > > On (26/07/28 17:29), Haoqin Huang wrote:
> > > [..]
> > > > #define ZSTD_MINMATCH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */
> > > > #define ZSTD_MINMATCH_MIN 3 /* only for ZSTD_btopt+, faster strategies are limited to 4 */
> > > > +#define ZSTD_MAX_CLEVEL 22
> > > > #define ZSTD_TARGETLENGTH_MAX ZSTD_BLOCKSIZE_MAX
> > > > #define ZSTD_TARGETLENGTH_MIN 0 /* note : comparing this constant to an unsigned results in a tautological test */
> > > > #define ZSTD_STRATEGY_MIN ZSTD_fast
> > > > diff --git a/lib/zstd/compress/clevels.h b/lib/zstd/compress/clevels.h
> > > > index 6ab8be6532ef..06565e064456 100644
> > > > --- a/lib/zstd/compress/clevels.h
> > > > +++ b/lib/zstd/compress/clevels.h
> > > > @@ -17,8 +17,6 @@
> > > >
> > > > /*-===== Pre-defined compression levels =====-*/
> > > >
> > > > -#define ZSTD_MAX_CLEVEL 22
> > > > -
> > > > __attribute__((__unused__))
> > >
> > > Sashiko made a good point. Can we use zstd_max_clevel() instead?
> >
> > Good point, I'll drop this patch and use zstd_max_clevel() instead
> > in v3.
> >
> > Since it's a runtime function and can't be used for static struct
> > initialization, I plan to set backend_zstd's level_max to -1 as a
> > sentinel value, and query the actual maximum in
> > zcomp_validate_params():
> >
> > s32 max = backend->level_max;
> > if (max < 0)
> > max = zstd_max_clevel();
> >
> > Do you think this approach is feasible?
>
> Hmm, no, that doesn't look good. zcomp should not include
> zstd.h or any other libs directly. Should params validation
> be a per-backend callback then?
Good point, zcomp.c shouldn't include library headers. A per-backend
callback would be cleaner.
I'll add an optional validate_params to zcomp_ops: the zstd backend
implements it using zstd_max_clevel() internally, while lzo/deflate
and others just rely on the static caps check (no callback needed).
zcomp.c stays free of any library headers.
I'll send this in v3.
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
2026-07-29 4:32 ` haoqin huang
@ 2026-07-29 4:49 ` Sergey Senozhatsky
2026-07-30 2:48 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 4:49 UTC (permalink / raw)
To: haoqin huang
Cc: Sergey Senozhatsky, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/29 12:32), haoqin huang wrote:
> > > > > -#define ZSTD_MAX_CLEVEL 22
> > > > > -
> > > > > __attribute__((__unused__))
> > > >
> > > > Sashiko made a good point. Can we use zstd_max_clevel() instead?
> > >
> > > Good point, I'll drop this patch and use zstd_max_clevel() instead
> > > in v3.
> > >
> > > Since it's a runtime function and can't be used for static struct
> > > initialization, I plan to set backend_zstd's level_max to -1 as a
> > > sentinel value, and query the actual maximum in
> > > zcomp_validate_params():
> > >
> > > s32 max = backend->level_max;
> > > if (max < 0)
> > > max = zstd_max_clevel();
> > >
> > > Do you think this approach is feasible?
> >
> > Hmm, no, that doesn't look good. zcomp should not include
> > zstd.h or any other libs directly. Should params validation
> > be a per-backend callback then?
>
> Good point, zcomp.c shouldn't include library headers. A per-backend
> callback would be cleaner.
>
> I'll add an optional validate_params to zcomp_ops: the zstd backend
> implements it using zstd_max_clevel() internally, while lzo/deflate
> and others just rely on the static caps check (no callback needed).
> zcomp.c stays free of any library headers.
I'm actually leaning towards validation in .setup_params() now.
It's not immediate but, first, it doesn't matter that much, we still
don't create zram with invalid params configuration and, second, it
sort of makes sense to do validation in .setup_params(). We already
started doing that for deflate winbits (a patch from earlier today).
Can you please add your validation to per-backend .setup_params()?
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
2026-07-29 4:49 ` Sergey Senozhatsky
@ 2026-07-30 2:48 ` haoqin huang
0 siblings, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-07-30 2:48 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 12:50 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/29 12:32), haoqin huang wrote:
> > > > > > -#define ZSTD_MAX_CLEVEL 22
> > > > > > -
> > > > > > __attribute__((__unused__))
> > > > >
> > > > > Sashiko made a good point. Can we use zstd_max_clevel() instead?
> > > >
> > > > Good point, I'll drop this patch and use zstd_max_clevel() instead
> > > > in v3.
> > > >
> > > > Since it's a runtime function and can't be used for static struct
> > > > initialization, I plan to set backend_zstd's level_max to -1 as a
> > > > sentinel value, and query the actual maximum in
> > > > zcomp_validate_params():
> > > >
> > > > s32 max = backend->level_max;
> > > > if (max < 0)
> > > > max = zstd_max_clevel();
> > > >
> > > > Do you think this approach is feasible?
> > >
> > > Hmm, no, that doesn't look good. zcomp should not include
> > > zstd.h or any other libs directly. Should params validation
> > > be a per-backend callback then?
> >
> > Good point, zcomp.c shouldn't include library headers. A per-backend
> > callback would be cleaner.
> >
> > I'll add an optional validate_params to zcomp_ops: the zstd backend
> > implements it using zstd_max_clevel() internally, while lzo/deflate
> > and others just rely on the static caps check (no callback needed).
> > zcomp.c stays free of any library headers.
>
> I'm actually leaning towards validation in .setup_params() now.
> It's not immediate but, first, it doesn't matter that much, we still
> don't create zram with invalid params configuration and, second, it
> sort of makes sense to do validation in .setup_params(). We already
> started doing that for deflate winbits (a patch from earlier today).
>
> Can you please add your validation to per-backend .setup_params()?
Agreed, I'll move the validation to zstd_setup_params() in v3.
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v2 4/5] zram: add per-backend caps and validate parameters early
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
2026-07-28 9:29 ` [PATCH v2 2/5] zram: make dict update in comp_params_store() atomic Haoqin Huang
2026-07-28 9:29 ` [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h Haoqin Huang
@ 2026-07-28 9:29 ` Haoqin Huang
2026-07-29 2:26 ` Sergey Senozhatsky
2026-07-28 9:29 ` [PATCH v2 5/5] zram: reset per-priority params when changing algorithm before init Haoqin Huang
` (4 subsequent siblings)
7 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-07-28 9:29 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Writing dict or out-of-range level for backends that don't support
them is currently silently accepted but has no effect. For example,
"algo=lzo dict=/data/dict" succeeds but lzo_setup_params() ignores
the dict entirely.
Add caps, level_min and level_max to zcomp_ops and validate
user-supplied parameters in algorithm_params_store() before storing,
printing an error on failure so the user knows what went wrong.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_842.c | 1 +
drivers/block/zram/backend_deflate.c | 3 +++
drivers/block/zram/backend_lz4.c | 3 +++
drivers/block/zram/backend_lz4hc.c | 3 +++
drivers/block/zram/backend_lzo.c | 1 +
drivers/block/zram/backend_lzorle.c | 1 +
drivers/block/zram/backend_zstd.c | 3 +++
drivers/block/zram/zcomp.c | 23 +++++++++++++++++++++++
drivers/block/zram/zcomp.h | 8 ++++++++
drivers/block/zram/zram_drv.c | 19 +++++++++++++++++++
10 files changed, 65 insertions(+)
diff --git a/drivers/block/zram/backend_842.c b/drivers/block/zram/backend_842.c
index 10d9d5c60f53..d796ebda1fa0 100644
--- a/drivers/block/zram/backend_842.c
+++ b/drivers/block/zram/backend_842.c
@@ -57,5 +57,6 @@ const struct zcomp_ops backend_842 = {
.destroy_ctx = destroy_842,
.setup_params = setup_params_842,
.release_params = release_params_842,
+ .caps = 0,
.name = "842",
};
diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index f92a52a720d1..cedc3daad33a 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -144,5 +144,8 @@ const struct zcomp_ops backend_deflate = {
.destroy_ctx = deflate_destroy,
.setup_params = deflate_setup_params,
.release_params = deflate_release_params,
+ .caps = ZCOMP_CAP_LEVEL,
+ .level_min = Z_DEFAULT_COMPRESSION,
+ .level_max = Z_BEST_COMPRESSION,
.name = "deflate",
};
diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
index c449d511ba86..bd1e5ca4d134 100644
--- a/drivers/block/zram/backend_lz4.c
+++ b/drivers/block/zram/backend_lz4.c
@@ -146,5 +146,8 @@ const struct zcomp_ops backend_lz4 = {
.destroy_ctx = lz4_destroy,
.setup_params = lz4_setup_params,
.release_params = lz4_release_params,
+ .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL,
+ .level_min = LZ4_ACCELERATION_DEFAULT,
+ .level_max = 65535,
.name = "lz4",
};
diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
index f6a336acfe20..0e0d7c68a7d4 100644
--- a/drivers/block/zram/backend_lz4hc.c
+++ b/drivers/block/zram/backend_lz4hc.c
@@ -124,5 +124,8 @@ const struct zcomp_ops backend_lz4hc = {
.destroy_ctx = lz4hc_destroy,
.setup_params = lz4hc_setup_params,
.release_params = lz4hc_release_params,
+ .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL,
+ .level_min = LZ4HC_MIN_CLEVEL,
+ .level_max = LZ4HC_MAX_CLEVEL,
.name = "lz4hc",
};
diff --git a/drivers/block/zram/backend_lzo.c b/drivers/block/zram/backend_lzo.c
index 4c906beaae6b..965f007e2ca8 100644
--- a/drivers/block/zram/backend_lzo.c
+++ b/drivers/block/zram/backend_lzo.c
@@ -55,5 +55,6 @@ const struct zcomp_ops backend_lzo = {
.destroy_ctx = lzo_destroy,
.setup_params = lzo_setup_params,
.release_params = lzo_release_params,
+ .caps = 0,
.name = "lzo",
};
diff --git a/drivers/block/zram/backend_lzorle.c b/drivers/block/zram/backend_lzorle.c
index 10640c96cbfc..757b4598be03 100644
--- a/drivers/block/zram/backend_lzorle.c
+++ b/drivers/block/zram/backend_lzorle.c
@@ -55,5 +55,6 @@ const struct zcomp_ops backend_lzorle = {
.destroy_ctx = lzorle_destroy,
.setup_params = lzorle_setup_params,
.release_params = lzorle_release_params,
+ .caps = 0,
.name = "lzo-rle",
};
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 2584f47c9b3c..0fbd2460883a 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -212,5 +212,8 @@ const struct zcomp_ops backend_zstd = {
.destroy_ctx = zstd_destroy,
.setup_params = zstd_setup_params,
.release_params = zstd_release_params,
+ .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL,
+ .level_min = (int)-ZSTD_TARGETLENGTH_MAX,
+ .level_max = ZSTD_MAX_CLEVEL,
.name = "zstd",
};
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 974c4691887e..15de28b50d42 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -9,6 +9,9 @@
#include <linux/cpuhotplug.h>
#include <linux/vmalloc.h>
#include <linux/sysfs.h>
+#include <linux/lz4.h>
+#include <linux/zlib.h>
+#include <linux/zstd.h>
#include "zcomp.h"
@@ -94,6 +97,26 @@ const char *zcomp_lookup_backend_name(const char *comp)
return NULL;
}
+unsigned int zcomp_get_caps(const char *comp)
+{
+ const struct zcomp_ops *backend = lookup_backend_ops(comp);
+
+ return backend ? backend->caps : 0;
+}
+
+int zcomp_validate_level(const char *comp, s32 level)
+{
+ const struct zcomp_ops *backend = lookup_backend_ops(comp);
+
+ if (!backend)
+ return -EINVAL;
+ if (!(backend->caps & ZCOMP_CAP_LEVEL))
+ return -EOPNOTSUPP;
+ if (level < backend->level_min || level > backend->level_max)
+ return -EINVAL;
+ return 0;
+}
+
/* show available compressors */
ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at)
{
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 81a0f3f6ff48..16f812a94c67 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -7,6 +7,9 @@
#define ZCOMP_PARAM_NOT_SET INT_MIN
+#define ZCOMP_CAP_DICT BIT(0) /* dictionary support */
+#define ZCOMP_CAP_LEVEL BIT(1) /* adjustable compression level */
+
struct deflate_params {
s32 winbits;
};
@@ -66,6 +69,9 @@ struct zcomp_ops {
int (*setup_params)(struct zcomp_params *params);
void (*release_params)(struct zcomp_params *params);
+ unsigned int caps;
+ s32 level_min;
+ s32 level_max;
const char *name;
};
@@ -81,6 +87,8 @@ int zcomp_cpu_up_prepare(unsigned int cpu, struct hlist_node *node);
int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node);
ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at);
const char *zcomp_lookup_backend_name(const char *comp);
+unsigned int zcomp_get_caps(const char *comp);
+int zcomp_validate_level(const char *comp, s32 level);
struct zcomp *zcomp_create(const char *alg, struct zcomp_params *params);
void zcomp_destroy(struct zcomp *comp);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 9ea7ba9d1ed0..d86f4eb06d58 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1796,6 +1796,25 @@ static ssize_t algorithm_params_store(struct device *dev,
return -EINVAL;
}
+ if (zram->comp_algs[prio]) {
+ unsigned int caps = zcomp_get_caps(zram->comp_algs[prio]);
+
+ if (dict_path && !(caps & ZCOMP_CAP_DICT)) {
+ pr_err("zram: %s does not support dictionary\n",
+ zram->comp_algs[prio]);
+ return -EOPNOTSUPP;
+ }
+
+ if (level != ZCOMP_PARAM_NOT_SET) {
+ ret = zcomp_validate_level(zram->comp_algs[prio], level);
+ if (ret) {
+ pr_err("zram: invalid level for %s\n",
+ zram->comp_algs[prio]);
+ return ret;
+ }
+ }
+ }
+
ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
return ret ? ret : len;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 4/5] zram: add per-backend caps and validate parameters early
2026-07-28 9:29 ` [PATCH v2 4/5] zram: add per-backend caps and validate parameters early Haoqin Huang
@ 2026-07-29 2:26 ` Sergey Senozhatsky
2026-07-29 3:53 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 2:26 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/28 17:29), Haoqin Huang wrote:
[..]
> +unsigned int zcomp_get_caps(const char *comp)
> +{
> + const struct zcomp_ops *backend = lookup_backend_ops(comp);
> +
> + return backend ? backend->caps : 0;
> +}
> +
> +int zcomp_validate_level(const char *comp, s32 level)
> +{
> + const struct zcomp_ops *backend = lookup_backend_ops(comp);
> +
> + if (!backend)
> + return -EINVAL;
> + if (!(backend->caps & ZCOMP_CAP_LEVEL))
> + return -EOPNOTSUPP;
> + if (level < backend->level_min || level > backend->level_max)
> + return -EINVAL;
> + return 0;
> +}
[..]
> +unsigned int zcomp_get_caps(const char *comp);
> +int zcomp_validate_level(const char *comp, s32 level);
[..]
> @@ -1796,6 +1796,25 @@ static ssize_t algorithm_params_store(struct device *dev,
> return -EINVAL;
> }
>
> + if (zram->comp_algs[prio]) {
> + unsigned int caps = zcomp_get_caps(zram->comp_algs[prio]);
> +
> + if (dict_path && !(caps & ZCOMP_CAP_DICT)) {
> + pr_err("zram: %s does not support dictionary\n",
> + zram->comp_algs[prio]);
> + return -EOPNOTSUPP;
> + }
> +
> + if (level != ZCOMP_PARAM_NOT_SET) {
> + ret = zcomp_validate_level(zram->comp_algs[prio], level);
> + if (ret) {
> + pr_err("zram: invalid level for %s\n",
> + zram->comp_algs[prio]);
> + return ret;
> + }
> + }
> + }
So I wonder if instead of introducing 2 new zcomp functions (zcomp_get_caps()
and zcomp_validate_level()) and still basically open-coding params verification
in zram, maybe we we can just have one
int zcomp_validate_params(comp, level, dict_path)
and handle all the validation in zcomp internally. So that in
algorithm_params_store() it will be just
ret = zcomp_validate_params(zram->comp_algs[prio], level, dict_path);
if (ret)
return ret;
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 4/5] zram: add per-backend caps and validate parameters early
2026-07-29 2:26 ` Sergey Senozhatsky
@ 2026-07-29 3:53 ` haoqin huang
0 siblings, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-07-29 3:53 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 10:26 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/28 17:29), Haoqin Huang wrote:
> [..]
> > +unsigned int zcomp_get_caps(const char *comp)
> > +{
> > + const struct zcomp_ops *backend = lookup_backend_ops(comp);
> > +
> > + return backend ? backend->caps : 0;
> > +}
> > +
> > +int zcomp_validate_level(const char *comp, s32 level)
> > +{
> > + const struct zcomp_ops *backend = lookup_backend_ops(comp);
> > +
> > + if (!backend)
> > + return -EINVAL;
> > + if (!(backend->caps & ZCOMP_CAP_LEVEL))
> > + return -EOPNOTSUPP;
> > + if (level < backend->level_min || level > backend->level_max)
> > + return -EINVAL;
> > + return 0;
> > +}
>
> [..]
>
> > +unsigned int zcomp_get_caps(const char *comp);
> > +int zcomp_validate_level(const char *comp, s32 level);
>
> [..]
>
> > @@ -1796,6 +1796,25 @@ static ssize_t algorithm_params_store(struct device *dev,
> > return -EINVAL;
> > }
> >
> > + if (zram->comp_algs[prio]) {
> > + unsigned int caps = zcomp_get_caps(zram->comp_algs[prio]);
> > +
> > + if (dict_path && !(caps & ZCOMP_CAP_DICT)) {
> > + pr_err("zram: %s does not support dictionary\n",
> > + zram->comp_algs[prio]);
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (level != ZCOMP_PARAM_NOT_SET) {
> > + ret = zcomp_validate_level(zram->comp_algs[prio], level);
> > + if (ret) {
> > + pr_err("zram: invalid level for %s\n",
> > + zram->comp_algs[prio]);
> > + return ret;
> > + }
> > + }
> > + }
>
> So I wonder if instead of introducing 2 new zcomp functions (zcomp_get_caps()
> and zcomp_validate_level()) and still basically open-coding params verification
> in zram, maybe we we can just have one
> int zcomp_validate_params(comp, level, dict_path)
> and handle all the validation in zcomp internally. So that in
> algorithm_params_store() it will be just
>
> ret = zcomp_validate_params(zram->comp_algs[prio], level, dict_path);
> if (ret)
> return ret;
Sounds good, I'll merge them into a single zcomp_validate_params() in v3.
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v2 5/5] zram: reset per-priority params when changing algorithm before init
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
` (2 preceding siblings ...)
2026-07-28 9:29 ` [PATCH v2 4/5] zram: add per-backend caps and validate parameters early Haoqin Huang
@ 2026-07-28 9:29 ` Haoqin Huang
2026-07-29 0:34 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path Andrew Morton
` (3 subsequent siblings)
7 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-28 9:29 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Parameters validated against one algorithm may be invalid for another
(e.g. lz4 accepts level=65535 but zstd does not). Although algorithm
changes are blocked after disksize is set, they are allowed before
device initialization. Reset per-priority params on algorithm change
so that stale parameters do not silently carry over.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d86f4eb06d58..da2921082712 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1661,6 +1661,8 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
zram->comp_algs[prio] = alg;
}
+static void comp_params_reset(struct zram *zram, u32 prio);
+
static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
{
const char *alg;
@@ -1681,6 +1683,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
}
comp_algorithm_set(zram, prio, alg);
+ comp_params_reset(zram, prio);
return 0;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
` (3 preceding siblings ...)
2026-07-28 9:29 ` [PATCH v2 5/5] zram: reset per-priority params when changing algorithm before init Haoqin Huang
@ 2026-07-29 0:34 ` Andrew Morton
2026-07-29 2:49 ` Sergey Senozhatsky
2026-07-29 3:33 ` haoqin huang
2026-07-29 2:20 ` Sergey Senozhatsky
` (2 subsequent siblings)
7 siblings, 2 replies; 81+ messages in thread
From: Andrew Morton @ 2026-07-29 0:34 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Tue, 28 Jul 2026 17:29:31 +0800 Haoqin Huang <haoqinhuang7@gmail.com> wrote:
> zstd_setup_params() creates global cdict and ddict stored in
> params->drv_data, shared across all per-CPU contexts. The per-CPU
> zstd_create() error path called zstd_release_params(), which freed
> those globally-shared objects. While the drv_data=NULL guard in
> zstd_release_params() prevents a double-free on the init failure
> path, this is still a layering violation: a per-CPU callback should
> only clean up its own context, not release resources owned by the
> compression lifecycle (zcomp_init / zcomp_destroy).
>
> Fix by removing zstd_release_params() from the per-CPU error path
> and replacing it with only zstd_destroy(), which properly cleans
> up the per-CPU context without touching the global params->drv_data.
Thanks. A [0/N] cover letter would be appropriate.
Do any of these changes have userspace-visible runtime effects? If so,
please changelog these in full detail. If not, a statement (in the
[0/N]!) telling us this would be helpful.
AI review might have found a few things, about half of them
pre-existing (zram/zcomp):
https://sashiko.dev/#/patchset/20260728092935.31139-1-haoqinhuang7@gmail.com
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path
2026-07-29 0:34 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path Andrew Morton
@ 2026-07-29 2:49 ` Sergey Senozhatsky
2026-07-29 3:33 ` haoqin huang
1 sibling, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 2:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Haoqin Huang, Minchan Kim, Sergey Senozhatsky, Jens Axboe,
Nick Terrell, David Sterba, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/28 17:34), Andrew Morton wrote:
> AI review might have found a few things, about half of them
> pre-existing (zram/zcomp):
>
> https://sashiko.dev/#/patchset/20260728092935.31139-1-haoqinhuang7@gmail.com
I'll take a look at those pre-existing issues.
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path
2026-07-29 0:34 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path Andrew Morton
2026-07-29 2:49 ` Sergey Senozhatsky
@ 2026-07-29 3:33 ` haoqin huang
1 sibling, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-07-29 3:33 UTC (permalink / raw)
To: Andrew Morton
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 8:34 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 28 Jul 2026 17:29:31 +0800 Haoqin Huang <haoqinhuang7@gmail.com> wrote:
>
> > zstd_setup_params() creates global cdict and ddict stored in
> > params->drv_data, shared across all per-CPU contexts. The per-CPU
> > zstd_create() error path called zstd_release_params(), which freed
> > those globally-shared objects. While the drv_data=NULL guard in
> > zstd_release_params() prevents a double-free on the init failure
> > path, this is still a layering violation: a per-CPU callback should
> > only clean up its own context, not release resources owned by the
> > compression lifecycle (zcomp_init / zcomp_destroy).
> >
> > Fix by removing zstd_release_params() from the per-CPU error path
> > and replacing it with only zstd_destroy(), which properly cleans
> > up the per-CPU context without touching the global params->drv_data.
>
> Thanks. A [0/N] cover letter would be appropriate.
>
>
> Do any of these changes have userspace-visible runtime effects? If so,
> please changelog these in full detail. If not, a statement (in the
> [0/N]!) telling us this would be helpful.
>
That's fair, these are fairly small changes so I skipped the cover
letter. I'll add one with a changelog in v3.
The changes don't have any userspace-visible runtime effects.
>
> AI review might have found a few things, about half of them
> pre-existing (zram/zcomp):
>
> https://sashiko.dev/#/patchset/20260728092935.31139-1-haoqinhuang7@gmail.com
>
>
I took a look at the AI review and there are a few things I can clean
up in the next version — I'll fold those into v3 as well.
Thanks.
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
` (4 preceding siblings ...)
2026-07-29 0:34 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path Andrew Morton
@ 2026-07-29 2:20 ` Sergey Senozhatsky
2026-07-29 3:40 ` haoqin huang
2026-07-29 2:40 ` Sergey Senozhatsky
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
7 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 2:20 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/28 17:29), Haoqin Huang wrote:
> zstd_setup_params() creates global cdict and ddict stored in
> params->drv_data, shared across all per-CPU contexts. The per-CPU
> zstd_create() error path called zstd_release_params(), which freed
> those globally-shared objects. While the drv_data=NULL guard in
> zstd_release_params() prevents a double-free on the init failure
> path, this is still a layering violation: a per-CPU callback should
> only clean up its own context, not release resources owned by the
> compression lifecycle (zcomp_init / zcomp_destroy).
>
> Fix by removing zstd_release_params() from the per-CPU error path
> and replacing it with only zstd_destroy(), which properly cleans
> up the per-CPU context without touching the global params->drv_data.
>
> Fixes: 6a559ecd6e7e ("zram: add dictionary support to zstd backend")
As we agreed earlier, this patch doesn't fix any known issues per se.
Let's not send a false signal to stable folks that this patch is
fixing a bug, let's not add Fixes: where they don't belong.
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path
2026-07-29 2:20 ` Sergey Senozhatsky
@ 2026-07-29 3:40 ` haoqin huang
0 siblings, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-07-29 3:40 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Wed, Jul 29, 2026 at 10:20 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/28 17:29), Haoqin Huang wrote:
> > zstd_setup_params() creates global cdict and ddict stored in
> > params->drv_data, shared across all per-CPU contexts. The per-CPU
> > zstd_create() error path called zstd_release_params(), which freed
> > those globally-shared objects. While the drv_data=NULL guard in
> > zstd_release_params() prevents a double-free on the init failure
> > path, this is still a layering violation: a per-CPU callback should
> > only clean up its own context, not release resources owned by the
> > compression lifecycle (zcomp_init / zcomp_destroy).
> >
> > Fix by removing zstd_release_params() from the per-CPU error path
> > and replacing it with only zstd_destroy(), which properly cleans
> > up the per-CPU context without touching the global params->drv_data.
> >
> > Fixes: 6a559ecd6e7e ("zram: add dictionary support to zstd backend")
>
> As we agreed earlier, this patch doesn't fix any known issues per se.
> Let's not send a false signal to stable folks that this patch is
> fixing a bug, let's not add Fixes: where they don't belong.
Agreed, I'll drop the Fixes: tag and rename it to "zram: do not
release zstd params in per-CPU error path" in v3, which I'll send
out later.
Thanks.
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
` (5 preceding siblings ...)
2026-07-29 2:20 ` Sergey Senozhatsky
@ 2026-07-29 2:40 ` Sergey Senozhatsky
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
7 siblings, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-29 2:40 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/28 17:29), Haoqin Huang wrote:
> Subject: [PATCH v2 1/5] zram: fix early release of global cdict/ddict in per-CPU error path
Let's rename it to something like
"zram: do not release zstd params in per-CPU error path"
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation
2026-07-28 9:29 ` [PATCH v2 1/5] zram: fix early release of global cdict/ddict in " Haoqin Huang
` (6 preceding siblings ...)
2026-07-29 2:40 ` Sergey Senozhatsky
@ 2026-07-30 2:52 ` Haoqin Huang
2026-07-30 2:52 ` [PATCH v3 1/5] zram: do not release zstd params in per-CPU error path Haoqin Huang
` (5 more replies)
7 siblings, 6 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 2:52 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang
From: Haoqin Huang <haoqinhuang@tencent.com>
Patch 1 removes zstd_release_params() from the per-CPU zstd_create()
error path since per-CPU callbacks should not free global resources.
Patch 2 rejects zero-size dictionaries and prints an error on dict
load failure (currently errors are silently swallowed).
Patches 3-4 add parameter validation: per-backend level bounds
checking in zstd_setup_params() (patch 3), and a generic
zcomp_validate_params() with per-backend caps to reject unsupported
dict/level parameters before storing (patch 4).
Patch 5 resets per-priority params on algorithm change before init.
Changes since v2:
- Patch 1: removed "fix" from subject, reworded commit message
- Patch 2: dropped atomic dict swap; kernel_read_file() already
rejects empty files, so just use sz <= 0 and add pr_err
- Patch 3: dropped ZSTD_MAX_CLEVEL header move, validate in
setup_params using runtime zstd_max_clevel() instead
- Patch 4: merged validate functions into zcomp_validate_params()
with internal pr_err for specific error messages; sentinel
level_max=-1 for zstd
v2: https://lore.kernel.org/all/20260728092935.31139-1-haoqinhuang7@gmail.com/
Haoqin Huang (5):
zram: do not release zstd params in per-CPU error path
zram: reject zero-size dictionary
zram: add level validation in zstd setup_params
zram: add per-backend caps and validate parameters early
zram: reset per-priority params when changing algorithm before init
drivers/block/zram/backend_842.c | 1 +
drivers/block/zram/backend_deflate.c | 3 +++
drivers/block/zram/backend_lz4.c | 3 +++
drivers/block/zram/backend_lz4hc.c | 3 +++
drivers/block/zram/backend_lzo.c | 1 +
drivers/block/zram/backend_lzorle.c | 1 +
drivers/block/zram/backend_zstd.c | 9 ++++++++-
drivers/block/zram/zcomp.c | 27 +++++++++++++++++++++++++++
drivers/block/zram/zcomp.h | 7 +++++++
drivers/block/zram/zram_drv.c | 15 ++++++++++++++-
10 files changed, 68 insertions(+), 2 deletions(-)
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v3 1/5] zram: do not release zstd params in per-CPU error path
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
@ 2026-07-30 2:52 ` Haoqin Huang
2026-07-30 2:52 ` [PATCH v3 2/5] zram: reject zero-size dictionary Haoqin Huang
` (4 subsequent siblings)
5 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 2:52 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
zstd_setup_params() creates global cdict and ddict stored in
params->drv_data, shared across all per-CPU contexts. The per-CPU
zstd_create() error path called zstd_release_params(), which freed
those globally-shared objects. While the drv_data=NULL guard in
zstd_release_params() prevents a double-free on the init failure
path, this is still a layering violation: a per-CPU callback should
only clean up its own context, not release resources owned by the
compression lifecycle (zcomp_init / zcomp_destroy).
Remove zstd_release_params() from the per-CPU error path and call
only zstd_destroy(), which properly cleans up the per-CPU context
without touching the global params->drv_data.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_zstd.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index d00b548056dc..2584f47c9b3c 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -161,7 +161,6 @@ static int zstd_create(struct zcomp_params *params, struct zcomp_ctx *ctx)
return 0;
error:
- zstd_release_params(params);
zstd_destroy(ctx);
return -EINVAL;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v3 2/5] zram: reject zero-size dictionary
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
2026-07-30 2:52 ` [PATCH v3 1/5] zram: do not release zstd params in per-CPU error path Haoqin Huang
@ 2026-07-30 2:52 ` Haoqin Huang
2026-07-30 2:52 ` [PATCH v3 3/5] zram: add level validation in zstd setup_params Haoqin Huang
` (3 subsequent siblings)
5 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 2:52 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
kernel_read_file_from_path() already rejects empty files (i_size <= 0)
and returns -EINVAL, but the current implementation only checks for
sz < 0 without logging any information. Use sz <= 0 to cover the
zero-size case and print an error message if dictionary loading fails.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/zram_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index ace65c586072..0223fd83bbba 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1709,8 +1709,11 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
INT_MAX,
NULL,
READING_POLICY);
- if (sz < 0)
+ if (sz <= 0) {
+ pr_err("zram: failed to load dictionary %s (err=%zd)\n",
+ dict_path, sz);
return -EINVAL;
+ }
}
zram->params[prio].dict_sz = sz;
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v3 3/5] zram: add level validation in zstd setup_params
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
2026-07-30 2:52 ` [PATCH v3 1/5] zram: do not release zstd params in per-CPU error path Haoqin Huang
2026-07-30 2:52 ` [PATCH v3 2/5] zram: reject zero-size dictionary Haoqin Huang
@ 2026-07-30 2:52 ` Haoqin Huang
2026-07-30 3:10 ` Sergey Senozhatsky
2026-07-30 2:52 ` [PATCH v3 4/5] zram: add per-backend caps and validate parameters early Haoqin Huang
` (2 subsequent siblings)
5 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 2:52 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
zstd_setup_params() currently accepts any level and silently clamps
it via zstd_get_params(). Add explicit bounds checking using
zstd_max_clevel() to reject out-of-range levels early with an error
message. Since zstd_max_clevel() is a runtime function, the check
is done here rather than in the generic validation path.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_zstd.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 2584f47c9b3c..6febb366f76e 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params)
params->drv_data = zp;
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = zstd_default_clevel();
+ else if (params->level < -(int)ZSTD_TARGETLENGTH_MAX ||
+ params->level > zstd_max_clevel()) {
+ pr_err("zstd: invalid compression level %d\n", params->level);
+ goto error;
+ }
zp->cprm = zstd_get_params(params->level, PAGE_SIZE);
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v3 3/5] zram: add level validation in zstd setup_params
2026-07-30 2:52 ` [PATCH v3 3/5] zram: add level validation in zstd setup_params Haoqin Huang
@ 2026-07-30 3:10 ` Sergey Senozhatsky
2026-07-30 5:55 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-30 3:10 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/30 10:52), Haoqin Huang wrote:
[..]
> diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
> index 2584f47c9b3c..6febb366f76e 100644
> --- a/drivers/block/zram/backend_zstd.c
> +++ b/drivers/block/zram/backend_zstd.c
> @@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params)
> params->drv_data = zp;
> if (params->level == ZCOMP_PARAM_NOT_SET)
> params->level = zstd_default_clevel();
> + else if (params->level < -(int)ZSTD_TARGETLENGTH_MAX ||
I was expecting to see zstd_min_clevel() here. What is this
-(int)ZSTD_TARGETLENGTH_MAX?
> + params->level > zstd_max_clevel()) {
> + pr_err("zstd: invalid compression level %d\n", params->level);
> + goto error;
> + }
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v3 3/5] zram: add level validation in zstd setup_params
2026-07-30 3:10 ` Sergey Senozhatsky
@ 2026-07-30 5:55 ` haoqin huang
0 siblings, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-07-30 5:55 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Thu, Jul 30, 2026 at 11:10 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/30 10:52), Haoqin Huang wrote:
> [..]
> > diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
> > index 2584f47c9b3c..6febb366f76e 100644
> > --- a/drivers/block/zram/backend_zstd.c
> > +++ b/drivers/block/zram/backend_zstd.c
> > @@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params)
> > params->drv_data = zp;
> > if (params->level == ZCOMP_PARAM_NOT_SET)
> > params->level = zstd_default_clevel();
> > + else if (params->level < -(int)ZSTD_TARGETLENGTH_MAX ||
>
> I was expecting to see zstd_min_clevel() here. What is this
> -(int)ZSTD_TARGETLENGTH_MAX?
>
Ah yes, that was an oversight, it should be zstd_min_clevel().
Fixed in v4.
> > + params->level > zstd_max_clevel()) {
> > + pr_err("zstd: invalid compression level %d\n", params->level);
> > + goto error;
> > + }
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v3 4/5] zram: add per-backend caps and validate parameters early
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
` (2 preceding siblings ...)
2026-07-30 2:52 ` [PATCH v3 3/5] zram: add level validation in zstd setup_params Haoqin Huang
@ 2026-07-30 2:52 ` Haoqin Huang
2026-07-30 3:14 ` Sergey Senozhatsky
2026-07-30 2:52 ` [PATCH v3 5/5] zram: reset per-priority params when changing algorithm before init Haoqin Huang
2026-07-30 6:01 ` [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
5 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 2:52 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Dict and level parameters are silently accepted even for backends
that do not support them, e.g. "algo=lzo dict=/data/dict" succeeds
but has no effect. Add per-backend caps and zcomp_validate_params()
to reject invalid parameters with a specific error message before
storing. For zstd, set level_max to -1 as a sentinel since its
maximum level is determined at runtime by zstd_max_clevel().
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_842.c | 1 +
drivers/block/zram/backend_deflate.c | 3 +++
drivers/block/zram/backend_lz4.c | 3 +++
drivers/block/zram/backend_lz4hc.c | 3 +++
drivers/block/zram/backend_lzo.c | 1 +
drivers/block/zram/backend_lzorle.c | 1 +
drivers/block/zram/backend_zstd.c | 3 +++
drivers/block/zram/zcomp.c | 27 +++++++++++++++++++++++++++
drivers/block/zram/zcomp.h | 7 +++++++
drivers/block/zram/zram_drv.c | 7 +++++++
10 files changed, 56 insertions(+)
diff --git a/drivers/block/zram/backend_842.c b/drivers/block/zram/backend_842.c
index 10d9d5c60f53..d796ebda1fa0 100644
--- a/drivers/block/zram/backend_842.c
+++ b/drivers/block/zram/backend_842.c
@@ -57,5 +57,6 @@ const struct zcomp_ops backend_842 = {
.destroy_ctx = destroy_842,
.setup_params = setup_params_842,
.release_params = release_params_842,
+ .caps = 0,
.name = "842",
};
diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index f92a52a720d1..cedc3daad33a 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -144,5 +144,8 @@ const struct zcomp_ops backend_deflate = {
.destroy_ctx = deflate_destroy,
.setup_params = deflate_setup_params,
.release_params = deflate_release_params,
+ .caps = ZCOMP_CAP_LEVEL,
+ .level_min = Z_DEFAULT_COMPRESSION,
+ .level_max = Z_BEST_COMPRESSION,
.name = "deflate",
};
diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
index c449d511ba86..bd1e5ca4d134 100644
--- a/drivers/block/zram/backend_lz4.c
+++ b/drivers/block/zram/backend_lz4.c
@@ -146,5 +146,8 @@ const struct zcomp_ops backend_lz4 = {
.destroy_ctx = lz4_destroy,
.setup_params = lz4_setup_params,
.release_params = lz4_release_params,
+ .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL,
+ .level_min = LZ4_ACCELERATION_DEFAULT,
+ .level_max = 65535,
.name = "lz4",
};
diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
index f6a336acfe20..0e0d7c68a7d4 100644
--- a/drivers/block/zram/backend_lz4hc.c
+++ b/drivers/block/zram/backend_lz4hc.c
@@ -124,5 +124,8 @@ const struct zcomp_ops backend_lz4hc = {
.destroy_ctx = lz4hc_destroy,
.setup_params = lz4hc_setup_params,
.release_params = lz4hc_release_params,
+ .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL,
+ .level_min = LZ4HC_MIN_CLEVEL,
+ .level_max = LZ4HC_MAX_CLEVEL,
.name = "lz4hc",
};
diff --git a/drivers/block/zram/backend_lzo.c b/drivers/block/zram/backend_lzo.c
index 4c906beaae6b..965f007e2ca8 100644
--- a/drivers/block/zram/backend_lzo.c
+++ b/drivers/block/zram/backend_lzo.c
@@ -55,5 +55,6 @@ const struct zcomp_ops backend_lzo = {
.destroy_ctx = lzo_destroy,
.setup_params = lzo_setup_params,
.release_params = lzo_release_params,
+ .caps = 0,
.name = "lzo",
};
diff --git a/drivers/block/zram/backend_lzorle.c b/drivers/block/zram/backend_lzorle.c
index 10640c96cbfc..757b4598be03 100644
--- a/drivers/block/zram/backend_lzorle.c
+++ b/drivers/block/zram/backend_lzorle.c
@@ -55,5 +55,6 @@ const struct zcomp_ops backend_lzorle = {
.destroy_ctx = lzorle_destroy,
.setup_params = lzorle_setup_params,
.release_params = lzorle_release_params,
+ .caps = 0,
.name = "lzo-rle",
};
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 6febb366f76e..801ee8ee4ba6 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -217,5 +217,8 @@ const struct zcomp_ops backend_zstd = {
.destroy_ctx = zstd_destroy,
.setup_params = zstd_setup_params,
.release_params = zstd_release_params,
+ .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL,
+ .level_min = (int)-ZSTD_TARGETLENGTH_MAX,
+ .level_max = -1, /* validated by zstd_setup_params() */
.name = "zstd",
};
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 974c4691887e..149eff4590e1 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -94,6 +94,33 @@ const char *zcomp_lookup_backend_name(const char *comp)
return NULL;
}
+int zcomp_validate_params(const char *comp, s32 level, const char *dict_path)
+{
+ const struct zcomp_ops *backend = lookup_backend_ops(comp);
+
+ if (!backend)
+ return -EINVAL;
+
+ if (dict_path && !(backend->caps & ZCOMP_CAP_DICT)) {
+ pr_err("zram: %s does not support dictionary\n", comp);
+ return -EOPNOTSUPP;
+ }
+
+ if (level != ZCOMP_PARAM_NOT_SET) {
+ if (!(backend->caps & ZCOMP_CAP_LEVEL)) {
+ pr_err("zram: %s does not support level\n", comp);
+ return -EOPNOTSUPP;
+ }
+ /* level_max == -1 means validate in .setup_params() */
+ if (backend->level_max >= 0 &&
+ (level < backend->level_min || level > backend->level_max)) {
+ pr_err("zram: invalid level %d for %s\n", level, comp);
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+
/* show available compressors */
ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at)
{
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 81a0f3f6ff48..366250050d4a 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -7,6 +7,9 @@
#define ZCOMP_PARAM_NOT_SET INT_MIN
+#define ZCOMP_CAP_DICT BIT(0) /* dictionary support */
+#define ZCOMP_CAP_LEVEL BIT(1) /* adjustable compression level */
+
struct deflate_params {
s32 winbits;
};
@@ -66,6 +69,9 @@ struct zcomp_ops {
int (*setup_params)(struct zcomp_params *params);
void (*release_params)(struct zcomp_params *params);
+ unsigned int caps;
+ s32 level_min;
+ s32 level_max;
const char *name;
};
@@ -81,6 +87,7 @@ int zcomp_cpu_up_prepare(unsigned int cpu, struct hlist_node *node);
int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node);
ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at);
const char *zcomp_lookup_backend_name(const char *comp);
+int zcomp_validate_params(const char *comp, s32 level, const char *dict_path);
struct zcomp *zcomp_create(const char *alg, struct zcomp_params *params);
void zcomp_destroy(struct zcomp *comp);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0223fd83bbba..0c804bb4a701 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1797,6 +1797,13 @@ static ssize_t algorithm_params_store(struct device *dev,
return -EINVAL;
}
+ if (zram->comp_algs[prio]) {
+ ret = zcomp_validate_params(zram->comp_algs[prio], level,
+ dict_path);
+ if (ret)
+ return ret;
+ }
+
ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
return ret ? ret : len;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v3 4/5] zram: add per-backend caps and validate parameters early
2026-07-30 2:52 ` [PATCH v3 4/5] zram: add per-backend caps and validate parameters early Haoqin Huang
@ 2026-07-30 3:14 ` Sergey Senozhatsky
2026-07-30 5:57 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-30 3:14 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/30 10:52), Haoqin Huang wrote:
[..]
> +int zcomp_validate_params(const char *comp, s32 level, const char *dict_path)
> +{
> + const struct zcomp_ops *backend = lookup_backend_ops(comp);
> +
> + if (!backend)
> + return -EINVAL;
> +
> + if (dict_path && !(backend->caps & ZCOMP_CAP_DICT)) {
> + pr_err("zram: %s does not support dictionary\n", comp);
> + return -EOPNOTSUPP;
> + }
> +
> + if (level != ZCOMP_PARAM_NOT_SET) {
> + if (!(backend->caps & ZCOMP_CAP_LEVEL)) {
> + pr_err("zram: %s does not support level\n", comp);
> + return -EOPNOTSUPP;
> + }
> + /* level_max == -1 means validate in .setup_params() */
> + if (backend->level_max >= 0 &&
> + (level < backend->level_min || level > backend->level_max)) {
> + pr_err("zram: invalid level %d for %s\n", level, comp);
> + return -EINVAL;
> + }
> + }
> + return 0;
> +}
I was thinking that you'd move all params validation to backend's
.setup_params(), not just zstd, but for every backend. Sorry if
my message was not clear. Can we move all validation to backends?
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v3 4/5] zram: add per-backend caps and validate parameters early
2026-07-30 3:14 ` Sergey Senozhatsky
@ 2026-07-30 5:57 ` haoqin huang
0 siblings, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-07-30 5:57 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Thu, Jul 30, 2026 at 11:14 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/30 10:52), Haoqin Huang wrote:
> [..]
> > +int zcomp_validate_params(const char *comp, s32 level, const char *dict_path)
> > +{
> > + const struct zcomp_ops *backend = lookup_backend_ops(comp);
> > +
> > + if (!backend)
> > + return -EINVAL;
> > +
> > + if (dict_path && !(backend->caps & ZCOMP_CAP_DICT)) {
> > + pr_err("zram: %s does not support dictionary\n", comp);
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (level != ZCOMP_PARAM_NOT_SET) {
> > + if (!(backend->caps & ZCOMP_CAP_LEVEL)) {
> > + pr_err("zram: %s does not support level\n", comp);
> > + return -EOPNOTSUPP;
> > + }
> > + /* level_max == -1 means validate in .setup_params() */
> > + if (backend->level_max >= 0 &&
> > + (level < backend->level_min || level > backend->level_max)) {
> > + pr_err("zram: invalid level %d for %s\n", level, comp);
> > + return -EINVAL;
> > + }
> > + }
> > + return 0;
> > +}
>
> I was thinking that you'd move all params validation to backend's
> .setup_params(), not just zstd, but for every backend. Sorry if
> my message was not clear. Can we move all validation to backends?
Sorry, I misunderstood. I thought you meant only the zstd-level check.
Done in v4: all validation (dict and level) now lives in each backend's
.setup_params(), no caps or zcomp_validate_params() needed.
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v3 5/5] zram: reset per-priority params when changing algorithm before init
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
` (3 preceding siblings ...)
2026-07-30 2:52 ` [PATCH v3 4/5] zram: add per-backend caps and validate parameters early Haoqin Huang
@ 2026-07-30 2:52 ` Haoqin Huang
2026-07-30 6:01 ` [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
5 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 2:52 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Parameters validated against one algorithm may be invalid for another
(e.g. lz4 accepts level=65535 but zstd does not). Although algorithm
changes are blocked after disksize is set, they are allowed before
device initialization. Reset per-priority params on algorithm change
so that stale parameters do not silently carry over.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0c804bb4a701..b4585b3bc576 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1661,6 +1661,8 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
zram->comp_algs[prio] = alg;
}
+static void comp_params_reset(struct zram *zram, u32 prio);
+
static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
{
const char *alg;
@@ -1681,6 +1683,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
}
comp_algorithm_set(zram, prio, alg);
+ comp_params_reset(zram, prio);
return 0;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation
2026-07-30 2:52 ` [PATCH v3 0/5] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
` (4 preceding siblings ...)
2026-07-30 2:52 ` [PATCH v3 5/5] zram: reset per-priority params when changing algorithm before init Haoqin Huang
@ 2026-07-30 6:01 ` Haoqin Huang
2026-07-30 6:01 ` [PATCH v4 1/4] zram: do not release zstd params in per-CPU error path Haoqin Huang
` (4 more replies)
5 siblings, 5 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 6:01 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang
From: Haoqin Huang <haoqinhuang@tencent.com>
Patch 1 removes zstd_release_params() from the per-CPU zstd_create()
error path since per-CPU callbacks should not free global resources.
Patch 2 rejects zero-size dictionaries and prints an error on dict
load failure (currently errors are silently swallowed).
Patch 3 moves parameter validation into each backend's .setup_params():
dict/level support checks for backends that don't support them, and
level bounds checking for those that do (zstd uses zstd_min_clevel()/
zstd_max_clevel(), lz4hc uses LZ4HC_MIN/MAX_CLEVEL, etc.).
Patch 4 resets per-priority params on algorithm change before init.
Changes since v3:
- Patch 3-4: dropped the generic caps + zcomp_validate_params()
approach in favor of per-backend .setup_params() validation,
keeping zcomp.c free of any library headers
v3: https://lore.kernel.org/all/20260730025240.17724-1-haoqinhuang7@gmail.com/
Haoqin Huang (4):
zram: do not release zstd params in per-CPU error path
zram: reject zero-size dictionary
zram: validate parameters in each backend's setup_params
zram: reset per-priority params when changing algorithm before init
drivers/block/zram/backend_842.c | 8 ++++++++
drivers/block/zram/backend_deflate.c | 9 +++++++++
drivers/block/zram/backend_lz4.c | 5 +++++
drivers/block/zram/backend_lz4hc.c | 5 +++++
drivers/block/zram/backend_lzo.c | 8 ++++++++
drivers/block/zram/backend_lzorle.c | 8 ++++++++
drivers/block/zram/backend_zstd.c | 6 +++++-
drivers/block/zram/zram_drv.c | 8 +++++++-
8 files changed, 55 insertions(+), 2 deletions(-)
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v4 1/4] zram: do not release zstd params in per-CPU error path
2026-07-30 6:01 ` [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
@ 2026-07-30 6:01 ` Haoqin Huang
2026-07-30 6:01 ` [PATCH v4 2/4] zram: reject zero-size dictionary Haoqin Huang
` (3 subsequent siblings)
4 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 6:01 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
zstd_setup_params() creates global cdict and ddict stored in
params->drv_data, shared across all per-CPU contexts. The per-CPU
zstd_create() error path called zstd_release_params(), which freed
those globally-shared objects. While the drv_data=NULL guard in
zstd_release_params() prevents a double-free on the init failure
path, this is still a layering violation: a per-CPU callback should
only clean up its own context, not release resources owned by the
compression lifecycle (zcomp_init / zcomp_destroy).
Remove zstd_release_params() from the per-CPU error path and call
only zstd_destroy(), which properly cleans up the per-CPU context
without touching the global params->drv_data.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_zstd.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index d00b548056dc..2584f47c9b3c 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -161,7 +161,6 @@ static int zstd_create(struct zcomp_params *params, struct zcomp_ctx *ctx)
return 0;
error:
- zstd_release_params(params);
zstd_destroy(ctx);
return -EINVAL;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v4 2/4] zram: reject zero-size dictionary
2026-07-30 6:01 ` [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
2026-07-30 6:01 ` [PATCH v4 1/4] zram: do not release zstd params in per-CPU error path Haoqin Huang
@ 2026-07-30 6:01 ` Haoqin Huang
2026-07-30 7:00 ` Sergey Senozhatsky
2026-07-30 6:01 ` [PATCH v4 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
` (2 subsequent siblings)
4 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 6:01 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
kernel_read_file_from_path() already rejects empty files (i_size <= 0)
and returns -EINVAL, but the current implementation only checks for
sz < 0 without logging any information. Use sz <= 0 to cover the
zero-size case and print an error message if dictionary loading fails.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/zram_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index ace65c586072..0223fd83bbba 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1709,8 +1709,11 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
INT_MAX,
NULL,
READING_POLICY);
- if (sz < 0)
+ if (sz <= 0) {
+ pr_err("zram: failed to load dictionary %s (err=%zd)\n",
+ dict_path, sz);
return -EINVAL;
+ }
}
zram->params[prio].dict_sz = sz;
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v4 2/4] zram: reject zero-size dictionary
2026-07-30 6:01 ` [PATCH v4 2/4] zram: reject zero-size dictionary Haoqin Huang
@ 2026-07-30 7:00 ` Sergey Senozhatsky
0 siblings, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-30 7:00 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/30 14:01), Haoqin Huang wrote:
> kernel_read_file_from_path() already rejects empty files (i_size <= 0)
> and returns -EINVAL, but the current implementation only checks for
> sz < 0 without logging any information. Use sz <= 0 to cover the
> zero-size case and print an error message if dictionary loading fails.
>
> Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
> Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
> ---
> drivers/block/zram/zram_drv.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index ace65c586072..0223fd83bbba 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1709,8 +1709,11 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> INT_MAX,
> NULL,
> READING_POLICY);
> - if (sz < 0)
> + if (sz <= 0) {
> + pr_err("zram: failed to load dictionary %s (err=%zd)\n",
This leads to a double-prefixed line "zram: zram: " as zram already
defines pr_fmt().
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v4 3/4] zram: validate parameters in each backend's setup_params
2026-07-30 6:01 ` [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
2026-07-30 6:01 ` [PATCH v4 1/4] zram: do not release zstd params in per-CPU error path Haoqin Huang
2026-07-30 6:01 ` [PATCH v4 2/4] zram: reject zero-size dictionary Haoqin Huang
@ 2026-07-30 6:01 ` Haoqin Huang
2026-07-30 7:24 ` Sergey Senozhatsky
2026-07-30 7:35 ` Sergey Senozhatsky
2026-07-30 6:01 ` [PATCH v4 4/4] zram: reset per-priority params when changing algorithm before init Haoqin Huang
2026-08-03 14:12 ` [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation Haoqin Huang
4 siblings, 2 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 6:01 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Dict and level parameters are silently accepted even for backends
that do not support them. Validate these parameters in each backend's
.setup_params() to reject unsupported combinations and out-of-range
levels with a specific error message.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_842.c | 8 ++++++++
drivers/block/zram/backend_deflate.c | 9 +++++++++
drivers/block/zram/backend_lz4.c | 5 +++++
drivers/block/zram/backend_lz4hc.c | 5 +++++
drivers/block/zram/backend_lzo.c | 8 ++++++++
drivers/block/zram/backend_lzorle.c | 8 ++++++++
drivers/block/zram/backend_zstd.c | 5 +++++
7 files changed, 48 insertions(+)
diff --git a/drivers/block/zram/backend_842.c b/drivers/block/zram/backend_842.c
index 10d9d5c60f53..91a22313fe82 100644
--- a/drivers/block/zram/backend_842.c
+++ b/drivers/block/zram/backend_842.c
@@ -13,6 +13,14 @@ static void release_params_842(struct zcomp_params *params)
static int setup_params_842(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("842: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("842: compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index f92a52a720d1..1a2be462bc67 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -22,8 +22,17 @@ static void deflate_release_params(struct zcomp_params *params)
static int deflate_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("deflate: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = Z_DEFAULT_COMPRESSION;
+ else if (params->level < Z_DEFAULT_COMPRESSION ||
+ params->level > Z_BEST_COMPRESSION) {
+ pr_err("deflate: invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET)
params->deflate.winbits = DEFLATE_DEF_WINBITS;
diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
index c449d511ba86..6577b484941a 100644
--- a/drivers/block/zram/backend_lz4.c
+++ b/drivers/block/zram/backend_lz4.c
@@ -30,6 +30,11 @@ static int lz4_setup_params(struct zcomp_params *params)
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = LZ4_ACCELERATION_DEFAULT;
+ else if (params->level < LZ4_ACCELERATION_DEFAULT ||
+ params->level > U16_MAX) {
+ pr_err("lz4: invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
if (!params->dict || !params->dict_sz)
return 0;
diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
index f6a336acfe20..5d551d165213 100644
--- a/drivers/block/zram/backend_lz4hc.c
+++ b/drivers/block/zram/backend_lz4hc.c
@@ -20,6 +20,11 @@ static int lz4hc_setup_params(struct zcomp_params *params)
{
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = LZ4HC_DEFAULT_CLEVEL;
+ else if (params->level < LZ4HC_MIN_CLEVEL ||
+ params->level > LZ4HC_MAX_CLEVEL) {
+ pr_err("lz4hc: invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_lzo.c b/drivers/block/zram/backend_lzo.c
index 4c906beaae6b..cdbb89aaa0de 100644
--- a/drivers/block/zram/backend_lzo.c
+++ b/drivers/block/zram/backend_lzo.c
@@ -12,6 +12,14 @@ static void lzo_release_params(struct zcomp_params *params)
static int lzo_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("lzo: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("lzo: compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_lzorle.c b/drivers/block/zram/backend_lzorle.c
index 10640c96cbfc..58f7ddf5a55f 100644
--- a/drivers/block/zram/backend_lzorle.c
+++ b/drivers/block/zram/backend_lzorle.c
@@ -12,6 +12,14 @@ static void lzorle_release_params(struct zcomp_params *params)
static int lzorle_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("lzo-rle: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("lzo-rle: compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 2584f47c9b3c..4d19d6089f13 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params)
params->drv_data = zp;
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = zstd_default_clevel();
+ else if (params->level < zstd_min_clevel() ||
+ params->level > zstd_max_clevel()) {
+ pr_err("zstd: invalid compression level %d\n", params->level);
+ goto error;
+ }
zp->cprm = zstd_get_params(params->level, PAGE_SIZE);
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v4 3/4] zram: validate parameters in each backend's setup_params
2026-07-30 6:01 ` [PATCH v4 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
@ 2026-07-30 7:24 ` Sergey Senozhatsky
2026-08-03 12:20 ` haoqin huang
2026-07-30 7:35 ` Sergey Senozhatsky
1 sibling, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-30 7:24 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/30 14:01), Haoqin Huang wrote:
[..]
> static int deflate_setup_params(struct zcomp_params *params)
> {
> + if (params->dict_sz) {
> + pr_err("deflate: dictionary is not supported\n");
> + return -EOPNOTSUPP;
> + }
> if (params->level == ZCOMP_PARAM_NOT_SET)
> params->level = Z_DEFAULT_COMPRESSION;
> + else if (params->level < Z_DEFAULT_COMPRESSION ||
> + params->level > Z_BEST_COMPRESSION) {
> + pr_err("deflate: invalid compression level %d\n", params->level);
> + return -EINVAL;
> + }
> if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET)
> params->deflate.winbits = DEFLATE_DEF_WINBITS;
This will conflict with winbits validation change that we landed
yesterday. Can I trouble you with a rebase request (either on top
of linux-next, when winbits patch hits it, or on top of Andrew's mm
tree)?
[..]
> diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
> index f6a336acfe20..5d551d165213 100644
> --- a/drivers/block/zram/backend_lz4hc.c
> +++ b/drivers/block/zram/backend_lz4hc.c
> @@ -20,6 +20,11 @@ static int lz4hc_setup_params(struct zcomp_params *params)
> {
> if (params->level == ZCOMP_PARAM_NOT_SET)
> params->level = LZ4HC_DEFAULT_CLEVEL;
> + else if (params->level < LZ4HC_MIN_CLEVEL ||
> + params->level > LZ4HC_MAX_CLEVEL) {
> + pr_err("lz4hc: invalid compression level %d\n", params->level);
> + return -EINVAL;
> + }
So... lib/lz4/lz4hc_compress.c supports levels 1 and 2. However,
LZ4HC_MIN_CLEVEL is set to 3, but clearly the compression library
supports levels lower than LZ4HC_MIN_CLEVEL. In fact, LZ4HC_MIN_CLEVEL
is never used in the lz4 code. Maybe here we need to just hardcode
"< 1" and put a comment:
} else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
/*
* Not LZ4HC_MIN_CLEVEL: that constant is advisory, and
* LZ4HC_compress_generic() only clamps levels below 1.
* Levels 1-2 are valid.
*/
pr_err("lz4hc: invalid compression level %d\n", params->level);
return -EINVAL;
}
[..]
> diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
> index 2584f47c9b3c..4d19d6089f13 100644
> --- a/drivers/block/zram/backend_zstd.c
> +++ b/drivers/block/zram/backend_zstd.c
> @@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params)
> params->drv_data = zp;
> if (params->level == ZCOMP_PARAM_NOT_SET)
> params->level = zstd_default_clevel();
> + else if (params->level < zstd_min_clevel() ||
> + params->level > zstd_max_clevel()) {
> + pr_err("zstd: invalid compression level %d\n", params->level);
> + goto error;
Should we also remove "zstd_release_params(params);" from here?
Same reason as with zstd_create().
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v4 3/4] zram: validate parameters in each backend's setup_params
2026-07-30 7:24 ` Sergey Senozhatsky
@ 2026-08-03 12:20 ` haoqin huang
2026-08-04 4:42 ` Sergey Senozhatsky
0 siblings, 1 reply; 81+ messages in thread
From: haoqin huang @ 2026-08-03 12:20 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Thu, Jul 30, 2026 at 3:24 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/07/30 14:01), Haoqin Huang wrote:
> [..]
> > static int deflate_setup_params(struct zcomp_params *params)
> > {
> > + if (params->dict_sz) {
> > + pr_err("deflate: dictionary is not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > if (params->level == ZCOMP_PARAM_NOT_SET)
> > params->level = Z_DEFAULT_COMPRESSION;
> > + else if (params->level < Z_DEFAULT_COMPRESSION ||
> > + params->level > Z_BEST_COMPRESSION) {
> > + pr_err("deflate: invalid compression level %d\n", params->level);
> > + return -EINVAL;
> > + }
> > if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET)
> > params->deflate.winbits = DEFLATE_DEF_WINBITS;
>
> This will conflict with winbits validation change that we landed
> yesterday. Can I trouble you with a rebase request (either on top
> of linux-next, when winbits patch hits it, or on top of Andrew's mm
> tree)?
>
Sorry for the delayed response, I'll rebase on linux-next for v5.
> [..]
> > diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
> > index f6a336acfe20..5d551d165213 100644
> > --- a/drivers/block/zram/backend_lz4hc.c
> > +++ b/drivers/block/zram/backend_lz4hc.c
> > @@ -20,6 +20,11 @@ static int lz4hc_setup_params(struct zcomp_params *params)
> > {
> > if (params->level == ZCOMP_PARAM_NOT_SET)
> > params->level = LZ4HC_DEFAULT_CLEVEL;
> > + else if (params->level < LZ4HC_MIN_CLEVEL ||
> > + params->level > LZ4HC_MAX_CLEVEL) {
> > + pr_err("lz4hc: invalid compression level %d\n", params->level);
> > + return -EINVAL;
> > + }
>
> So... lib/lz4/lz4hc_compress.c supports levels 1 and 2. However,
> LZ4HC_MIN_CLEVEL is set to 3, but clearly the compression library
> supports levels lower than LZ4HC_MIN_CLEVEL. In fact, LZ4HC_MIN_CLEVEL
> is never used in the lz4 code. Maybe here we need to just hardcode
> "< 1" and put a comment:
>
My bad, completely missed that LZ4HC_MIN_CLEVEL is advisory and the
library actually accepts 1-2. Will hardcode < 1 in v5.
Btw, would it make sense to fix LZ4HC_MIN_CLEVEL to 1 in the lz4
header as a separate cleanup? It seems misleading as-is.
> } else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
> /*
> * Not LZ4HC_MIN_CLEVEL: that constant is advisory, and
> * LZ4HC_compress_generic() only clamps levels below 1.
> * Levels 1-2 are valid.
> */
> pr_err("lz4hc: invalid compression level %d\n", params->level);
> return -EINVAL;
> }
>
> [..]
> > diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
> > index 2584f47c9b3c..4d19d6089f13 100644
> > --- a/drivers/block/zram/backend_zstd.c
> > +++ b/drivers/block/zram/backend_zstd.c
> > @@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params)
> > params->drv_data = zp;
> > if (params->level == ZCOMP_PARAM_NOT_SET)
> > params->level = zstd_default_clevel();
> > + else if (params->level < zstd_min_clevel() ||
> > + params->level > zstd_max_clevel()) {
> > + pr_err("zstd: invalid compression level %d\n", params->level);
> > + goto error;
>
> Should we also remove "zstd_release_params(params);" from here?
> Same reason as with zstd_create().
Right, same reasoning as patch 1, zstd_setup_params() shouldn't do
its own teardown since zcomp_init() already calls release_params() on
failure. Will merge this into patch 1 in v5.
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v4 3/4] zram: validate parameters in each backend's setup_params
2026-08-03 12:20 ` haoqin huang
@ 2026-08-04 4:42 ` Sergey Senozhatsky
2026-08-05 1:26 ` Jaegeuk Kim
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 4:42 UTC (permalink / raw)
To: haoqin huang
Cc: Sergey Senozhatsky, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang, Jaegeuk Kim, Chao Yu,
linux-f2fs-devel
On (26/08/03 20:20), haoqin huang wrote:
> On Thu, Jul 30, 2026 at 3:24 PM Sergey Senozhatsky
> <senozhatsky@chromium.org> wrote:
> > > diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
> > > index f6a336acfe20..5d551d165213 100644
> > > --- a/drivers/block/zram/backend_lz4hc.c
> > > +++ b/drivers/block/zram/backend_lz4hc.c
> > > @@ -20,6 +20,11 @@ static int lz4hc_setup_params(struct zcomp_params *params)
> > > {
> > > if (params->level == ZCOMP_PARAM_NOT_SET)
> > > params->level = LZ4HC_DEFAULT_CLEVEL;
> > > + else if (params->level < LZ4HC_MIN_CLEVEL ||
> > > + params->level > LZ4HC_MAX_CLEVEL) {
> > > + pr_err("lz4hc: invalid compression level %d\n", params->level);
> > > + return -EINVAL;
> > > + }
> >
> > So... lib/lz4/lz4hc_compress.c supports levels 1 and 2. However,
> > LZ4HC_MIN_CLEVEL is set to 3, but clearly the compression library
> > supports levels lower than LZ4HC_MIN_CLEVEL. In fact, LZ4HC_MIN_CLEVEL
> > is never used in the lz4 code. Maybe here we need to just hardcode
> > "< 1" and put a comment:
> >
>
> My bad, completely missed that LZ4HC_MIN_CLEVEL is advisory and the
> library actually accepts 1-2. Will hardcode < 1 in v5.
No worries, that LZ4HC_MIN_CLEVEL thing is difficult to spot.
> Btw, would it make sense to fix LZ4HC_MIN_CLEVEL to 1 in the lz4
> header as a separate cleanup? It seems misleading as-is.
I'm afraid we cannot do that. f2fs uses LZ4HC_MIN_CLEVEL, I assume
compression level is stored per-inode? So if we change LZ4HC_MIN_CLEVEL
then newer f2fs will start accepting compression levels that older kernels
don't support. Cc-ed Jaegeuk and Chao just for visibility.
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v4 3/4] zram: validate parameters in each backend's setup_params
2026-08-04 4:42 ` Sergey Senozhatsky
@ 2026-08-05 1:26 ` Jaegeuk Kim
0 siblings, 0 replies; 81+ messages in thread
From: Jaegeuk Kim @ 2026-08-05 1:26 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: haoqin huang, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang, Chao Yu, linux-f2fs-devel
On 08/04, Sergey Senozhatsky wrote:
> On (26/08/03 20:20), haoqin huang wrote:
> > On Thu, Jul 30, 2026 at 3:24 PM Sergey Senozhatsky
> > <senozhatsky@chromium.org> wrote:
> > > > diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
> > > > index f6a336acfe20..5d551d165213 100644
> > > > --- a/drivers/block/zram/backend_lz4hc.c
> > > > +++ b/drivers/block/zram/backend_lz4hc.c
> > > > @@ -20,6 +20,11 @@ static int lz4hc_setup_params(struct zcomp_params *params)
> > > > {
> > > > if (params->level == ZCOMP_PARAM_NOT_SET)
> > > > params->level = LZ4HC_DEFAULT_CLEVEL;
> > > > + else if (params->level < LZ4HC_MIN_CLEVEL ||
> > > > + params->level > LZ4HC_MAX_CLEVEL) {
> > > > + pr_err("lz4hc: invalid compression level %d\n", params->level);
> > > > + return -EINVAL;
> > > > + }
> > >
> > > So... lib/lz4/lz4hc_compress.c supports levels 1 and 2. However,
> > > LZ4HC_MIN_CLEVEL is set to 3, but clearly the compression library
> > > supports levels lower than LZ4HC_MIN_CLEVEL. In fact, LZ4HC_MIN_CLEVEL
> > > is never used in the lz4 code. Maybe here we need to just hardcode
> > > "< 1" and put a comment:
> > >
> >
> > My bad, completely missed that LZ4HC_MIN_CLEVEL is advisory and the
> > library actually accepts 1-2. Will hardcode < 1 in v5.
>
> No worries, that LZ4HC_MIN_CLEVEL thing is difficult to spot.
>
> > Btw, would it make sense to fix LZ4HC_MIN_CLEVEL to 1 in the lz4
> > header as a separate cleanup? It seems misleading as-is.
>
> I'm afraid we cannot do that. f2fs uses LZ4HC_MIN_CLEVEL, I assume
> compression level is stored per-inode? So if we change LZ4HC_MIN_CLEVEL
> then newer f2fs will start accepting compression levels that older kernels
> don't support. Cc-ed Jaegeuk and Chao just for visibility.
Yeah, since we have
239 clevel = le16_to_cpu(ri->i_compress_flag) >>
240 COMPRESS_LEVEL_OFFSET;
255 #ifdef CONFIG_F2FS_FS_LZ4
256 #ifdef CONFIG_F2FS_FS_LZ4HC
257 if (clevel &&
258 (clevel < LZ4HC_MIN_CLEVEL || clevel > LZ4HC_MAX_CLEVEL))
259 goto err_level;
260 #else
261 if (clevel)
262 goto err_level;
263 #endif
264 #endif
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v4 3/4] zram: validate parameters in each backend's setup_params
2026-07-30 6:01 ` [PATCH v4 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
2026-07-30 7:24 ` Sergey Senozhatsky
@ 2026-07-30 7:35 ` Sergey Senozhatsky
1 sibling, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-30 7:35 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/30 14:01), Haoqin Huang wrote:
> --- a/drivers/block/zram/backend_lz4.c
> +++ b/drivers/block/zram/backend_lz4.c
> @@ -30,6 +30,11 @@ static int lz4_setup_params(struct zcomp_params *params)
>
> if (params->level == ZCOMP_PARAM_NOT_SET)
> params->level = LZ4_ACCELERATION_DEFAULT;
> + else if (params->level < LZ4_ACCELERATION_DEFAULT ||
> + params->level > U16_MAX) {
Maybe there should be no upper bound check? It kind of seems
to me that lz4 doesn't define any clear upper bound. Or am I
missing something?
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v4 4/4] zram: reset per-priority params when changing algorithm before init
2026-07-30 6:01 ` [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
` (2 preceding siblings ...)
2026-07-30 6:01 ` [PATCH v4 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
@ 2026-07-30 6:01 ` Haoqin Huang
2026-07-30 7:28 ` Sergey Senozhatsky
2026-08-03 14:12 ` [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation Haoqin Huang
4 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-07-30 6:01 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Parameters validated against one algorithm may be invalid for another
(e.g. lz4 accepts level=65535 but zstd does not). Although algorithm
changes are blocked after disksize is set, they are allowed before
device initialization. Reset per-priority params on algorithm change
so that stale parameters do not silently carry over.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0223fd83bbba..814f0af4d77d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1661,6 +1661,8 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
zram->comp_algs[prio] = alg;
}
+static void comp_params_reset(struct zram *zram, u32 prio);
+
static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
{
const char *alg;
@@ -1681,6 +1683,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
}
comp_algorithm_set(zram, prio, alg);
+ comp_params_reset(zram, prio);
return 0;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v4 4/4] zram: reset per-priority params when changing algorithm before init
2026-07-30 6:01 ` [PATCH v4 4/4] zram: reset per-priority params when changing algorithm before init Haoqin Huang
@ 2026-07-30 7:28 ` Sergey Senozhatsky
0 siblings, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-07-30 7:28 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/07/30 14:01), Haoqin Huang wrote:
[..]
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 0223fd83bbba..814f0af4d77d 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1661,6 +1661,8 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
> zram->comp_algs[prio] = alg;
> }
>
> +static void comp_params_reset(struct zram *zram, u32 prio);
Or maybe just move that function up. I guess that's better than
forward declarations.
> static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
> {
> const char *alg;
> @@ -1681,6 +1683,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
> }
>
> comp_algorithm_set(zram, prio, alg);
> + comp_params_reset(zram, prio);
> return 0;
> }
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation
2026-07-30 6:01 ` [PATCH v4 0/4] zram: fix zstd per-CPU error path and add parameter validation Haoqin Huang
` (3 preceding siblings ...)
2026-07-30 6:01 ` [PATCH v4 4/4] zram: reset per-priority params when changing algorithm before init Haoqin Huang
@ 2026-08-03 14:12 ` Haoqin Huang
2026-08-03 14:12 ` [PATCH v5 1/4] zram: do not release zstd global params from error paths Haoqin Huang
` (4 more replies)
4 siblings, 5 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-03 14:12 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang
From: Haoqin Huang <haoqinhuang@tencent.com>
Patch 1 removes zstd_release_params() from both zstd_create() and
zstd_setup_params() error paths, the former is a layering violation
in a per-CPU callback, the latter is redundant as zcomp_init() already
calls release_params() on setup failure.
Patch 2 rejects zero-size dictionaries and prints an error on dict
load failure (currently errors are silently swallowed).
Patch 3 validates dict and level parameters in each backend's
.setup_params(), rejecting unsupported combinations and out-of-range
levels.
Patch 4 resets per-priority params on algorithm change before init.
Changes since v4:
- Patch 1: merged zstd_setup_params() zstd_release_params() removal;
reworded commit message
- Patch 2: removed "zram:" prefix from pr_err (pr_fmt already adds
it); reworded commit message
- Patch 3: dropped lz4 U16_MAX upper bound (the library has no
limit); changed lz4hc lower bound from LZ4HC_MIN_CLEVEL to < 1
(the library supports levels 1-2); rebased on tree with deflate
winbits validation
- Patch 4: moved comp_params_reset() up instead of using a forward
declaration
v4: https://lore.kernel.org/all/20260730060133.80233-1-haoqinhuang7@gmail.com/
Haoqin Huang (4):
zram: do not release zstd global params from error paths
zram: reject zero-size dictionary
zram: validate parameters in each backend's setup_params
zram: reset per-priority params when changing algorithm before init
drivers/block/zram/backend_842.c | 8 ++++++++
drivers/block/zram/backend_deflate.c | 11 +++++++++++
drivers/block/zram/backend_lz4.c | 4 ++++
drivers/block/zram/backend_lz4hc.c | 4 ++++
drivers/block/zram/backend_lzo.c | 8 ++++++++
drivers/block/zram/backend_lzorle.c | 8 ++++++++
drivers/block/zram/backend_zstd.c | 7 +++++--
drivers/block/zram/zram_drv.c | 29 ++++++++++++++++------------
8 files changed, 65 insertions(+), 14 deletions(-)
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v5 1/4] zram: do not release zstd global params from error paths
2026-08-03 14:12 ` [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation Haoqin Huang
@ 2026-08-03 14:12 ` Haoqin Huang
2026-08-03 14:12 ` [PATCH v5 2/4] zram: reject zero-size dictionary Haoqin Huang
` (3 subsequent siblings)
4 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-03 14:12 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
zstd_setup_params() creates global cdict and ddict stored in
params->drv_data, shared across all per-CPU contexts. The per-CPU
zstd_create() error path called zstd_release_params(), which freed
those globally-shared objects. This is a layering violation: a
per-CPU callback should only clean up its own context, not release
resources owned by the compression lifecycle.
zstd_setup_params() called zstd_release_params() on its own error
path as well, but zcomp_init() already calls release_params() when
setup fails, so this is redundant.
Remove zstd_release_params() from both error paths.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_zstd.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index d00b548056dc..5fabc3e7e975 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -85,7 +85,6 @@ static int zstd_setup_params(struct zcomp_params *params)
return 0;
error:
- zstd_release_params(params);
return -EINVAL;
}
@@ -161,7 +160,6 @@ static int zstd_create(struct zcomp_params *params, struct zcomp_ctx *ctx)
return 0;
error:
- zstd_release_params(params);
zstd_destroy(ctx);
return -EINVAL;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v5 2/4] zram: reject zero-size dictionary
2026-08-03 14:12 ` [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation Haoqin Huang
2026-08-03 14:12 ` [PATCH v5 1/4] zram: do not release zstd global params from error paths Haoqin Huang
@ 2026-08-03 14:12 ` Haoqin Huang
2026-08-04 5:32 ` Sergey Senozhatsky
2026-08-03 14:12 ` [PATCH v5 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
` (2 subsequent siblings)
4 siblings, 1 reply; 81+ messages in thread
From: Haoqin Huang @ 2026-08-03 14:12 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
kernel_read_file_from_path() already rejects empty files (i_size <= 0)
and returns -EINVAL, but the current implementation only checks for
sz < 0 without logging any information. Use sz <= 0 to cover the
zero-size case and print an error message if dictionary loading fails.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/zram_drv.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cfa98846ac48..68e60c9eb8b3 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1700,8 +1700,12 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
INT_MAX,
NULL,
READING_POLICY);
- if (sz < 0)
+ if (sz <= 0) {
+ pr_err("failed to load dictionary %s (err=%zd)\n",
+ dict_path, sz);
+
return -EINVAL;
+ }
}
zram->params[prio].dict_sz = sz;
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v5 2/4] zram: reject zero-size dictionary
2026-08-03 14:12 ` [PATCH v5 2/4] zram: reject zero-size dictionary Haoqin Huang
@ 2026-08-04 5:32 ` Sergey Senozhatsky
2026-08-04 7:15 ` haoqin huang
0 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 5:32 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/08/03 22:12), Haoqin Huang wrote:
[..]
> @@ -1700,8 +1700,12 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> INT_MAX,
> NULL,
> READING_POLICY);
> - if (sz < 0)
> + if (sz <= 0) {
> + pr_err("failed to load dictionary %s (err=%zd)\n",
> + dict_path, sz);
So for empty file this will read
"failed to load dictionary foo-bar (err=0)"
which might be confusing. I wonder if we want to separate these two:
if (sz < 0) {
pr_err("failed to load dictionary %s (err=%zd)\n",
dict_path, sz);
return sz;
}
if (sz == 0) {
pr_err("failed to load dictionary %s (empty file)\n",
dict_path);
return -EINVAL;
}
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v5 2/4] zram: reject zero-size dictionary
2026-08-04 5:32 ` Sergey Senozhatsky
@ 2026-08-04 7:15 ` haoqin huang
0 siblings, 0 replies; 81+ messages in thread
From: haoqin huang @ 2026-08-04 7:15 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Tue, Aug 4, 2026 at 1:32 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/08/03 22:12), Haoqin Huang wrote:
> [..]
> > @@ -1700,8 +1700,12 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
> > INT_MAX,
> > NULL,
> > READING_POLICY);
> > - if (sz < 0)
> > + if (sz <= 0) {
> > + pr_err("failed to load dictionary %s (err=%zd)\n",
> > + dict_path, sz);
>
> So for empty file this will read
>
> "failed to load dictionary foo-bar (err=0)"
>
> which might be confusing. I wonder if we want to separate these two:
>
> if (sz < 0) {
> pr_err("failed to load dictionary %s (err=%zd)\n",
> dict_path, sz);
> return sz;
> }
> if (sz == 0) {
> pr_err("failed to load dictionary %s (empty file)\n",
> dict_path);
> return -EINVAL;
> }
Good idea, much clearer. I will split them in v6. Thanks.
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v5 3/4] zram: validate parameters in each backend's setup_params
2026-08-03 14:12 ` [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation Haoqin Huang
2026-08-03 14:12 ` [PATCH v5 1/4] zram: do not release zstd global params from error paths Haoqin Huang
2026-08-03 14:12 ` [PATCH v5 2/4] zram: reject zero-size dictionary Haoqin Huang
@ 2026-08-03 14:12 ` Haoqin Huang
2026-08-04 5:36 ` Sergey Senozhatsky
2026-08-04 5:48 ` Sergey Senozhatsky
2026-08-03 14:12 ` [PATCH v5 4/4] zram: reset per-priority params when changing algorithm before init Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
4 siblings, 2 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-03 14:12 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Dict and level parameters are silently accepted even for backends
that do not support them. Validate these parameters in each backend's
.setup_params() to reject unsupported combinations and out-of-range
levels with a specific error message.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_842.c | 8 ++++++++
drivers/block/zram/backend_deflate.c | 11 +++++++++++
drivers/block/zram/backend_lz4.c | 4 ++++
drivers/block/zram/backend_lz4hc.c | 4 ++++
drivers/block/zram/backend_lzo.c | 8 ++++++++
drivers/block/zram/backend_lzorle.c | 8 ++++++++
drivers/block/zram/backend_zstd.c | 5 +++++
7 files changed, 48 insertions(+)
diff --git a/drivers/block/zram/backend_842.c b/drivers/block/zram/backend_842.c
index 10d9d5c60f53..91a22313fe82 100644
--- a/drivers/block/zram/backend_842.c
+++ b/drivers/block/zram/backend_842.c
@@ -13,6 +13,14 @@ static void release_params_842(struct zcomp_params *params)
static int setup_params_842(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("842: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("842: compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index b3f7d08b49d9..b2074c44b2d0 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -22,8 +22,19 @@ static void deflate_release_params(struct zcomp_params *params)
static int deflate_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("deflate: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = Z_DEFAULT_COMPRESSION;
+ else if (params->level < Z_DEFAULT_COMPRESSION ||
+ params->level > Z_BEST_COMPRESSION) {
+ pr_err("deflate: invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
+
if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET) {
params->deflate.winbits = DEFLATE_DEF_WINBITS;
} else {
diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
index c449d511ba86..f4f5111220c7 100644
--- a/drivers/block/zram/backend_lz4.c
+++ b/drivers/block/zram/backend_lz4.c
@@ -30,6 +30,10 @@ static int lz4_setup_params(struct zcomp_params *params)
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = LZ4_ACCELERATION_DEFAULT;
+ else if (params->level < LZ4_ACCELERATION_DEFAULT) {
+ pr_err("lz4: invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
if (!params->dict || !params->dict_sz)
return 0;
diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
index f6a336acfe20..d7cd97b898bd 100644
--- a/drivers/block/zram/backend_lz4hc.c
+++ b/drivers/block/zram/backend_lz4hc.c
@@ -20,6 +20,10 @@ static int lz4hc_setup_params(struct zcomp_params *params)
{
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = LZ4HC_DEFAULT_CLEVEL;
+ else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
+ pr_err("lz4hc: invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_lzo.c b/drivers/block/zram/backend_lzo.c
index 4c906beaae6b..cdbb89aaa0de 100644
--- a/drivers/block/zram/backend_lzo.c
+++ b/drivers/block/zram/backend_lzo.c
@@ -12,6 +12,14 @@ static void lzo_release_params(struct zcomp_params *params)
static int lzo_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("lzo: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("lzo: compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_lzorle.c b/drivers/block/zram/backend_lzorle.c
index 10640c96cbfc..58f7ddf5a55f 100644
--- a/drivers/block/zram/backend_lzorle.c
+++ b/drivers/block/zram/backend_lzorle.c
@@ -12,6 +12,14 @@ static void lzorle_release_params(struct zcomp_params *params)
static int lzorle_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("lzo-rle: dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("lzo-rle: compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 5fabc3e7e975..e071fa584f6b 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params)
params->drv_data = zp;
if (params->level == ZCOMP_PARAM_NOT_SET)
params->level = zstd_default_clevel();
+ else if (params->level < zstd_min_clevel() ||
+ params->level > zstd_max_clevel()) {
+ pr_err("zstd: invalid compression level %d\n", params->level);
+ goto error;
+ }
zp->cprm = zstd_get_params(params->level, PAGE_SIZE);
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v5 3/4] zram: validate parameters in each backend's setup_params
2026-08-03 14:12 ` [PATCH v5 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
@ 2026-08-04 5:36 ` Sergey Senozhatsky
2026-08-04 7:02 ` haoqin huang
2026-08-04 5:48 ` Sergey Senozhatsky
1 sibling, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 5:36 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/08/03 22:12), Haoqin Huang wrote:
> static int deflate_setup_params(struct zcomp_params *params)
> {
> + if (params->dict_sz) {
> + pr_err("deflate: dictionary is not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> if (params->level == ZCOMP_PARAM_NOT_SET)
> params->level = Z_DEFAULT_COMPRESSION;
If we want to be pedantic, then {} should also be added to the "if"
in this case. And in other similar cases.
> + else if (params->level < Z_DEFAULT_COMPRESSION ||
> + params->level > Z_BEST_COMPRESSION) {
> + pr_err("deflate: invalid compression level %d\n", params->level);
> + return -EINVAL;
> + }
> +
[..]
> + else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
> + pr_err("lz4hc: invalid compression level %d\n", params->level);
> + return -EINVAL;
> + }
Let's add a small comment justifying/explaining that "1" constant?
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v5 3/4] zram: validate parameters in each backend's setup_params
2026-08-04 5:36 ` Sergey Senozhatsky
@ 2026-08-04 7:02 ` haoqin huang
2026-08-04 8:30 ` Sergey Senozhatsky
0 siblings, 1 reply; 81+ messages in thread
From: haoqin huang @ 2026-08-04 7:02 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Tue, Aug 4, 2026 at 1:37 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/08/03 22:12), Haoqin Huang wrote:
> > static int deflate_setup_params(struct zcomp_params *params)
> > {
> > + if (params->dict_sz) {
> > + pr_err("deflate: dictionary is not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > if (params->level == ZCOMP_PARAM_NOT_SET)
> > params->level = Z_DEFAULT_COMPRESSION;
>
> If we want to be pedantic, then {} should also be added to the "if"
> in this case. And in other similar cases.
>
Okay, thanks for the reminder. I will add {} to the if-branch for consistency.
> > + else if (params->level < Z_DEFAULT_COMPRESSION ||
> > + params->level > Z_BEST_COMPRESSION) {
> > + pr_err("deflate: invalid compression level %d\n", params->level);
> > + return -EINVAL;
> > + }
> > +
>
> [..]
> > + else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
> > + pr_err("lz4hc: invalid compression level %d\n", params->level);
> > + return -EINVAL;
> > + }
>
> Let's add a small comment justifying/explaining that "1" constant?
For the lz4hc comment, how about:
/*
* LZ4HC_compress_generic() clamps levels below 1 to
* LZ4HC_DEFAULT_CLEVEL, so < 1 is the real lower bound.
*/
Does that look reasonable?
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v5 3/4] zram: validate parameters in each backend's setup_params
2026-08-04 7:02 ` haoqin huang
@ 2026-08-04 8:30 ` Sergey Senozhatsky
0 siblings, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 8:30 UTC (permalink / raw)
To: haoqin huang
Cc: Sergey Senozhatsky, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/08/04 15:02), haoqin huang wrote:
> On Tue, Aug 4, 2026 at 1:37 PM Sergey Senozhatsky
> <senozhatsky@chromium.org> wrote:
> >
> > On (26/08/03 22:12), Haoqin Huang wrote:
> > > static int deflate_setup_params(struct zcomp_params *params)
> > > {
> > > + if (params->dict_sz) {
> > > + pr_err("deflate: dictionary is not supported\n");
> > > + return -EOPNOTSUPP;
> > > + }
> > > +
> > > if (params->level == ZCOMP_PARAM_NOT_SET)
> > > params->level = Z_DEFAULT_COMPRESSION;
> >
> > If we want to be pedantic, then {} should also be added to the "if"
> > in this case. And in other similar cases.
> >
>
> Okay, thanks for the reminder. I will add {} to the if-branch for consistency.
Thanks.
> > > + else if (params->level < Z_DEFAULT_COMPRESSION ||
> > > + params->level > Z_BEST_COMPRESSION) {
> > > + pr_err("deflate: invalid compression level %d\n", params->level);
> > > + return -EINVAL;
> > > + }
> > > +
> >
> > [..]
> > > + else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
> > > + pr_err("lz4hc: invalid compression level %d\n", params->level);
> > > + return -EINVAL;
> > > + }
> >
> > Let's add a small comment justifying/explaining that "1" constant?
>
> For the lz4hc comment, how about:
> /*
> * LZ4HC_compress_generic() clamps levels below 1 to
> * LZ4HC_DEFAULT_CLEVEL, so < 1 is the real lower bound.
> */
> Does that look reasonable?
I suppose we want to document the fact that LZ4HC_MIN_CLEVEL (which
would be naturally expected here) is set to 3 and cuts off levels 1
and 2 which are perfectly valid. Maybe just say something like that,
for simplicity.
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v5 3/4] zram: validate parameters in each backend's setup_params
2026-08-03 14:12 ` [PATCH v5 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
2026-08-04 5:36 ` Sergey Senozhatsky
@ 2026-08-04 5:48 ` Sergey Senozhatsky
2026-08-04 7:13 ` haoqin huang
1 sibling, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 5:48 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/08/03 22:12), Haoqin Huang wrote:
[..]
> static int setup_params_842(struct zcomp_params *params)
> {
> + if (params->dict_sz) {
> + pr_err("842: dictionary is not supported\n");
> + return -EOPNOTSUPP;
> + }
> + if (params->level != ZCOMP_PARAM_NOT_SET) {
> + pr_err("842: compression level is not supported\n");
> + return -EOPNOTSUPP;
> + }
> return 0;
> }
[..]
> static int deflate_setup_params(struct zcomp_params *params)
> {
> + if (params->dict_sz) {
> + pr_err("deflate: dictionary is not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> if (params->level == ZCOMP_PARAM_NOT_SET)
> params->level = Z_DEFAULT_COMPRESSION;
> + else if (params->level < Z_DEFAULT_COMPRESSION ||
> + params->level > Z_BEST_COMPRESSION) {
> + pr_err("deflate: invalid compression level %d\n", params->level);
> + return -EINVAL;
> + }
This is purely optional, if you add per-backend pr_fmt() with backend
name e.g. "zstd:","lz4:" and so on (in a separate patch) then you don't
need to explicitly prefix every pr_err().
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v5 3/4] zram: validate parameters in each backend's setup_params
2026-08-04 5:48 ` Sergey Senozhatsky
@ 2026-08-04 7:13 ` haoqin huang
2026-08-04 8:28 ` Sergey Senozhatsky
0 siblings, 1 reply; 81+ messages in thread
From: haoqin huang @ 2026-08-04 7:13 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jens Axboe, Nick Terrell, David Sterba,
Andrew Morton, linux-kernel, linux-block, Haoqin Huang,
Rongwei Wang
On Tue, Aug 4, 2026 at 1:48 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/08/03 22:12), Haoqin Huang wrote:
> [..]
> > static int setup_params_842(struct zcomp_params *params)
> > {
> > + if (params->dict_sz) {
> > + pr_err("842: dictionary is not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > + if (params->level != ZCOMP_PARAM_NOT_SET) {
> > + pr_err("842: compression level is not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > return 0;
> > }
> [..]
> > static int deflate_setup_params(struct zcomp_params *params)
> > {
> > + if (params->dict_sz) {
> > + pr_err("deflate: dictionary is not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > if (params->level == ZCOMP_PARAM_NOT_SET)
> > params->level = Z_DEFAULT_COMPRESSION;
> > + else if (params->level < Z_DEFAULT_COMPRESSION ||
> > + params->level > Z_BEST_COMPRESSION) {
> > + pr_err("deflate: invalid compression level %d\n", params->level);
> > + return -EINVAL;
> > + }
>
> This is purely optional, if you add per-backend pr_fmt() with backend
> name e.g. "zstd:","lz4:" and so on (in a separate patch) then you don't
> need to explicitly prefix every pr_err().
I considered adding pr_fmt, but the existing winbits message already
has "deflate" inline:
pr_err("invalid deflate winbits: %d\n", wb);
which would become "deflate: invalid deflate winbits:" and look redundant.
I can add pr_fmt in a separate patch and clean that up to just
"invalid winbits %d" at the same time though, if you prefer.
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v5 3/4] zram: validate parameters in each backend's setup_params
2026-08-04 7:13 ` haoqin huang
@ 2026-08-04 8:28 ` Sergey Senozhatsky
0 siblings, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 8:28 UTC (permalink / raw)
To: haoqin huang
Cc: Sergey Senozhatsky, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, Andrew Morton, linux-kernel, linux-block,
Haoqin Huang, Rongwei Wang
On (26/08/04 15:13), haoqin huang wrote:
> > On (26/08/03 22:12), Haoqin Huang wrote:
> > [..]
> > > static int setup_params_842(struct zcomp_params *params)
> > > {
> > > + if (params->dict_sz) {
> > > + pr_err("842: dictionary is not supported\n");
> > > + return -EOPNOTSUPP;
> > > + }
> > > + if (params->level != ZCOMP_PARAM_NOT_SET) {
> > > + pr_err("842: compression level is not supported\n");
> > > + return -EOPNOTSUPP;
> > > + }
> > > return 0;
> > > }
> > [..]
> > > static int deflate_setup_params(struct zcomp_params *params)
> > > {
> > > + if (params->dict_sz) {
> > > + pr_err("deflate: dictionary is not supported\n");
> > > + return -EOPNOTSUPP;
> > > + }
> > > +
> > > if (params->level == ZCOMP_PARAM_NOT_SET)
> > > params->level = Z_DEFAULT_COMPRESSION;
> > > + else if (params->level < Z_DEFAULT_COMPRESSION ||
> > > + params->level > Z_BEST_COMPRESSION) {
> > > + pr_err("deflate: invalid compression level %d\n", params->level);
> > > + return -EINVAL;
> > > + }
> >
> > This is purely optional, if you add per-backend pr_fmt() with backend
> > name e.g. "zstd:","lz4:" and so on (in a separate patch) then you don't
> > need to explicitly prefix every pr_err().
>
> I considered adding pr_fmt, but the existing winbits message already
> has "deflate" inline:
>
> pr_err("invalid deflate winbits: %d\n", wb);
Feel free to remove it, if you add per-backend pr_fmt().
> which would become "deflate: invalid deflate winbits:" and look redundant.
> I can add pr_fmt in a separate patch and clean that up to just
> "invalid winbits %d" at the same time though, if you prefer.
Sounds good.
^ permalink raw reply [flat|nested] 81+ messages in thread
* [PATCH v5 4/4] zram: reset per-priority params when changing algorithm before init
2026-08-03 14:12 ` [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation Haoqin Huang
` (2 preceding siblings ...)
2026-08-03 14:12 ` [PATCH v5 3/4] zram: validate parameters in each backend's setup_params Haoqin Huang
@ 2026-08-03 14:12 ` Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
4 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-03 14:12 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Parameters validated against one algorithm may be invalid for another
(e.g. lz4 accepts level=65535 but zstd does not). Although algorithm
changes are blocked after disksize is set, they are allowed before
device initialization. Reset per-priority params on algorithm change
so that stale parameters do not silently carry over.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/zram_drv.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 68e60c9eb8b3..cc96b18bf647 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1652,6 +1652,17 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
zram->comp_algs[prio] = alg;
}
+static void comp_params_reset(struct zram *zram, u32 prio)
+{
+ struct zcomp_params *params = &zram->params[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_algorithm_store(struct zram *zram, u32 prio, const char *buf)
{
const char *alg;
@@ -1672,20 +1683,10 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
}
comp_algorithm_set(zram, prio, alg);
+ comp_params_reset(zram, prio);
return 0;
}
-static void comp_params_reset(struct zram *zram, u32 prio)
-{
- struct zcomp_params *params = &zram->params[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,
struct deflate_params *deflate_params)
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation
2026-08-03 14:12 ` [PATCH v5 0/4] zram: fix zstd error paths and add parameter validation Haoqin Huang
` (3 preceding siblings ...)
2026-08-03 14:12 ` [PATCH v5 4/4] zram: reset per-priority params when changing algorithm before init Haoqin Huang
@ 2026-08-04 9:38 ` Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 1/5] zram: do not release zstd global params from error paths Haoqin Huang
` (6 more replies)
4 siblings, 7 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-04 9:38 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang
From: Haoqin Huang <haoqinhuang@tencent.com>
Patch 1 removes zstd_release_params() from both zstd_create() and
zstd_setup_params() error paths -- the former is a layering violation
in a per-CPU callback, the latter is redundant as zcomp_init() already
calls release_params() on setup failure.
Patch 2 rejects zero-size dictionaries and prints distinct error
messages for sz < 0 (returns the original error code) and sz == 0
("empty file"). Currently errors are silently swallowed.
Patch 3 adds pr_fmt to each backend file so that pr_err() messages
are auto-prefixed with the algorithm name.
Patch 4 validates dict and level parameters in each backend's
.setup_params(), rejecting unsupported combinations and out-of-range
levels.
Patch 5 resets per-priority params on algorithm change before init.
Changes since v5:
- Patch 2: split pr_err into sz < 0 and sz == 0 branches
- New patch 3: add pr_fmt to all backends; tweak winbits message;
add missing SPDX headers to lz4 and lz4hc
- Patch 4: removed inline algo-name prefixes (now handled by pr_fmt);
added comment for lz4hc < 1 lower bound; added braces to if
branches for consistency
v5: https://lore.kernel.org/all/20260803141256.60599-1-haoqinhuang7@gmail.com/
Haoqin Huang (5):
zram: do not release zstd global params from error paths
zram: reject zero-size dictionary
zram: add pr_fmt to backend files
zram: validate parameters in each backend's setup_params
zram: reset per-priority params when changing algorithm before init
drivers/block/zram/backend_842.c | 10 +++++++++
drivers/block/zram/backend_deflate.c | 17 ++++++++++++--
drivers/block/zram/backend_lz4.c | 10 ++++++++-
drivers/block/zram/backend_lz4hc.c | 16 +++++++++++++-
drivers/block/zram/backend_lzo.c | 10 +++++++++
drivers/block/zram/backend_lzorle.c | 10 +++++++++
drivers/block/zram/backend_zstd.c | 11 +++++++---
drivers/block/zram/zram_drv.c | 33 ++++++++++++++++++----------
8 files changed, 98 insertions(+), 19 deletions(-)
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v6 1/5] zram: do not release zstd global params from error paths
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
@ 2026-08-04 9:38 ` Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 2/5] zram: reject zero-size dictionary Haoqin Huang
` (5 subsequent siblings)
6 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-04 9:38 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
zstd_setup_params() creates global cdict and ddict stored in
params->drv_data, shared across all per-CPU contexts. The per-CPU
zstd_create() error path called zstd_release_params(), which freed
those globally-shared objects. This is a layering violation: a
per-CPU callback should only clean up its own context, not release
resources owned by the compression lifecycle.
zstd_setup_params() called zstd_release_params() on its own error
path as well, but zcomp_init() already calls release_params() when
setup fails, so this is redundant.
Remove zstd_release_params() from both error paths.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_zstd.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index d00b548056dc..5fabc3e7e975 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -85,7 +85,6 @@ static int zstd_setup_params(struct zcomp_params *params)
return 0;
error:
- zstd_release_params(params);
return -EINVAL;
}
@@ -161,7 +160,6 @@ static int zstd_create(struct zcomp_params *params, struct zcomp_ctx *ctx)
return 0;
error:
- zstd_release_params(params);
zstd_destroy(ctx);
return -EINVAL;
}
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v6 2/5] zram: reject zero-size dictionary
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 1/5] zram: do not release zstd global params from error paths Haoqin Huang
@ 2026-08-04 9:38 ` Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 3/5] zram: add pr_fmt to backend files Haoqin Huang
` (4 subsequent siblings)
6 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-04 9:38 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
kernel_read_file_from_path() already rejects empty files (i_size <= 0)
and returns -EINVAL, but the current implementation only checks for
sz < 0 without logging any information. Use sz == 0 to reject the
zero-size case and print distinct error messages for each failure type.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/zram_drv.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cfa98846ac48..f73e30b61067 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1700,8 +1700,16 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level,
INT_MAX,
NULL,
READING_POLICY);
- if (sz < 0)
+ if (sz < 0) {
+ pr_err("failed to load dictionary %s (err=%zd)\n",
+ dict_path, sz);
+ return sz;
+ }
+ if (sz == 0) {
+ pr_err("failed to load dictionary %s (empty file)\n",
+ dict_path);
return -EINVAL;
+ }
}
zram->params[prio].dict_sz = sz;
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v6 3/5] zram: add pr_fmt to backend files
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 1/5] zram: do not release zstd global params from error paths Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 2/5] zram: reject zero-size dictionary Haoqin Huang
@ 2026-08-04 9:38 ` Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 4/5] zram: validate parameters in each backend's setup_params Haoqin Huang
` (3 subsequent siblings)
6 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-04 9:38 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Add pr_fmt to each backend so that pr_err() messages are auto-prefixed
with the algorithm name. While at it, tweak the deflate winbits
pr_err to avoid a duplicated "deflate" prefix.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_842.c | 2 ++
drivers/block/zram/backend_deflate.c | 4 +++-
drivers/block/zram/backend_lz4.c | 4 ++++
drivers/block/zram/backend_lz4hc.c | 4 ++++
drivers/block/zram/backend_lzo.c | 2 ++
drivers/block/zram/backend_lzorle.c | 2 ++
drivers/block/zram/backend_zstd.c | 2 ++
7 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/backend_842.c b/drivers/block/zram/backend_842.c
index 10d9d5c60f53..d9b8a6bba2cb 100644
--- a/drivers/block/zram/backend_842.c
+++ b/drivers/block/zram/backend_842.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#define pr_fmt(fmt) "842: " fmt
+
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/sw842.h>
diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index b3f7d08b49d9..ee26e6c9282f 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#define pr_fmt(fmt) "deflate: " fmt
+
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
@@ -30,7 +32,7 @@ static int deflate_setup_params(struct zcomp_params *params)
s32 wb = params->deflate.winbits;
if ((wb < -15 || wb > -9) && (wb < 9 || wb > 15)) {
- pr_err("invalid deflate winbits: %d\n", wb);
+ pr_err("invalid winbits %d\n", wb);
return -EINVAL;
}
}
diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
index c449d511ba86..6d58956ed5b2 100644
--- a/drivers/block/zram/backend_lz4.c
+++ b/drivers/block/zram/backend_lz4.c
@@ -1,3 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#define pr_fmt(fmt) "lz4: " fmt
+
#include <linux/kernel.h>
#include <linux/lz4.h>
#include <linux/slab.h>
diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
index f6a336acfe20..c0c3715087c8 100644
--- a/drivers/block/zram/backend_lz4hc.c
+++ b/drivers/block/zram/backend_lz4hc.c
@@ -1,3 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#define pr_fmt(fmt) "lz4hc: " fmt
+
#include <linux/kernel.h>
#include <linux/lz4.h>
#include <linux/slab.h>
diff --git a/drivers/block/zram/backend_lzo.c b/drivers/block/zram/backend_lzo.c
index 4c906beaae6b..84330dea6af5 100644
--- a/drivers/block/zram/backend_lzo.c
+++ b/drivers/block/zram/backend_lzo.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#define pr_fmt(fmt) "lzo: " fmt
+
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/lzo.h>
diff --git a/drivers/block/zram/backend_lzorle.c b/drivers/block/zram/backend_lzorle.c
index 10640c96cbfc..b3b03a008b64 100644
--- a/drivers/block/zram/backend_lzorle.c
+++ b/drivers/block/zram/backend_lzorle.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#define pr_fmt(fmt) "lzo-rle: " fmt
+
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/lzo.h>
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 5fabc3e7e975..fb61acdaef67 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#define pr_fmt(fmt) "zstd: " fmt
+
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v6 4/5] zram: validate parameters in each backend's setup_params
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
` (2 preceding siblings ...)
2026-08-04 9:38 ` [PATCH v6 3/5] zram: add pr_fmt to backend files Haoqin Huang
@ 2026-08-04 9:38 ` Haoqin Huang
2026-08-04 9:38 ` [PATCH v6 5/5] zram: reset per-priority params when changing algorithm before init Haoqin Huang
` (2 subsequent siblings)
6 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-04 9:38 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Dict and level parameters are silently accepted even for backends
that do not support them. Validate these parameters in each backend's
.setup_params() to reject unsupported combinations and out-of-range
levels with a specific error message.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/backend_842.c | 8 ++++++++
drivers/block/zram/backend_deflate.c | 13 ++++++++++++-
drivers/block/zram/backend_lz4.c | 6 +++++-
drivers/block/zram/backend_lz4hc.c | 12 +++++++++++-
drivers/block/zram/backend_lzo.c | 8 ++++++++
drivers/block/zram/backend_lzorle.c | 8 ++++++++
drivers/block/zram/backend_zstd.c | 7 ++++++-
7 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/drivers/block/zram/backend_842.c b/drivers/block/zram/backend_842.c
index d9b8a6bba2cb..3846a04c69d7 100644
--- a/drivers/block/zram/backend_842.c
+++ b/drivers/block/zram/backend_842.c
@@ -15,6 +15,14 @@ static void release_params_842(struct zcomp_params *params)
static int setup_params_842(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c
index ee26e6c9282f..f71b11bcac78 100644
--- a/drivers/block/zram/backend_deflate.c
+++ b/drivers/block/zram/backend_deflate.c
@@ -24,8 +24,19 @@ 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)
+ if (params->dict_sz) {
+ pr_err("dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (params->level == ZCOMP_PARAM_NOT_SET) {
params->level = Z_DEFAULT_COMPRESSION;
+ } else if (params->level < Z_DEFAULT_COMPRESSION ||
+ params->level > Z_BEST_COMPRESSION) {
+ pr_err("invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
+
if (params->deflate.winbits == ZCOMP_PARAM_NOT_SET) {
params->deflate.winbits = DEFLATE_DEF_WINBITS;
} else {
diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c
index 6d58956ed5b2..1e28104ad964 100644
--- a/drivers/block/zram/backend_lz4.c
+++ b/drivers/block/zram/backend_lz4.c
@@ -32,8 +32,12 @@ static int lz4_setup_params(struct zcomp_params *params)
LZ4_stream_t *dict_stream;
int ret;
- if (params->level == ZCOMP_PARAM_NOT_SET)
+ if (params->level == ZCOMP_PARAM_NOT_SET) {
params->level = LZ4_ACCELERATION_DEFAULT;
+ } else if (params->level < LZ4_ACCELERATION_DEFAULT) {
+ pr_err("invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
if (!params->dict || !params->dict_sz)
return 0;
diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c
index c0c3715087c8..d8aa01bb258f 100644
--- a/drivers/block/zram/backend_lz4hc.c
+++ b/drivers/block/zram/backend_lz4hc.c
@@ -22,8 +22,18 @@ static void lz4hc_release_params(struct zcomp_params *params)
static int lz4hc_setup_params(struct zcomp_params *params)
{
- if (params->level == ZCOMP_PARAM_NOT_SET)
+ if (params->level == ZCOMP_PARAM_NOT_SET) {
params->level = LZ4HC_DEFAULT_CLEVEL;
+ } else if (params->level < 1 || params->level > LZ4HC_MAX_CLEVEL) {
+ /*
+ * Use < 1 rather than < LZ4HC_MIN_CLEVEL here because
+ * LZ4HC_compress_generic() only clamps levels below 1
+ * (levels 1 and 2 are valid). LZ4HC_MIN_CLEVEL (3) is
+ * advisory and not enforced by the library.
+ */
+ pr_err("invalid compression level %d\n", params->level);
+ return -EINVAL;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_lzo.c b/drivers/block/zram/backend_lzo.c
index 84330dea6af5..d83f92cf757c 100644
--- a/drivers/block/zram/backend_lzo.c
+++ b/drivers/block/zram/backend_lzo.c
@@ -14,6 +14,14 @@ static void lzo_release_params(struct zcomp_params *params)
static int lzo_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_lzorle.c b/drivers/block/zram/backend_lzorle.c
index b3b03a008b64..1b120d062c92 100644
--- a/drivers/block/zram/backend_lzorle.c
+++ b/drivers/block/zram/backend_lzorle.c
@@ -14,6 +14,14 @@ static void lzorle_release_params(struct zcomp_params *params)
static int lzorle_setup_params(struct zcomp_params *params)
{
+ if (params->dict_sz) {
+ pr_err("dictionary is not supported\n");
+ return -EOPNOTSUPP;
+ }
+ if (params->level != ZCOMP_PARAM_NOT_SET) {
+ pr_err("compression level is not supported\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index fb61acdaef67..08da3810cffd 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -60,8 +60,13 @@ static int zstd_setup_params(struct zcomp_params *params)
return -ENOMEM;
params->drv_data = zp;
- if (params->level == ZCOMP_PARAM_NOT_SET)
+ if (params->level == ZCOMP_PARAM_NOT_SET) {
params->level = zstd_default_clevel();
+ } else if (params->level < zstd_min_clevel() ||
+ params->level > zstd_max_clevel()) {
+ pr_err("invalid compression level %d\n", params->level);
+ goto error;
+ }
zp->cprm = zstd_get_params(params->level, PAGE_SIZE);
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* [PATCH v6 5/5] zram: reset per-priority params when changing algorithm before init
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
` (3 preceding siblings ...)
2026-08-04 9:38 ` [PATCH v6 4/5] zram: validate parameters in each backend's setup_params Haoqin Huang
@ 2026-08-04 9:38 ` Haoqin Huang
2026-08-04 9:53 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Sergey Senozhatsky
2026-08-04 19:55 ` Andrew Morton
6 siblings, 0 replies; 81+ messages in thread
From: Haoqin Huang @ 2026-08-04 9:38 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky
Cc: Jens Axboe, Nick Terrell, David Sterba, Andrew Morton,
linux-kernel, linux-block, Haoqin Huang, Rongwei Wang
From: Haoqin Huang <haoqinhuang@tencent.com>
Parameters validated against one algorithm may be invalid for another
(e.g. lz4 accepts level=65535 but zstd does not). Although algorithm
changes are blocked after disksize is set, they are allowed before
device initialization. Reset per-priority params on algorithm change
so that stale parameters do not silently carry over.
Signed-off-by: Haoqin Huang <haoqinhuang@tencent.com>
Signed-off-by: Rongwei Wang <zigiwang@tencent.com>
---
drivers/block/zram/zram_drv.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f73e30b61067..56183c827e1b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1652,6 +1652,17 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
zram->comp_algs[prio] = alg;
}
+static void comp_params_reset(struct zram *zram, u32 prio)
+{
+ struct zcomp_params *params = &zram->params[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_algorithm_store(struct zram *zram, u32 prio, const char *buf)
{
const char *alg;
@@ -1672,20 +1683,10 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
}
comp_algorithm_set(zram, prio, alg);
+ comp_params_reset(zram, prio);
return 0;
}
-static void comp_params_reset(struct zram *zram, u32 prio)
-{
- struct zcomp_params *params = &zram->params[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,
struct deflate_params *deflate_params)
--
2.43.7
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
` (4 preceding siblings ...)
2026-08-04 9:38 ` [PATCH v6 5/5] zram: reset per-priority params when changing algorithm before init Haoqin Huang
@ 2026-08-04 9:53 ` Sergey Senozhatsky
2026-08-04 9:55 ` Sergey Senozhatsky
2026-08-04 19:55 ` Andrew Morton
6 siblings, 1 reply; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 9:53 UTC (permalink / raw)
To: Andrew Morton, Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, linux-kernel, linux-block, Haoqin Huang
On (26/08/04 17:38), Haoqin Huang wrote:
> From: Haoqin Huang <haoqinhuang@tencent.com>
>
> Patch 1 removes zstd_release_params() from both zstd_create() and
> zstd_setup_params() error paths -- the former is a layering violation
> in a per-CPU callback, the latter is redundant as zcomp_init() already
> calls release_params() on setup failure.
>
> Patch 2 rejects zero-size dictionaries and prints distinct error
> messages for sz < 0 (returns the original error code) and sz == 0
> ("empty file"). Currently errors are silently swallowed.
>
> Patch 3 adds pr_fmt to each backend file so that pr_err() messages
> are auto-prefixed with the algorithm name.
>
> Patch 4 validates dict and level parameters in each backend's
> .setup_params(), rejecting unsupported combinations and out-of-range
> levels.
>
> Patch 5 resets per-priority params on algorithm change before init.
>
> Changes since v5:
> - Patch 2: split pr_err into sz < 0 and sz == 0 branches
> - New patch 3: add pr_fmt to all backends; tweak winbits message;
> add missing SPDX headers to lz4 and lz4hc
> - Patch 4: removed inline algo-name prefixes (now handled by pr_fmt);
> added comment for lz4hc < 1 lower bound; added braces to if
> branches for consistency
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
^ permalink raw reply [flat|nested] 81+ messages in thread* Re: [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation
2026-08-04 9:53 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Sergey Senozhatsky
@ 2026-08-04 9:55 ` Sergey Senozhatsky
0 siblings, 0 replies; 81+ messages in thread
From: Sergey Senozhatsky @ 2026-08-04 9:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Haoqin Huang, Minchan Kim, Jens Axboe, Nick Terrell,
David Sterba, linux-kernel, linux-block, Haoqin Huang,
Sergey Senozhatsky
On (26/08/04 18:53), Sergey Senozhatsky wrote:
> > Patch 1 removes zstd_release_params() from both zstd_create() and
> > zstd_setup_params() error paths -- the former is a layering violation
> > in a per-CPU callback, the latter is redundant as zcomp_init() already
> > calls release_params() on setup failure.
> >
> > Patch 2 rejects zero-size dictionaries and prints distinct error
> > messages for sz < 0 (returns the original error code) and sz == 0
> > ("empty file"). Currently errors are silently swallowed.
> >
> > Patch 3 adds pr_fmt to each backend file so that pr_err() messages
> > are auto-prefixed with the algorithm name.
> >
> > Patch 4 validates dict and level parameters in each backend's
> > .setup_params(), rejecting unsupported combinations and out-of-range
> > levels.
> >
> > Patch 5 resets per-priority params on algorithm change before init.
> >
> > Changes since v5:
> > - Patch 2: split pr_err into sz < 0 and sz == 0 branches
> > - New patch 3: add pr_fmt to all backends; tweak winbits message;
> > add missing SPDX headers to lz4 and lz4hc
> > - Patch 4: removed inline algo-name prefixes (now handled by pr_fmt);
> > added comment for lz4hc < 1 lower bound; added braces to if
> > branches for consistency
>
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Oh, and also
Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
^ permalink raw reply [flat|nested] 81+ messages in thread
* Re: [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation
2026-08-04 9:38 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Haoqin Huang
` (5 preceding siblings ...)
2026-08-04 9:53 ` [PATCH v6 0/5] zram: fix zstd error paths and add parameter validation Sergey Senozhatsky
@ 2026-08-04 19:55 ` Andrew Morton
6 siblings, 0 replies; 81+ messages in thread
From: Andrew Morton @ 2026-08-04 19:55 UTC (permalink / raw)
To: Haoqin Huang
Cc: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Nick Terrell,
David Sterba, linux-kernel, linux-block, Haoqin Huang
On Tue, 4 Aug 2026 17:38:36 +0800 Haoqin Huang <haoqinhuang7@gmail.com> wrote:
> Patch 1 removes zstd_release_params() from both zstd_create() and
> zstd_setup_params() error paths -- the former is a layering violation
> in a per-CPU callback, the latter is redundant as zcomp_init() already
> calls release_params() on setup failure.
>
> Patch 2 rejects zero-size dictionaries and prints distinct error
> messages for sz < 0 (returns the original error code) and sz == 0
> ("empty file"). Currently errors are silently swallowed.
>
> Patch 3 adds pr_fmt to each backend file so that pr_err() messages
> are auto-prefixed with the algorithm name.
>
> Patch 4 validates dict and level parameters in each backend's
> .setup_params(), rejecting unsupported combinations and out-of-range
> levels.
>
> Patch 5 resets per-priority params on algorithm change before init.
>
Thanks. AI review pointed at a few things, most of them pre-existing:
https://sashiko.dev/#/patchset/20260804093841.67920-1-haoqinhuang7@gmail.com
^ permalink raw reply [flat|nested] 81+ messages in thread