From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756748AbcFAAC3 (ORCPT ); Tue, 31 May 2016 20:02:29 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:39186 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751857AbcFAAC2 (ORCPT ); Tue, 31 May 2016 20:02:28 -0400 X-Original-SENDERIP: 156.147.1.125 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 165.244.98.76 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 10.177.223.161 X-Original-MAILFROM: minchan@kernel.org Date: Wed, 1 Jun 2016 09:03:04 +0900 From: Minchan Kim To: Sergey Senozhatsky CC: Andrew Morton , Joonsoo Kim , , Sergey Senozhatsky Subject: Re: [PATCH v2 4/8] zram: use crypto api to check alg availability Message-ID: <20160601000304.GF19976@bbox> References: <20160531122017.2878-1-sergey.senozhatsky@gmail.com> <20160531122017.2878-5-sergey.senozhatsky@gmail.com> MIME-Version: 1.0 In-Reply-To: <20160531122017.2878-5-sergey.senozhatsky@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/06/01 09:02:24, Serialize by Router on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/06/01 09:02:24, Serialize complete at 2016/06/01 09:02:24 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 Tue, May 31, 2016 at 09:20:13PM +0900, Sergey Senozhatsky wrote: > There is no way to get a string with all the crypto comp > algorithms supported by the crypto comp engine, so we need > to maintain our own backends list. At the same time we > additionally need to use crypto_has_comp() to make sure > that the user has requested a compression algorithm that is > recognized by the crypto comp engine. Relying on /proc/crypto > is not an options here, because it does not show not-yet-inserted > compression modules. > > Example: > > 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(). > We store all the compression algorithms names in zcomp's `backends' > array, regardless the CONFIG_CRYPTO_FOO configuration, but show > only those that are also supported by crypto engine. This helps > user to know the exact list of compression algorithms that can be > used. 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? > > Example: > module lz4 is not loaded yet, but is supported by the crypto > engine. /proc/crypto has no information on this module, while > zram's `comp_algorithm' lists it: > > cat /proc/crypto | grep -i lz4 > > cat /sys/block/zram0/comp_algorithm > [lzo] lz4 deflate lz4hc 842 > > We also now fully rely on crypto_has_comp() when configure a new > device. The existing `backends' array is kept for user's convenience > only -- there is no way to list all of the compression algorithms > supported by crypto -- and is not guaranteed to contain every > compression module name supported by the kernel. Switch to > crypto_has_comp() has an advantage of permitting the usage of > out-of-tree crypto compression modules (implementing S/W or H/W > compression). If user load out-of-tree crypto compression module, what's status of comp_algorithm? #> insmod foo_crypto.ko #> echo foo > /sys/block/zram0/comp_algorithm #> cat /sys/block/zram0/comp_algorithm lzo lz4 [foo] ? > > Signed-off-by: Sergey Senozhatsky > Cc: Minchan Kim > Cc: Joonsoo Kim > --- > Documentation/blockdev/zram.txt | 11 ++++++++ > drivers/block/zram/zcomp.c | 58 ++++++++++++++++++++++++----------------- > drivers/block/zram/zram_drv.c | 16 +++++++----- > drivers/block/zram/zram_drv.h | 5 ++-- > 4 files changed, 57 insertions(+), 33 deletions(-) > > diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt > index 13100fb..7c05357 100644 > --- a/Documentation/blockdev/zram.txt > +++ b/Documentation/blockdev/zram.txt > @@ -83,6 +83,17 @@ pre-created. Default: 1. > #select lzo compression algorithm > echo lzo > /sys/block/zram0/comp_algorithm > > + For the time being, the `comp_algorithm' content does not necessarily > + show every compression algorithm supported by the kernel. We keep this > + list primarily to simplify device configuration and one can configure > + a new device with a compression algorithm that is not listed in > + `comp_algorithm'. The thing is that, internally, ZRAM uses Crypto API > + and, if some of the algorithms were built as modules, it's impossible > + to list all of them using, for instance, /proc/crypto or any other > + method. This, however, has an advantage of permitting the usage of > + custom crypto compression modules (implementing S/W or H/W > + compression). > + > 4) Set Disksize > Set disk size by writing the value to sysfs node 'disksize'. > The value can be either in bytes or you can use mem suffixes. > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index f357268..2381ca9 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -26,17 +26,6 @@ static const char * const backends[] = { > NULL > }; > > -static const char *find_backend(const char *compress) > -{ > - int i = 0; > - while (backends[i]) { > - if (sysfs_streq(compress, backends[i])) > - break; > - i++; > - } > - return backends[i]; > -} > - > static void zcomp_strm_free(struct zcomp_strm *zstrm) > { > if (!IS_ERR_OR_NULL(zstrm->tfm)) > @@ -68,30 +57,53 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) > return zstrm; > } > > +bool zcomp_available_algorithm(const char *comp) > +{ > + /* > + * Crypto does not ignore a trailing new line symbol, > + * so make sure you don't supply a string containing > + * one. > + * This also means that we keep `backends' array for > + * zcomp_available_show() only and will init a new zram > + * device with any compressing algorithm known to crypto > + * api. > + */ > + return crypto_has_comp(comp, 0, 0) == 1; > +} > + > /* show available compressors */ > ssize_t zcomp_available_show(const char *comp, char *buf) > { > + bool known_algorithm = false; > ssize_t sz = 0; > int i = 0; > > - while (backends[i]) { > - if (!strcmp(comp, backends[i])) > + for (; backends[i]; i++) { > + if (!zcomp_available_algorithm(backends[i])) > + continue; > + > + if (!strcmp(comp, backends[i])) { > + known_algorithm = true; > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > "[%s] ", backends[i]); > - else > + } else { > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > "%s ", backends[i]); > - i++; > + } > } > + > + /* > + * Out-of-tree module known to crypto api or a missing > + * entry in `backends'. > + */ > + if (!known_algorithm && zcomp_available_algorithm(comp)) > + sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > + "[%s] ", comp); > + > sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n"); > return sz; > } > > -bool zcomp_available_algorithm(const char *comp) > -{ > - return find_backend(comp) != NULL; > -} > - > struct zcomp_strm *zcomp_stream_get(struct zcomp *comp) > { > return *get_cpu_ptr(comp->stream); > @@ -227,18 +239,16 @@ void zcomp_destroy(struct zcomp *comp) > struct zcomp *zcomp_create(const char *compress) > { > struct zcomp *comp; > - const char *backend; > int error; > > - backend = find_backend(compress); > - if (!backend) > + if (!zcomp_available_algorithm(compress)) > return ERR_PTR(-EINVAL); > > comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL); > if (!comp) > return ERR_PTR(-ENOMEM); > > - comp->name = backend; > + comp->name = compress; > error = zcomp_init(comp); > if (error) { > kfree(comp); > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index 65d1403..c2a1d7d 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -342,9 +342,16 @@ static ssize_t comp_algorithm_store(struct device *dev, > struct device_attribute *attr, const char *buf, size_t len) > { > struct zram *zram = dev_to_zram(dev); > + char compressor[CRYPTO_MAX_ALG_NAME]; > size_t sz; > > - if (!zcomp_available_algorithm(buf)) > + strlcpy(compressor, buf, sizeof(compressor)); > + /* ignore trailing newline */ > + sz = strlen(compressor); > + if (sz > 0 && compressor[sz - 1] == '\n') > + compressor[sz - 1] = 0x00; > + > + if (!zcomp_available_algorithm(compressor)) > return -EINVAL; > > down_write(&zram->init_lock); > @@ -353,13 +360,8 @@ static ssize_t comp_algorithm_store(struct device *dev, > pr_info("Can't change algorithm for initialized device\n"); > return -EBUSY; > } > - strlcpy(zram->compressor, buf, sizeof(zram->compressor)); > - > - /* ignore trailing newline */ > - sz = strlen(zram->compressor); > - if (sz > 0 && zram->compressor[sz - 1] == '\n') > - zram->compressor[sz - 1] = 0x00; > > + strlcpy(zram->compressor, compressor, sizeof(compressor)); > up_write(&zram->init_lock); > return len; > } > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h > index 3f5bf66..74fcf10 100644 > --- a/drivers/block/zram/zram_drv.h > +++ b/drivers/block/zram/zram_drv.h > @@ -15,8 +15,9 @@ > #ifndef _ZRAM_DRV_H_ > #define _ZRAM_DRV_H_ > > -#include > +#include > #include > +#include > > #include "zcomp.h" > > @@ -113,7 +114,7 @@ struct zram { > * we can store in a disk. > */ > u64 disksize; /* bytes */ > - char compressor[10]; > + char compressor[CRYPTO_MAX_ALG_NAME]; > /* > * zram is claimed so open request will be failed > */ > -- > 2.8.3.394.g3916adf >