Re: [PATCH 5/7] zram: use crypto api to check alg availability

[Minchan Kim]

On Fri, May 27, 2016 at 04:50:52PM +0900, Sergey Senozhatsky wrote:
> On (05/27/16 13:43), Minchan Kim wrote:
> [..]
> > >  modprobe zram
> > >  cat /proc/crypto | grep -i lz4
> > >  modprobe lz4
> > >  cat /proc/crypto | grep -i lz4
> > > name         : lz4
> > > driver       : lz4-generic
> > > module       : lz4
> > > 
> > > So the user can't tell exactly if the lz4 is really supported
> > > from /proc/crypto output, unless someone or something has loaded
> > > it.
> > > 
> > > This patch also adds crypto_has_comp() to zcomp_available_show().
> > 
> > crypto_has_comp works regardless of that whether module is loading or not?
> > IOW, currently, if lz4 modules is not loading, but crypto_has_comp return
> > true about lz4 module.
> > Right?
> 
> correct. crypto_has_comp() regardless the module being loaded.
> 
> # modprobe zram
> # cat /proc/crypto | egrep -e "lzo|lz4|deflate"
> 
> # echo lzo > /sys/block/zram0/comp_algorithm 
> # cat /proc/crypto | egrep -e "lzo|lz4|deflate"
> name         : lzo
> driver       : lzo-generic
> module       : lzo
> 
> # echo lz4 > /sys/block/zram0/comp_algorithm 
> # cat /proc/crypto | egrep -e "lzo|lz4|deflate"
> name         : lz4
> driver       : lz4-generic
> module       : lz4
> name         : lzo
> driver       : lzo-generic
> module       : lzo
> 
> # echo deflate > /sys/block/zram0/comp_algorithm 
> # cat /proc/crypto | egrep -e "lzo|lz4|deflate"
> name         : deflate
> driver       : deflate-generic
> module       : deflate
> name         : lz4
> driver       : lz4-generic
> module       : lz4
> name         : lzo
> driver       : lzo-generic
> module       : lzo
> 
> 
> whab it does, tho, it modprobs() the module upon the first
> request:
> 
> crypto_has_comp(...)
> 	crypto_has_alg()
> 		crypto_alg_mod_lookup()
> 			crypto_larval_lookup()
> 				request_module("crypto-%s", name)
> 					__request_module()
> 						call_modprobe()
> 
> and this is when /proc/crypto is getting updated. otherwise user has
> no information (well, unless modules were loaded by something/someome
> else, or crypto compressors were built-in into the kernel). I'm not
> aware of any other way to achieve this functionality for zram.

Now I got it. Thanks for spending time for me.

> 
> 
> > > We store all the compression algorithms names in zcomp's `backends'
> > > array, regardless the CONFIG_CRYPTO_FOO configuration, but show
> > 
> > Then, you mean we should add new string into backend array whenever
> > adding new crypto compatible compression algorithm?
> 
> yes. which looks quite trivial: adding or removing a string to/from
> the array.

Cc'ing Herbert.

Yes, it might be trivial to adding new "string" into the backend array
if we consider frequency of adding new compressoin algorithm in linux
but it would be better if we can get names of supported compression
algorithm name by crypto API.

If it's not good idea or something hard to implement, let's go with
hardcoding.

Herbert, Could you give us thought?

> 
> 
> > >  cat /proc/crypto | grep -i lz4
> > > 
> > >  cat /sys/block/zram0/comp_algorithm
> > > [lzo] lz4 deflate lz4hc 842
> > 
> > So, when lzo module is loading?
> 
> when we execute crypto_has_comp("lzo") for the first time.
> 
> 
> that's why doing just
> 
> # modprobe zram
> 
> will not cause /proc/crypto update
> 
> 
> # modprobe zram
> # cat /proc/crypto | egrep -e "lzo|lz4|deflate"
> 
> 
> crypto_has_comp() updates it -- when we read or write
> from/to comp_algorithm:
> 
> # cat /sys/block/zram0/comp_algorithm
> [lzo] lz4 deflate lz4hc 842
> 
> # cat /proc/crypto | egrep -e "lzo|lz4|deflate"
> name         : lzo
> driver       : lzo-generic
> module       : lzo
> 
> 
> but I didn't want zram to depend on this, or to depend on
> /proc/crypto content; that's why I did it the way it is.
> 
> 	-ss