On (24/05/31 14:47), Sergey Senozhatsky wrote:
> On (24/05/20 19:04), Herbert Xu wrote:
> [..]
> > +int crypto_scomp_setparam(struct crypto_scomp *tfm, const u8 *param,
> > + unsigned int len)
> > +{
> > + struct scomp_alg *scomp = crypto_scomp_alg(tfm);
> > + int err;
> > +
> > + err = scomp->setparam(tfm, param, len);
> > + if (unlikely(err)) {
> > + scomp_set_need_param(tfm, scomp);
> > + return err;
> > + }
> > +
> > + crypto_scomp_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
> > + return 0;
> > +}
>
> Is the idea here that each compression driver will have its own structure
> for params?
>
> In other words, something like this?
>
> static int setup_tfm(...)
> {
> ...
> this_cpu->tfm = crypto_alloc_comp(name, 0, 0);
>
> if (!strcmp(name, "zstd")) {
> struct crypto_comp_param_zstd param;
>
> param.dict = ...
> param.cleve = ...
>
> crypto_scomp_setparam(tfm, ¶m, sizeof(param));
> }
>
> if (!strcmp(name, "lz4")) {
> struct crupto_comp_param_lz4 param;
> ...
> }
>
> if (!strcmp(name, "lzo")) {
> struct crupto_comp_param_lzo param;
> ...
> }
> ...
> }
>
> Or should it be "struct crypto_comp_params param"?
So passing "raw" algorithm parameters to crypto_scomp_setparam(tfm) can be
suboptimal, depending on the compression driver. For instance, for zstd
(what is currently done in zram [1]) we pre-process "raw" parameters:
parse dictionary in order to get zstd_cdict and zstd_ddict which are then
shared by all tfm-s (as they access C/D dictionaries in read-only mode).
For zram/zswap doing this per-tfm would result in extra per-CPU
zstd_cdict/zstd_ddict allocations, which is a significant overhead.
Does this sound like adding two more callbacks to drivers
(e.g. parseparam/freeparam)?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/block/zram/backend_zstd.c?h=next-20240529
