Re: [PATCH v4 8/8] zram: enable contextless compression alg in zram

[Joonsoo Kim]

On Thu, Oct 15, 2015 at 09:29:03AM +0900, Sergey Senozhatsky wrote:
> Hi,
> 
> On (10/14/15 16:38), Joonsoo Kim wrote:
> [..]
> >  static const char * const backends[] = {
> >  	"lzo",
> > -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> >  	"lz4",
> > -#endif
> >  	NULL
> >  };
> >  
> >  static const char *find_backend(const char *compress)
> >  {
> > -	int i = 0;
> > -	while (backends[i]) {
> > -		if (sysfs_streq(compress, backends[i]) &&
> > -			crypto_has_comp(backends[i], 0, 0))
> > -			break;
> > -		i++;
> > -	}
> > -	return backends[i];
> > +	if (crypto_has_comp(compress, 0, 0))
> > +		return compress;
> > +
> > +	return NULL;
> >  }
> >  
> >  static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> > @@ -277,6 +271,9 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
> >  	int i = 0;
> >  
> >  	while (backends[i]) {
> > +		if (!crypto_has_comp(backends[i], 0, 0))
> > +			continue;
> > +
> 
> hm... this sort of looks a bit `unnatural' to me. we have two _independent_
> sets -- what zram supports and what crypto supports. that's why you have
> to do extra work and consult crypto. can we return back the old scheme:
> use ifdef CONFIG in backends, but replace CONFIG_ZRAM with CONFIG_CRYPTO?
> 
> e.g.
> 
> 	static const char * const backends[] = {
> 		"lzo",
> 	#ifdef CONFIG_CRYPTO_LZ4
> 		"lz4",
> 	#endif
> 		NULL
> 	};
> 
> 
> so you can remove `crypto_has_comp(backends[i], 0, 0)' from
> zcomp_available_show(), because zram will support *only* what
> crypto supports.

Hello, Sergey.

Okay. I will change it in next spin.

Anyway, now I noticed that crypto_has_comp() is not proper API to
check contextless compression algorithm. I will change it, too.

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