Re: [PATCH 2/3] zram: add per-backend capability flags and validate parameters early

haoqin huang <[email protected]> Tue, 28 Jul 2026 00:50:37 +0800
Newsgroups org.kernel.vger.linux-block,org.kernel.vger.linux-kernel
Message-ID <CAEjiKSknejc9biofmnJu_3xamZj8kuvbxiPuKp6phQJdGwQPCQ@mail.gmail.com>
On Fri, Jul 24, 2026 at 1:28 PM Sergey Senozhatsky
<[email protected]> wrote:
>
> On (26/06/27 15:02), Haoqin Huang wrote:
> > Writing dict or level parameters for algorithms that don't support
> > them was silently accepted but had no effect.  Out-of-range levels
> > were silently clamped by the underlying library.  Dict read failures
> > always lost the real error from kernel_read_file_from_path().
> >
> > Add caps, level_min and level_max to zcomp_ops and validate
> > user-supplied parameters in algorithm_params_store() before storing,
> > giving immediate error feedback. Also fix comp_params_store() to
> > read the new dict into a temporary buffer before resetting old
> > parameters, making the update atomic.
>
> I probably would prefer not to add this.  Again, zram setup is almost
> always automated, you figure out what you need to put into your init
> script once and you never touch it again.
>

That's a fair point. The sysfs validation is admittedly defensive.

My thinking was just that without any feedback, a misconfiguration can
be hard to notice. For instance:

 algo=lzo dict=/data/dict    -> setup_params() silently discards it
 algo=deflate dict=/data/dict -> same, silently ignored
 algo=zstd level=999         -> silently clamped to 22 by the library

All of these succeed without a peep -- not a crash, but also no
indication that anything was wrong.

> The 0 i_size for CD-dict is something that simply should not happen.
> If you insist on handling that then we can replace "sz < 0" with "sz <= 0",
> but that 0 len dictionary case is something purely theoretical.

That's fine, I'll use sz <= 0 in v2.

Anyway, let me split the original patch so the non-controversial part
stands on its own:

  [2/3] zram: make dict update in comp_params_store() atomic

    Move comp_params_reset() after kernel_read_file_from_path(), so an
    I/O error doesn't leave the old dict freed with nothing to replace
    it.  Also preserve the actual error code.

  [3/3] zram: add per-backend caps and validate parameters early

    Optional -- happy to drop if you still feel it's unnecessary.