From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757415AbcFAGqc (ORCPT ); Wed, 1 Jun 2016 02:46:32 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:35661 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751197AbcFAGqb (ORCPT ); Wed, 1 Jun 2016 02:46:31 -0400 X-Original-SENDERIP: 156.147.1.126 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 165.244.98.203 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 10.177.223.161 X-Original-MAILFROM: minchan@kernel.org Date: Wed, 1 Jun 2016 15:47:09 +0900 From: Minchan Kim To: Sergey Senozhatsky CC: Sergey Senozhatsky , Andrew Morton , Joonsoo Kim , Subject: Re: [PATCH v2 4/8] zram: use crypto api to check alg availability Message-ID: <20160601064709.GM19976@bbox> References: <20160531122017.2878-1-sergey.senozhatsky@gmail.com> <20160531122017.2878-5-sergey.senozhatsky@gmail.com> <20160601000304.GF19976@bbox> <20160601010707.GB461@swordfish> <20160601022708.GL19976@bbox> <20160601025236.GG461@swordfish> MIME-Version: 1.0 In-Reply-To: <20160601025236.GG461@swordfish> User-Agent: Mutt/1.5.21 (2010-09-15) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB02/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/06/01 15:46:27, Serialize by Router on LGEKRMHUB02/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/06/01 15:46:27, Serialize complete at 2016/06/01 15:46:27 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 01, 2016 at 12:17:35PM +0900, Sergey Senozhatsky wrote: > On (06/01/16 11:27), Minchan Kim wrote: > [..] > > > > So, if we do 'cat /sys/block/zram0/comp_algorithm", every crypto modules > > > > in the backend array are loaded in memory and not unloaded until admin > > > > executes rmmod? Right? > > > > > > yes, I think so. > > > > It scares me. Common case, except one we choosed, every loaded modules > > will be not used. I think it's really not good. Although the wastage > > might be not big now, it will be heavy as crypto comp modules are > > increased. > > well... if you have those modules enabled then you somehow expect > them to be loaded at some point, if not by zram, then by something > else (networking, etc.). /* not speaking of the systems that have > those modules built-in */ I'm not saying that what we have is > optimal, of course, but it's not so senseless at the same time. In my local system, there are a lot of modules I am not using but distro installed for some point but I believe that some point will not come. IMO, they shouldn't be loaded by the reason they will be used some point, potentially. > > > > What do you think about it? > > I can do something like this: > > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index 1a4bd20..9b704cc 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -20,10 +20,18 @@ > > static const char * const backends[] = { > "lzo", > +#if IS_ENABLED(CONFIG_CRYPTO_LZ4) > "lz4", > +#endif > +#if IS_ENABLED(CONFIG_CRYPTO_DEFLATE) > "deflate", > +#endif > +#if IS_ENABLED(CONFIG_CRYPTO_LZ4HC) > "lz4hc", > +#endif > +#if IS_ENABLED(CONFIG_CRYPTO_842) > "842", > +#endif > NULL > }; > > > so both BUILTIN and BUILT-AS-A-MODULE cases are handled at compile > time now and we can avoid crypto_has_comp() checks for most of the > comp_algorithm calls, except for the case when someone requests an > out-of-tree module. Hmm, isn't it problem, either? That module was built but not installed. In that case, setting the algorithm will be failed. IOW, we are lying to user. For solving the problem, if we check it with crypto_has_comp, again, it will load module into memory. :( 1) Use IS_BUILTIN, not IS_ENABLED for backend For module-crypto, user should set directly without supporting from backend. 2) Deprecated /sys/block/zram0/comp_alrogithm totally Admin should set algorithm by himself like zswap and other cyrpto users. 3) Supporting from zramctl. Maybe, we can teach zramctl so zramctl can find /lib/modules/`uname-r`/ into not-yet-known-modules and use lsmod to find loaded-known-module for zram to use compression algorithm. So, 1,3 combination can be or 2,3 combination can be. Welcome other suggestion. > > -ss