| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150827040712.GF1545@swordfish>
Date: Thu, 27 Aug 2015 13:07:12 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To: Joonsoo Kim <js1304@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Minchan Kim <minchan@...nel.org>,
Nitin Gupta <ngupta@...are.org>,
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
Herbert Xu <herbert@...dor.apana.org.au>,
"David S. Miller" <davem@...emloft.net>,
Stephan Mueller <smueller@...onox.de>,
Joonsoo Kim <iamjoonsoo.kim@....com>
Subject: Re: [PATCH v2 8/8] zram: use decompress_noctx
On (08/20/15 15:35), Joonsoo Kim wrote:
> Crypto subsystem supports noctx API which doesn't require tfm.
> To get tfm in zram, we need zstrm and it is limited resource.
> If we uses noctx API, we don't need to get zstrm so that
> we get much better performance when zstrm is insufficient.
>
> This patch restores zram's performance to the time that zram
> doesn't use crypto subsystem.
>
> Following is zram's read performance number.
>
> * iozone -t 4 -R -r 16K -s 60M -I +Z -i 0 -i 1
> * max_stream is set to 1
> * Output is in Kbytes/sec
>
> zram-base vs zram-crypto vs zram-crypto-noctx
>
> Read 10411701.88 6426911.62 9423894.38
> Re-read 10017386.62 6428218.88 11000063.50
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@....com>
> ---
> drivers/block/zram/zcomp.c | 28 +++++++++++++++++++++++++++-
> drivers/block/zram/zcomp.h | 9 ++++++++-
> drivers/block/zram/zram_drv.c | 9 +++++----
> 3 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index d2734f2..86b0c9b 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -291,8 +291,12 @@ bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
> return comp->set_max_streams(comp, num_strm);
> }
>
> -struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
> +struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp)
> {
> + /* We don't need decompression context so zstrm neither */
> + if (decomp && test_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags))
> + return NULL;
> +
> return comp->strm_find(comp);
> }
No. zcomp_strm_find() should never return NULL. And no, I don't like
"if (decomp)" change.
>
> @@ -307,6 +311,11 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> {
> *dst_len = PAGE_SIZE << 1;
>
> + if (!zstrm) {
> + return crypto_comp_compress_noctx(comp->alg, src, PAGE_SIZE,
> + dst, dst_len);
> + }
> +
> return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE, dst, dst_len);
> }
>
> @@ -316,12 +325,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> {
> unsigned int size = PAGE_SIZE;
>
> + if (!zstrm) {
> + return crypto_comp_decompress_noctx(comp->alg, src, src_len,
> + dst, &size);
> + }
> +
> return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
> }
>
> void zcomp_destroy(struct zcomp *comp)
> {
> comp->destroy(comp);
> + crypto_put_comp(comp->alg);
> kfree(comp);
> }
>
> @@ -344,12 +359,23 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> return ERR_PTR(-ENOMEM);
>
> comp->name = compress;
> + comp->alg = crypto_get_comp(compress, 0, 0);
> + if (!comp->alg) {
> + kfree(comp);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (crypto_has_compress_noctx(comp->alg))
> + set_bit(ZCOMP_COMPRESS_NOCTX, &comp->flags);
do you use ZCOMP_COMPRESS_NOCTX algs in this patch set?
> + if (crypto_has_decompress_noctx(comp->alg))
> + set_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags);
>
> if (max_strm > 1)
> zcomp_strm_multi_create(comp, max_strm);
> else
> zcomp_strm_single_create(comp);
> if (!comp->stream) {
> + crypto_put_comp(comp->alg);
> kfree(comp);
> return ERR_PTR(-ENOMEM);
> }
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index c47db4d..6c137c8 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -13,6 +13,11 @@
> #include <linux/crypto.h>
> #include <linux/mutex.h>
>
> +enum {
> + ZCOMP_COMPRESS_NOCTX,
> + ZCOMP_DECOMPRESS_NOCTX,
> +};
Can it be handled via crypto api? check if ->foo_noctx() is !NULL ?
> struct zcomp_strm {
> struct crypto_comp *tfm;
>
> @@ -27,6 +32,8 @@ struct zcomp_strm {
> struct zcomp {
> void *stream;
> const char *name;
> + struct crypto_alg *alg;
> + unsigned long flags;
>
> struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> @@ -39,7 +46,7 @@ ssize_t zcomp_available_show(const char *comp, char *buf);
> struct zcomp *zcomp_create(const char *comp, int max_strm);
> void zcomp_destroy(struct zcomp *comp);
>
> -struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
> +struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp);
> void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index b3044d3..8283bd3 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -623,7 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> /* Use a temporary buffer to decompress the page */
> uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
>
> - zstrm = zcomp_strm_find(zram->comp);
> + zstrm = zcomp_strm_find(zram->comp, true);
No, I don't like this.
> user_mem = kmap_atomic(page);
> if (!is_partial_io(bvec))
> uncmem = user_mem;
> @@ -647,7 +647,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> ret = 0;
> out_cleanup:
> kunmap_atomic(user_mem);
> - zcomp_strm_release(zram->comp, zstrm);
> + if (zstrm)
> + zcomp_strm_release(zram->comp, zstrm);
> if (is_partial_io(bvec))
> kfree(uncmem);
> return ret;
> @@ -676,14 +677,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> ret = -ENOMEM;
> goto out;
> }
> - zstrm = zcomp_strm_find(zram->comp);
> + zstrm = zcomp_strm_find(zram->comp, true);
> ret = zram_decompress_page(zram, zstrm, uncmem, index);
> if (ret)
> goto out;
> }
>
> if (!zstrm)
> - zstrm = zcomp_strm_find(zram->comp);
> + zstrm = zcomp_strm_find(zram->comp, false);
No. I don't like this.
-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists