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 <sergey.senozhat...@gmail.com>
> Cc: Minchan Kim <minc...@kernel.org>
> Cc: Joonsoo Kim <iamjoonsoo....@lge.com>
> ---
> 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 <linux/spinlock.h>
> +#include <linux/rwsem.h>
> #include <linux/zsmalloc.h>
> +#include <linux/crypto.h>
>
> #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
>
