Re: [PATCH v2 3/5] zstd: move ZSTD_MAX_CLEVEL to zstd_lib.h
haoqin huang <[email protected]> Thu, 30 Jul 2026 10:48:21 +0800
| Newsgroups | org.kernel.vger.linux-block,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAEjiKSkQ2+Amf2SbD09tnmKVKh0Of+c+SSBfEFtg=Lfk+b64NA@mail.gmail.com> |
On Wed, Jul 29, 2026 at 12:50 PM Sergey Senozhatsky <[email protected]> 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.