Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Sergey Senozhatsky]

On (09/25/15 14:46), Joonsoo Kim wrote:
[..]
> > >  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.

yeah, me neither.

-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Sergey Senozhatsky]

On (09/25/15 14:46), Joonsoo Kim wrote:
[..]
> > >  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.

yeah, me neither.

-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 02:29:18PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> > -/* 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;
> > +
> > 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, );
> > +   }
> 
> unneeded braces.

It's more readable for me. But, if you don't like it, I will remove brace.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 04:56:00PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > +   /*
> > +* 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.
> > +*/
> 
> if comp algorithm require zstrm for both compression and decompression,
> then there seems to be no need in allocating sharable ->tfm_noctx, we
> will never use it.
> 

Yes, in that case, NULL will be returned. I should describe it on
somewhere.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

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
> > 
> > Read10411701.88 6426911.62  9423894.38
> > Re-read 10017386.62 6428218.88  1163.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, );
> > +   }
> > +
> > return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
> >  }
> >  
> >  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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 04:56:00PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > +   /*
> > +* 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.
> > +*/
> 
> if comp algorithm require zstrm for both compression and decompression,
> then there seems to be no need in allocating sharable ->tfm_noctx, we
> will never use it.
> 

Yes, in that case, NULL will be returned. I should describe it on
somewhere.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

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
> > 
> > Read10411701.88 6426911.62  9423894.38
> > Re-read 10017386.62 6428218.88  1163.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, );
> > +   }
> > +
> > return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
> >  }
> >  
> >  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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 02:29:18PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> > -/* 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;
> > +
> > 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, );
> > +   }
> 
> unneeded braces.

It's more readable for me. But, if you don't like it, I will remove brace.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Sergey Senozhatsky]

On (09/18/15 14:19), Joonsoo Kim wrote:
[..]
> + /*
> +  * 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.
> +  */

if comp algorithm require zstrm for both compression and decompression,
then there seems to be no need in allocating sharable ->tfm_noctx, we
will never use it.

-ss

> + tfm = crypto_alloc_comp_noctx(compress, 0, 0);
> + if (!IS_ERR(tfm))
> + comp->tfm_noctx = tfm;
> +
>   return comp;
>  }
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 4f9df8e..c76d8e4 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -26,6 +26,7 @@ struct zcomp_strm {
>  /* dynamic per-device compression frontend */
>  struct zcomp {
>   void *stream;
> + struct crypto_comp *tfm_noctx;
>   const char *backend;
>  
>   struct zcomp_strm *(*strm_find)(struct zcomp *comp);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Sergey Senozhatsky]

On (09/18/15 14:19), Joonsoo Kim wrote:
[..]
> + /*
> +  * 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.
> +  */

if comp algorithm require zstrm for both compression and decompression,
then there seems to be no need in allocating sharable ->tfm_noctx, we
will never use it.

-ss

> + tfm = crypto_alloc_comp_noctx(compress, 0, 0);
> + if (!IS_ERR(tfm))
> + comp->tfm_noctx = tfm;
> +
>   return comp;
>  }
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 4f9df8e..c76d8e4 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -26,6 +26,7 @@ struct zcomp_strm {
>  /* dynamic per-device compression frontend */
>  struct zcomp {
>   void *stream;
> + struct crypto_comp *tfm_noctx;
>   const char *backend;
>  
>   struct zcomp_strm *(*strm_find)(struct zcomp *comp);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Sergey Senozhatsky]

On (09/18/15 14:19), Joonsoo Kim wrote:
> -/* 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;
> +
>   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, );
> + }

unneeded braces.

-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Minchan Kim]

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  1163.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.

> +
>   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, );
> + }
> +
>   return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
>  }
>  
>  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.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Minchan Kim]

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  1163.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.

> +
>   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, );
> + }
> +
>   return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
>  }
>  
>  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.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 9/9] zram: use crypto decompress_noctx API

[Sergey Senozhatsky]

On (09/18/15 14:19), Joonsoo Kim wrote:
> -/* 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;
> +
>   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, );
> + }

unneeded braces.

-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

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

Read10411701.88 6426911.62  9423894.38
Re-read 10017386.62 6428218.88  1163.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;
+
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, );
+   }
+
return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
 }
 
 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.
+*/
+   tfm = crypto_alloc_comp_noctx(compress, 0, 0);
+   if (!IS_ERR(tfm))
+   comp->tfm_noctx = tfm;
+
return comp;
 }
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 4f9df8e..c76d8e4 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -26,6 +26,7 @@ struct zcomp_strm {
 /* dynamic per-device compression frontend */
 struct zcomp {
void *stream;
+   struct crypto_comp *tfm_noctx;
const char *backend;
 
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

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

Read10411701.88 6426911.62  9423894.38
Re-read 10017386.62 6428218.88  1163.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;
+
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, );
+   }
+
return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
 }
 
 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.
+*/
+   tfm = crypto_alloc_comp_noctx(compress, 0, 0);
+   if (!IS_ERR(tfm))
+   comp->tfm_noctx = tfm;
+
return comp;
 }
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 4f9df8e..c76d8e4 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -26,6 +26,7 @@ struct zcomp_strm {
 /* dynamic per-device compression frontend */
 struct zcomp {
void *stream;
+   struct crypto_comp *tfm_noctx;
const char *backend;
 
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/