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.