Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1232576 > unrolled thread

Re: [PATCH v3 8/9] zram: use crypto API for compression

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 8/9] zram: use crypto API for compression Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-09-25 07:50 +0200
    Re: [PATCH v3 8/9] zram: use crypto API for compression Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-09-25 09:50 +0200

#1232576 — Re: [PATCH v3 8/9] zram: use crypto API for compression

On Mon, Sep 21, 2015 at 02:19:03PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > -static struct zcomp_backend *find_backend(const char *compress)
> > +static const char *find_backend(const char *compress)
> >  {
> >  	int i = 0;
> >  	while (backends[i]) {
> > -		if (sysfs_streq(compress, backends[i]->name))
> > +		if (sysfs_streq(compress, backends[i]) &&
> > +			crypto_has_comp(compress, 0, 0))
> 
> ok, just for note. zcomp does sysfs_streq(), because sysfs data
> usually contain a trailing new line, crypto_has_comp() does strcmp().

Okay. Will check.

> 
> 
> >  int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > -		const unsigned char *src, size_t *dst_len)
> > +		const unsigned char *src, unsigned int *dst_len)
> >  {
> > -	return comp->backend->compress(src, zstrm->buffer, dst_len,
> > -			zstrm->private);
> > +	*dst_len = PAGE_SIZE << 1;
> > +
> 
> hm... wouldn't it be better to say crypto api that we have a PAGE_SIZE
> buffer instead of PAGE_SIZE << 1, so in case of buffer overrun (or
> whatever is the correct term here) it will stop compression earlier
> (well, possibly)? zram will drop compressed data larger than PAGE_SIZE
> anyway, so if passing a smaller buffer size can save us CPU time then
> let's do it.

It can be implemented and maybe good way to go. But, in this patchset,
it isn't needed. It is better to do in separate patch.

> 
> > +	return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
> > +					zstrm->buffer, dst_len);
> >  }
> >  
> >  int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> >  		const unsigned char *src,
> > -		size_t src_len, unsigned char *dst)
> > +		unsigned int src_len, unsigned char *dst)
> >  {
> > -	return comp->backend->decompress(src, src_len, dst);
> > +	unsigned int size = PAGE_SIZE;
> > +
> > +	return crypto_comp_decompress(zstrm->tfm, src, src_len,	dst, &size);
> 
> 							^^^^^^^^ tab?
> 
> >  }
> >  
> >  void zcomp_destroy(struct zcomp *comp)
> > @@ -359,7 +365,7 @@ void zcomp_destroy(struct zcomp *comp)
> >  struct zcomp *zcomp_create(const char *compress, int max_strm)
> >  {
> >  	struct zcomp *comp;
> > -	struct zcomp_backend *backend;
> > +	const char *backend;
> 
> rebase against the current linux-next. this and the next patch do not
> apply cleanly. the function was touched recently: '+int error'.

Okay.

> 
> >  
> >  	backend = find_backend(compress);
> >  	if (!backend)
> > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> > index 4c09c01..4f9df8e 100644
> > --- a/drivers/block/zram/zcomp.h
> > +++ b/drivers/block/zram/zcomp.h
> > @@ -11,38 +11,22 @@
> >  #define _ZCOMP_H_
> >  
> >  #include <linux/mutex.h>
> > +#include <linux/crypto.h>
> >  
> >  struct zcomp_strm {
> > +	struct crypto_comp *tfm;
> > +
> >  	/* compression/decompression buffer */
> >  	void *buffer;
> > -	/*
> > -	 * The private data of the compression stream, only compression
> > -	 * stream backend can touch this (e.g. compression algorithm
> > -	 * working memory)
> > -	 */
> > -	void *private;
> > +
> >  	/* used in multi stream backend, protected by backend strm_lock */
> >  	struct list_head list;
> >  };
> >  
> > -/* static compression backend */
> > -struct zcomp_backend {
> > -	int (*compress)(const unsigned char *src, unsigned char *dst,
> > -			size_t *dst_len, void *private);
> > -
> > -	int (*decompress)(const unsigned char *src, size_t src_len,
> > -			unsigned char *dst);
> > -
> > -	void *(*create)(void);
> > -	void (*destroy)(void *private);
> > -
> > -	const char *name;
> > -};
> > -
> >  /* dynamic per-device compression frontend */
> >  struct zcomp {
> >  	void *stream;
> > -	struct zcomp_backend *backend;
> > +	const char *backend;
> 
> 	^^ what for?

Will remove.

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

[toc] | [next] | [standalone]

#1232632

On (09/25/15 14:43), Joonsoo Kim wrote:
[..]
> > >  /* dynamic per-device compression frontend */
> > >  struct zcomp {
> > >  	void *stream;
> > > -	struct zcomp_backend *backend;
> > > +	const char *backend;
> > 
> > 	^^ what for?
> 
> Will remove.


I think that was my mistake, I realized why do you keep it
later -- to allocate new zstreams. let's keep it.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web