From: Joonsoo Kim Subject: Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API Date: Fri, 25 Sep 2015 14:46:47 +0900 Message-ID: <20150925054647.GG7158@js1304-P5Q-DELUXE> References: <1442553564-3476-1-git-send-email-iamjoonsoo.kim@lge.com> <1442553564-3476-10-git-send-email-iamjoonsoo.kim@lge.com> <20150921035128.GD27729@bbox> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andrew Morton , Nitin Gupta , Sergey Senozhatsky , linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, Herbert Xu , "David S. Miller" , Stephan Mueller To: Minchan Kim Return-path: Received: from LGEAMRELO11.lge.com ([156.147.23.51]:54097 "EHLO lgeamrelo11.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750946AbbIYFpm (ORCPT ); Fri, 25 Sep 2015 01:45:42 -0400 Content-Disposition: inline In-Reply-To: <20150921035128.GD27729@bbox> Sender: linux-crypto-owner@vger.kernel.org List-ID: On Mon, Sep 21, 2015 at 12:51:28PM +0900, Minchan Kim wrote: > On Fri, Sep 18, 2015 at 02:19:24PM +0900, Joonsoo Kim wrote: > > Crypto subsystem now supports decompress_noctx API that requires > > special tfm_noctx. This tfm can be shared by multiple concurrent > > decompress user because this API doesn't rely on this tfm object > > except to fetch decompress function pointer. > > > > Until changing to use crypto API, zram doesn't require any zstrm > > on decompress so decompress is parallelized unlimitedly. But, previous > > patch make zram to use crypto API and this requires one zstrm on every > > decompress users so, in some zstrm contended situations, zram's > > performance would be degraded. > > > > This patch makes zram use decompress_noctx API and restore zram's > > performance as the time that zram doesn't use crypto API. > > > > 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 > > --- > > drivers/block/zram/zcomp.c | 24 +++++++++++++++++++++++- > > drivers/block/zram/zcomp.h | 1 + > > 2 files changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > > index c2ed7c8..a020200 100644 > > --- a/drivers/block/zram/zcomp.c > > +++ b/drivers/block/zram/zcomp.c > > @@ -319,9 +319,12 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm) > > zcomp_strm_release(comp, zstrm); > > } > > > > -/* Never return NULL, may sleep */ > > +/* May return NULL, may sleep */ > > struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp) > > { > > + if (comp->tfm_noctx) > > + return NULL; > > Hmm,, I understand why returns NULL but it seems to be awkward to use NULL > to express meaningful semantic and following functions relies on. > If I have a time, I will think over. I also think that it's not good thing but I can't think any better idea. Please let me know if you have better one. > > > + > > return zcomp_strm_find(comp); > > } > > > > @@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm, > > { > > unsigned int size = PAGE_SIZE; > > > > + if (!zstrm) { > > + return crypto_comp_decompress_noctx(comp->tfm_noctx, > > + src, src_len, dst, &size); > > + } > > + > > return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size); > > } > > > > void zcomp_destroy(struct zcomp *comp) > > { > > + if (comp->tfm_noctx) > > + crypto_free_comp_noctx(comp->tfm_noctx); > > comp->destroy(comp); > > kfree(comp); > > } > > @@ -366,6 +376,7 @@ struct zcomp *zcomp_create(const char *compress, int max_strm) > > { > > struct zcomp *comp; > > const char *backend; > > + struct crypto_comp *tfm; > > > > backend = find_backend(compress); > > if (!backend) > > @@ -384,5 +395,16 @@ struct zcomp *zcomp_create(const char *compress, int max_strm) > > kfree(comp); > > return ERR_PTR(-ENOMEM); > > } > > + > > + /* > > + * Prepare to use crypto decompress_noctx API. One tfm is required > > + * to initialize crypto algorithm properly and fetch corresponding > > + * function pointer. But, it is sharable for multiple concurrent > > + * decompress users. > > + */ > > Please comment out that this API will return NULL if the compressor doesn't > support noctx mode. Will do. Thanks.