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....@lge.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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to