Re: [RFC PATCH 2/3] zram: introduce zram compressor operations struct

From: Sergey Senozhatsky
Date: Tue Jan 21 2014 - 03:50:28 EST


On (01/21/14 09:09), Minchan Kim wrote:
> On Mon, Jan 20, 2014 at 01:03:48PM +0300, Sergey Senozhatsky wrote:
> > On (01/20/14 14:12), Minchan Kim wrote:
> > > Hello Sergey,
> > >
> > > I reviewed this patchset and I suggest somethings.
> > > Please have a look and feedback to me. :)
> > >
> > > 1. Let's define new file zram_comp.c
> > > 2. zram_comp includes following field
> > > .create
> > > .compress
> > > .decompress.
> > > .destroy
> > > .name
> > >
> >
> > alternatively, we can use crypto api, the same way as zswap does (that
> > will require handling of cpu hotplug).
> >
> > -ss
>
> I really doubt what's the benefit from crypto API for zram.
> It's maybe since I'm not familiar with it so I should ask a silly
> question.
>
> 1. What's the runtime overhead for using such frontend?
>
> As you know, zram is in-memory block device so I don't want to
> add unnecessary overhead to optimize.
>
> 2. What's the memory footprint for using such frontend?
>
> As you know, zram is very popular for small-memory embedded device
> so I don't want to consume more runtime memory and static memory
> due to CONFIG_CRYPTO friend.
>

schematically:

char *compressor = "lzo"; /* or supplied by user via COMPRESSOR_store() */

if (crypto_has_comp(compressor, 0, 0)) {
tfms = alloc_percpu(struct crypto_comp *);
tfm = crypto_alloc_comp(compressor, 0, 0);
*per_cpu_ptr(tfms, cpu) = tfm;
}

ret = crypto_comp_compress(tfm, src, slen, dst, dlen);
and
ret = crypto_comp_decompress(tfm, src, slen, dst, dlen);


memory overhead looks like a bunch of structures (transformation
contexts -- tfm). they touch tfm within compress/decompress, so crypto
users usually allocate per cpu tfm or somehow protect parallel tfm
usage, which is a big disadvantage to my opinion. otoh, crypto provides
HC compressors (LZ4HC at the moment).

my choice is to use raw compressor (as I did in initial patchset).

-ss

> 3. Is it a flexible to alloc/handle multiple compressor buffer for
> the our purpose? zswap and zcache have been used it with per-cpu
> buffer but it would a problem for write scalabitliy if we uses zlib
> which takes long time to compress.
> When I read code, maybe we can allocate multiple buffers through
> cryptop_alloc_compo several time but it would cause 1) and 2) problem
> again.
>
> So, what's the attractive point for using crypto?
> One of thing I could imagine is that it could make zram H/W compressor
> but I don't have heard about it so if we don't have any special reason,
> I'd like to go with raw compressor so we can get a *base* number. Then,
> if we really need crypto API, we can change it easily and benchmark.
> Finally, we could get a comparision number in future and it would make
> the decision easily.
>
> Thanks.
>
> --
> Kind regards,
> Minchan Kim
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/