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

[Sergey Senozhatsky]

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.

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

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>
---
drivers/block/zram/zcomp.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 79b30d7..a8593e9 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -29,12 +29,16 @@ static const char * const backends[] = {
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];
+
+ if (backends[i] && crypto_has_comp(backends[i], 0, 0))
+ return backends[i];
+ return NULL;
}

static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
@@ -74,14 +78,16 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
ssize_t sz = 0;
int i = 0;

- while (backends[i]) {
+ for (; backends[i]; i++) {
+ if (!crypto_has_comp(backends[i], 0, 0))
+ continue;
+
if (!strcmp(comp, backends[i]))
sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
"[%s] ", backends[i]);
else
sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
"%s ", backends[i]);
- i++;
}
sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
return sz;
--
2.8.3.394.g3916adf

[Sergey Senozhatsky]

Hello,

This has started as a 'add zlib support' work, but after some
thinking I saw no blockers for a bigger change -- a switch to
crypto API.

We don't have an idle zstreams list anymore and our write path
now works absolutely differently, preventing preemption during
compression. This removes possibilities of read paths preempting
writes at wrong places and opens the door for a move from custom
LZO/LZ4 compression backends implementation to a more generic one,
using crypto compress API.

This patch set also eliminates the need of a new context-less
crypto API interface, which was quite hard to sell, so we can
move along faster.


Sergey Senozhatsky (7):
zram: rename zstrm find-release functions
zram: switch to crypto compress API
zram: drop zcomp param from compress/decompress
zram: align zcomp interface to crypto comp API
zram: use crypto api to check alg availability
zram: delete custom lzo/lz4
zram: add more compression algorithms

drivers/block/zram/Kconfig | 15 +------
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 91 +++++++++++++++++++++++++++---------------
drivers/block/zram/zcomp.h | 29 ++++----------
drivers/block/zram/zcomp_lz4.c | 56 --------------------------
drivers/block/zram/zcomp_lz4.h | 17 --------
drivers/block/zram/zcomp_lzo.c | 56 --------------------------
drivers/block/zram/zcomp_lzo.h | 17 --------
drivers/block/zram/zram_drv.c | 26 +++++++-----
9 files changed, 84 insertions(+), 227 deletions(-)
delete mode 100644 drivers/block/zram/zcomp_lz4.c
delete mode 100644 drivers/block/zram/zcomp_lz4.h
delete mode 100644 drivers/block/zram/zcomp_lzo.c
delete mode 100644 drivers/block/zram/zcomp_lzo.h

--
2.8.3.394.g3916adf

[Sergey Senozhatsky]

We don't perform any zstream idle list lookup anymore, so
zcomp_strm_find()/zcomp_strm_release() names are not
representative.

Rename to zcomp_stream_get()/zcomp_stream_put().

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>
---

drivers/block/zram/zcomp.c | 4 ++--
drivers/block/zram/zcomp.h | 4 ++--
drivers/block/zram/zram_drv.c | 8 ++++----
3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index b51a816..400f826 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -95,12 +95,12 @@ bool zcomp_available_algorithm(const char *comp)
return find_backend(comp) != NULL;
}

-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
+struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
{
return *get_cpu_ptr(comp->stream);
}

-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
+void zcomp_stream_put(struct zcomp *comp)
{
put_cpu_ptr(comp->stream);
}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index ffd88cb..944b8e6 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -48,8 +48,8 @@ bool zcomp_available_algorithm(const char *comp);
struct zcomp *zcomp_create(const char *comp);
void zcomp_destroy(struct zcomp *comp);

-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
+struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
+void zcomp_stream_put(struct zcomp *comp);

int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src, size_t *dst_len);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 8fcad8b..9361a5d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -695,7 +695,7 @@ compress_again:
goto out;
}

- zstrm = zcomp_strm_find(zram->comp);
+ zstrm = zcomp_stream_get(zram->comp);
ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen);
if (!is_partial_io(bvec)) {
kunmap_atomic(user_mem);
@@ -734,7 +734,7 @@ compress_again:
__GFP_NOWARN |
__GFP_HIGHMEM);
if (!handle) {
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_stream_put(zram->comp);
zstrm = NULL;

atomic64_inc(&zram->stats.writestall);
@@ -769,7 +769,7 @@ compress_again:
memcpy(cmem, src, clen);
}

- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_stream_put(zram->comp);
zstrm = NULL;
zs_unmap_object(meta->mem_pool, handle);

@@ -789,7 +789,7 @@ compress_again:
atomic64_inc(&zram->stats.pages_stored);
out:
if (zstrm)
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_stream_put(zram->comp);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
--
2.8.3.394.g3916adf

[Sergey Senozhatsky]

Remove lzo/lz4 backends, we use crypto API now.

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>
---

drivers/block/zram/Kconfig | 9 -------
drivers/block/zram/Makefile | 4 +--
drivers/block/zram/zcomp.c | 2 --
drivers/block/zram/zcomp.h | 15 -----------
drivers/block/zram/zcomp_lz4.c | 56 ------------------------------------------
drivers/block/zram/zcomp_lz4.h | 17 -------------
drivers/block/zram/zcomp_lzo.c | 56 ------------------------------------------
drivers/block/zram/zcomp_lzo.h | 17 -------------
8 files changed, 1 insertion(+), 175 deletions(-)

delete mode 100644 drivers/block/zram/zcomp_lz4.c
delete mode 100644 drivers/block/zram/zcomp_lz4.h
delete mode 100644 drivers/block/zram/zcomp_lzo.c
delete mode 100644 drivers/block/zram/zcomp_lzo.h

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 2252cd7..b8ecba6 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -13,12 +13,3 @@ config ZRAM
disks and maybe many more.

See zram.txt for more information.
-
-config ZRAM_LZ4_COMPRESS
- bool "Enable LZ4 algorithm support"
- depends on ZRAM
- select CRYPTO_LZ4
- default n
- help
- This option enables LZ4 compression algorithm support. Compression
- algorithm can be changed using `comp_algorithm' device attribute.
diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
index be0763f..9e2b79e 100644
--- a/drivers/block/zram/Makefile
+++ b/drivers/block/zram/Makefile
@@ -1,5 +1,3 @@
-zram-y := zcomp_lzo.o zcomp.o zram_drv.o
-
-zram-$(CONFIG_ZRAM_LZ4_COMPRESS) += zcomp_lz4.o
+zram-y := zcomp.o zram_drv.o

obj-$(CONFIG_ZRAM) += zram.o
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index a8593e9..5ec0eb2 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -20,9 +20,7 @@

static const char * const backends[] = {

"lzo",
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
"lz4",
-#endif
NULL
};

diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index dcc0951..3cb13f4 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -21,24 +21,9 @@ struct zcomp_strm {
void *private;
};

-/* static compression backend */
-struct zcomp_backend {
- int (*compress)(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private);
-
- int (*decompress)(const unsigned char *src, size_t src_len,
- unsigned char *dst);
-
- void *(*create)(gfp_t flags);
- void (*destroy)(void *private);
-
- const char *name;
-};
-
/* dynamic per-device compression frontend */
struct zcomp {
struct zcomp_strm * __percpu *stream;
- struct zcomp_backend *backend;
struct notifier_block notifier;

const char *name;
diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
deleted file mode 100644
index 0110086..0000000
--- a/drivers/block/zram/zcomp_lz4.c
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/lz4.h>
-#include <linux/vmalloc.h>
-#include <linux/mm.h>
-
-#include "zcomp_lz4.h"
-
-static void *zcomp_lz4_create(gfp_t flags)
-{
- void *ret;
-
- ret = kmalloc(LZ4_MEM_COMPRESS, flags);
- if (!ret)
- ret = __vmalloc(LZ4_MEM_COMPRESS,
- flags | __GFP_HIGHMEM,
- PAGE_KERNEL);
- return ret;
-}
-
-static void zcomp_lz4_destroy(void *private)
-{
- kvfree(private);
-}
-
-static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private)
-{
- /* return : Success if return 0 */
- return lz4_compress(src, PAGE_SIZE, dst, dst_len, private);
-}
-
-static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
-{
- size_t dst_len = PAGE_SIZE;
- /* return : Success if return 0 */
- return lz4_decompress_unknownoutputsize(src, src_len, dst, &dst_len);
-}
-
-struct zcomp_backend zcomp_lz4 = {
- .compress = zcomp_lz4_compress,
- .decompress = zcomp_lz4_decompress,
- .create = zcomp_lz4_create,
- .destroy = zcomp_lz4_destroy,
- .name = "lz4",
-};
diff --git a/drivers/block/zram/zcomp_lz4.h b/drivers/block/zram/zcomp_lz4.h
deleted file mode 100644
index 60613fb..0000000
--- a/drivers/block/zram/zcomp_lz4.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _ZCOMP_LZ4_H_
-#define _ZCOMP_LZ4_H_
-
-#include "zcomp.h"
-
-extern struct zcomp_backend zcomp_lz4;
-
-#endif /* _ZCOMP_LZ4_H_ */
diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
deleted file mode 100644
index ed7a1f0..0000000
--- a/drivers/block/zram/zcomp_lzo.c
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/lzo.h>
-#include <linux/vmalloc.h>
-#include <linux/mm.h>
-
-#include "zcomp_lzo.h"
-
-static void *lzo_create(gfp_t flags)
-{
- void *ret;
-
- ret = kmalloc(LZO1X_MEM_COMPRESS, flags);
- if (!ret)
- ret = __vmalloc(LZO1X_MEM_COMPRESS,
- flags | __GFP_HIGHMEM,
- PAGE_KERNEL);
- return ret;
-}
-
-static void lzo_destroy(void *private)
-{
- kvfree(private);
-}
-
-static int lzo_compress(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private)
-{
- int ret = lzo1x_1_compress(src, PAGE_SIZE, dst, dst_len, private);
- return ret == LZO_E_OK ? 0 : ret;
-}
-
-static int lzo_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
-{
- size_t dst_len = PAGE_SIZE;
- int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len);
- return ret == LZO_E_OK ? 0 : ret;
-}
-
-struct zcomp_backend zcomp_lzo = {
- .compress = lzo_compress,
- .decompress = lzo_decompress,
- .create = lzo_create,
- .destroy = lzo_destroy,
- .name = "lzo",
-};
diff --git a/drivers/block/zram/zcomp_lzo.h b/drivers/block/zram/zcomp_lzo.h
deleted file mode 100644
index 128c580..0000000
--- a/drivers/block/zram/zcomp_lzo.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _ZCOMP_LZO_H_
-#define _ZCOMP_LZO_H_
-
-#include "zcomp.h"
-
-extern struct zcomp_backend zcomp_lzo;
-
-#endif /* _ZCOMP_LZO_H_ */
--
2.8.3.394.g3916adf

[Joonsoo Kim]

On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote:
> Hello,
>
> This has started as a 'add zlib support' work, but after some
> thinking I saw no blockers for a bigger change -- a switch to
> crypto API.
>
> We don't have an idle zstreams list anymore and our write path
> now works absolutely differently, preventing preemption during
> compression. This removes possibilities of read paths preempting
> writes at wrong places and opens the door for a move from custom
> LZO/LZ4 compression backends implementation to a more generic one,
> using crypto compress API.

Hello, Sergey.

I don't look at each patches deeply but nice work! I didn't notice that
rececnt zram changes makes thing simpler. :)

Thanks.

[Minchan Kim]

On Wed, May 25, 2016 at 11:30:00PM +0900, Sergey Senozhatsky wrote:
> We don't perform any zstream idle list lookup anymore, so
> zcomp_strm_find()/zcomp_strm_release() names are not
> representative.
>
> Rename to zcomp_stream_get()/zcomp_stream_put().

Actually, I wanted it when we applied percpu but didn't say to you because

1. It's preference of author.

Frankly speaking, I prefer get to find but you might think different
with me so I want to respect patch author's right if it's not huge pain
to me. :)
Now I realized you were on same page.

2. We might roll back to stream list.

In that case, find is proper word again but it's too trivial.
So, I want to merge this patch regardless of this patchset. :)

>
> Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
> Cc: Minchan Kim <min...@kernel.org>

Acked-by: Minchan Kim <min...@kernel.org>

[Minchan Kim]

On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote:

> Hello,
>
> This has started as a 'add zlib support' work, but after some
> thinking I saw no blockers for a bigger change -- a switch to
> crypto API.
>
> We don't have an idle zstreams list anymore and our write path
> now works absolutely differently, preventing preemption during
> compression. This removes possibilities of read paths preempting
> writes at wrong places and opens the door for a move from custom
> LZO/LZ4 compression backends implementation to a more generic one,
> using crypto compress API.
>
> This patch set also eliminates the need of a new context-less
> crypto API interface, which was quite hard to sell, so we can
> move along faster.

Super fast Sergey.
At a first glance, patchset looks nice.
I will review as soon as possible.

Thanks a lot!

[Sergey Senozhatsky]

thanks.

-ss

[Sergey Senozhatsky]

On (05/26/16 09:52), Minchan Kim wrote:
> On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote:
> > Hello,
> >
> > This has started as a 'add zlib support' work, but after some
> > thinking I saw no blockers for a bigger change -- a switch to
> > crypto API.
> >
> > We don't have an idle zstreams list anymore and our write path
> > now works absolutely differently, preventing preemption during
> > compression. This removes possibilities of read paths preempting
> > writes at wrong places and opens the door for a move from custom
> > LZO/LZ4 compression backends implementation to a more generic one,
> > using crypto compress API.
> >
> > This patch set also eliminates the need of a new context-less
> > crypto API interface, which was quite hard to sell, so we can
> > move along faster.
>
> Super fast Sergey.

hahaha :)

> At a first glance, patchset looks nice.
> I will review as soon as possible.
>
> Thanks a lot!

thanks!

-ss

[Sergey Senozhatsky]

On (05/26/16 09:43), Joonsoo Kim wrote:
[..]

> Hello, Sergey.
>
> I don't look at each patches deeply but nice work! I didn't notice that
> rececnt zram changes makes thing simpler. :)

Hello Joonsoo,

thanks.

I owe you a drink for pushing it in the context-less crypto
API direction. sorry about that.

-ss

[Joonsoo Kim]

Good to hear. I will wait for your invitation. :)

Thanks.

[Minchan Kim]

Hello Sergey,

I want to know more how it works so below questions goes.

On Wed, May 25, 2016 at 11:30:04PM +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().

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?

> 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?

> only those that are also supported by crypto engine. This helps
> user to know the exact list of compression algorithms that can be
> used.
>
> 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

So, when lzo module is loading?

[Sergey Senozhatsky]

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


what is 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.

> > 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.

> > 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

[Minchan Kim]

> 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?

[Herbert Xu]

On Fri, May 27, 2016 at 05:27:44PM +0900, Minchan Kim wrote:
>
> 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?

It is fundamentally impossible to get a list of all *potential*
algorithms if you allow loadable modules. By definition someone
could load a new module and thus introduce a new algorithm.

Given a specific algorithm name you could determine whether it
is present on the system.

Cheers,
--
Email: Herbert Xu <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Minchan Kim]

On Fri, May 27, 2016 at 04:43:45PM +0800, Herbert Xu wrote:
> On Fri, May 27, 2016 at 05:27:44PM +0900, Minchan Kim wrote:
> >
> > 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?
>
> It is fundamentally impossible to get a list of all *potential*
> algorithms if you allow loadable modules. By definition someone
> could load a new module and thus introduce a new algorithm.

Thanks for the quick response. I understand now.

Sergey,

Then, shouldn't we add functionality to load new algorithm although
there is no entry in zcomp backend array?

The main reason we changed to crypto is to support various compression
algorithm for zram so we should be able to support anyone who want to
use custom crypto compression module.

A idea is that let's not adding new compression string into backend
array from now on and deprecates /zram/comp_algorithm to show name.
So, from now on, users should be aware of the compression naming and
load module by oneself. It's not kind but I hope admins already knows
what cryto compressor they have.

I don't have better idea.

[Sergey Senozhatsky]

Hello,

On (05/27/16 18:04), Minchan Kim wrote:
> > It is fundamentally impossible to get a list of all *potential*
> > algorithms if you allow loadable modules. By definition someone
> > could load a new module and thus introduce a new algorithm.
>
> Thanks for the quick response. I understand now.
>
> Sergey,
>
> Then, shouldn't we add functionality to load new algorithm although
> there is no entry in zcomp backend array?

can do, sure: make find_backend() depend on crypto_has_comp() only,
*may be* printk a message when we have a mismatch in backends array
and crypto_has_comp(). but not really sure about the latter one.

> The main reason we changed to crypto is to support various compression
> algorithm for zram so we should be able to support anyone who want to
> use custom crypto compression module.

yes.

> A idea is that let's not adding new compression string into backend
> array from now on and deprecates /zram/comp_algorithm to show name.

I'd really prefer not to deprecate /zram/comp_algorithm now.

> So, from now on, users should be aware of the compression naming and
> load module by oneself. It's not kind but I hope admins already knows
> what cryto compressor they have.

um, I'm not in love with this approach.

what admins? zram users may be completely unaware of the existence of
the crypto API and the kernel rebuild process. for some people switching
to new zram will be a matter of "apt-get upgrade/pacman -Syu", not
"git am ...; make menuconfig; make -j...". this looks like a major step
back. keeping this dummy list of compressing backends around does not take
a lot of effort, especially if we will make it unimportant for find_backend()
and only use it for that nice user-friendly `cat /zram/comp_algorithm`
output.

-ss

[Minchan Kim]

Hi Sergey,

On Sun, May 29, 2016 at 12:24:58PM +0900, Sergey Senozhatsky wrote:
> Hello,
>
> On (05/27/16 18:04), Minchan Kim wrote:
> > > It is fundamentally impossible to get a list of all *potential*
> > > algorithms if you allow loadable modules. By definition someone
> > > could load a new module and thus introduce a new algorithm.
> >
> > Thanks for the quick response. I understand now.
> >
> > Sergey,
> >
> > Then, shouldn't we add functionality to load new algorithm although
> > there is no entry in zcomp backend array?
>
> can do, sure: make find_backend() depend on crypto_has_comp() only,
> *may be* printk a message when we have a mismatch in backends array
> and crypto_has_comp(). but not really sure about the latter one.

Me, too. We don't need to warn about that. Instead, we should allow loading
other algorithm which doesn't live in backend array more clearly.
We need to update documentation to clear it out.

>
> > The main reason we changed to crypto is to support various compression
> > algorithm for zram so we should be able to support anyone who want to
> > use custom crypto compression module.
>
> yes.
>
> > A idea is that let's not adding new compression string into backend
> > array from now on and deprecates /zram/comp_algorithm to show name.
>
> I'd really prefer not to deprecate /zram/comp_algorithm now.
>
> > So, from now on, users should be aware of the compression naming and
> > load module by oneself. It's not kind but I hope admins already knows
> > what cryto compressor they have.
>
> um, I'm not in love with this approach.
>
> what admins? zram users may be completely unaware of the existence of
> the crypto API and the kernel rebuild process. for some people switching
> to new zram will be a matter of "apt-get upgrade/pacman -Syu", not
> "git am ...; make menuconfig; make -j...". this looks like a major step
> back. keeping this dummy list of compressing backends around does not take
> a lot of effort, especially if we will make it unimportant for find_backend()
> and only use it for that nice user-friendly `cat /zram/comp_algorithm`
> output.

I am not against with it. Instead, please clear it out to write down docuement
"comp_algorithm is just optional so user can load any new algorithm like this
blah blah"

Thanks.
>
> -ss

[Sergey Senozhatsky]

Hello Minchan,

On (05/30/16 13:47), Minchan Kim wrote:
[..]

> > can do, sure: make find_backend() depend on crypto_has_comp() only,
> > *may be* printk a message when we have a mismatch in backends array
> > and crypto_has_comp(). but not really sure about the latter one.
>
> Me, too. We don't need to warn about that. Instead, we should allow loading
> other algorithm which doesn't live in backend array more clearly.
> We need to update documentation to clear it out.

yes, entirely forgot to update the Documentation.

> > > So, from now on, users should be aware of the compression naming and
> > > load module by oneself. It's not kind but I hope admins already knows
> > > what cryto compressor they have.
> >
> > um, I'm not in love with this approach.
> >
> > what admins? zram users may be completely unaware of the existence of
> > the crypto API and the kernel rebuild process. for some people switching
> > to new zram will be a matter of "apt-get upgrade/pacman -Syu", not
> > "git am ...; make menuconfig; make -j...". this looks like a major step
> > back. keeping this dummy list of compressing backends around does not take
> > a lot of effort, especially if we will make it unimportant for find_backend()
> > and only use it for that nice user-friendly `cat /zram/comp_algorithm`
> > output.
>
> I am not against with it. Instead, please clear it out to write down docuement
> "comp_algorithm is just optional so user can load any new algorithm like this
> blah blah"

ok, will do.

thanks.

-ss

[Sergey Senozhatsky]

We don't perform any zstream idle list lookup anymore, so
zcomp_strm_find()/zcomp_strm_release() names are not
representative.

Rename to zcomp_stream_get()/zcomp_stream_put().

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>

Acked-by: Minchan Kim <min...@kernel.org>

---
drivers/block/zram/zcomp.c | 4 ++--
drivers/block/zram/zcomp.h | 4 ++--
drivers/block/zram/zram_drv.c | 8 ++++----

3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index b51a816..400f826 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c

@@ -95,12 +95,12 @@ bool zcomp_available_algorithm(const char *comp)
return find_backend(comp) != NULL;
}

-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
+struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
{
return *get_cpu_ptr(comp->stream);
}

-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
+void zcomp_stream_put(struct zcomp *comp)
{
put_cpu_ptr(comp->stream);
}

diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index ffd88cb..944b8e6 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h

[Sergey Senozhatsky]

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.

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).

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@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

[Sergey Senozhatsky]

Hello,

This has started as a 'add zlib support' work, but after some
thinking I saw no blockers for a bigger change -- a switch to
crypto API.

We don't have an idle zstreams list anymore and our write path
now works absolutely differently, preventing preemption during
compression. This removes possibilities of read paths preempting
writes at wrong places and opens the door for a move from custom
LZO/LZ4 compression backends implementation to a more generic one,
using crypto compress API.

This patch set also eliminates the need of a new context-less
crypto API interface, which was quite hard to sell, so we can
move along faster.

v2:
-- addressed Minchan's review points
-- allow out-of-tree comp algorithms, per Minchan
-- some other cleanups, reworks and improvements

Sergey Senozhatsky (8):

zram: rename zstrm find-release functions
zram: switch to crypto compress API

zram: align zcomp interface to crypto comp API
zram: use crypto api to check alg availability

zram: cosmetic: cleanup documentation

zram: delete custom lzo/lz4
zram: add more compression algorithms

zram: drop gfp_t from zcomp_strm_alloc()

Documentation/blockdev/zram.txt | 82 +++++++++++++----------
drivers/block/zram/Kconfig | 15 +----
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 143 ++++++++++++++++++++++++----------------
drivers/block/zram/zcomp.h | 36 +++-------
drivers/block/zram/zcomp_lz4.c | 56 ----------------
drivers/block/zram/zcomp_lz4.h | 17 -----
drivers/block/zram/zcomp_lzo.c | 56 ----------------
drivers/block/zram/zcomp_lzo.h | 17 -----
drivers/block/zram/zram_drv.c | 42 +++++++-----
drivers/block/zram/zram_drv.h | 5 +-
11 files changed, 171 insertions(+), 302 deletions(-)

[Sergey Senozhatsky]

Remove lzo/lz4 backends, we use crypto API now.

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>
---

drivers/block/zram/Kconfig | 9 -------
drivers/block/zram/Makefile | 4 +--
drivers/block/zram/zcomp.c | 2 --
drivers/block/zram/zcomp.h | 15 -----------
drivers/block/zram/zcomp_lz4.c | 56 ------------------------------------------
drivers/block/zram/zcomp_lz4.h | 17 -------------
drivers/block/zram/zcomp_lzo.c | 56 ------------------------------------------
drivers/block/zram/zcomp_lzo.h | 17 -------------

8 files changed, 1 insertion(+), 175 deletions(-)

delete mode 100644 drivers/block/zram/zcomp_lz4.c
delete mode 100644 drivers/block/zram/zcomp_lz4.h
delete mode 100644 drivers/block/zram/zcomp_lzo.c
delete mode 100644 drivers/block/zram/zcomp_lzo.h

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 2381ca9..0c8606b 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -20,9 +20,7 @@

static const char * const backends[] = {

"lzo",
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
"lz4",
-#endif
NULL
};

diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index c914ab7..478cac2 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -16,24 +16,9 @@ struct zcomp_strm {
struct crypto_comp *tfm;

[Sergey Senozhatsky]

On (05/31/16 21:20), Sergey Senozhatsky wrote:
> This has started as a 'add zlib support' work, but after some
> thinking I saw no blockers for a bigger change -- a switch to
> crypto API.
>
> We don't have an idle zstreams list anymore and our write path
> now works absolutely differently, preventing preemption during
> compression. This removes possibilities of read paths preempting
> writes at wrong places and opens the door for a move from custom
> LZO/LZ4 compression backends implementation to a more generic one,
> using crypto compress API.
>
> This patch set also eliminates the need of a new context-less
> crypto API interface, which was quite hard to sell, so we can
> move along faster.

forgot to attach some benchmarks:

(x86_64, 4GB, zram-perf script)

perf reported run-time fio (max jobs=3)

test-fio-zram-842
197.907655282 seconds time elapsed
201.623142884 seconds time elapsed
226.854291345 seconds time elapsed
test-fio-zram-DEFLATE
253.259516155 seconds time elapsed
258.148563401 seconds time elapsed
290.251909365 seconds time elapsed
test-fio-zram-LZ4
27.022598717 seconds time elapsed
29.580522717 seconds time elapsed
33.293463430 seconds time elapsed
test-fio-zram-LZ4HC
56.393954615 seconds time elapsed
74.904659747 seconds time elapsed
101.940998564 seconds time elapsed
test-fio-zram-LZO
28.155948075 seconds time elapsed
30.390036330 seconds time elapsed
34.455773159 seconds time elapsed


zram mm_stat-s (max fio jobs=3)

test-fio-zram-842
mm_stat (jobs1): 3221225472 673185792 690266112 0 690266112 0 0
mm_stat (jobs2): 3221225472 673185792 690266112 0 690266112 0 0
mm_stat (jobs3): 3221225472 673185792 690266112 0 690266112 0 0
test-fio-zram-DEFLATE
mm_stat (jobs1): 3221225472 24379392 37761024 0 37761024 0 0
mm_stat (jobs2): 3221225472 24379392 37761024 0 37761024 0 0
mm_stat (jobs3): 3221225472 24379392 37761024 0 37761024 0 0
test-fio-zram-LZ4
mm_stat (jobs1): 3221225472 23592960 37761024 0 37761024 0 0
mm_stat (jobs2): 3221225472 23592960 37761024 0 37761024 0 0
mm_stat (jobs3): 3221225472 23592960 37761024 0 37761024 0 0
test-fio-zram-LZ4HC
mm_stat (jobs1): 3221225472 23592960 37761024 0 37761024 0 0
mm_stat (jobs2): 3221225472 23592960 37761024 0 37761024 0 0
mm_stat (jobs3): 3221225472 23592960 37761024 0 37761024 0 0
test-fio-zram-LZO
mm_stat (jobs1): 3221225472 34603008 50335744 0 50335744 0 0
mm_stat (jobs2): 3221225472 34603008 50335744 0 50335744 0 0
mm_stat (jobs3): 3221225472 34603008 50335744 0 50339840 0 0


-ss

[Andrew Morton]

On Tue, 31 May 2016 21:29:18 +0900 Sergey Senozhatsky <sergey.se...@gmail.com> wrote:

> forgot to attach some benchmarks:
>
> (x86_64, 4GB, zram-perf script)
>
> perf reported run-time fio (max jobs=3)
>
> test-fio-zram-842
> 197.907655282 seconds time elapsed
> 201.623142884 seconds time elapsed
> 226.854291345 seconds time elapsed
> test-fio-zram-DEFLATE
> 253.259516155 seconds time elapsed
> 258.148563401 seconds time elapsed
> 290.251909365 seconds time elapsed
> test-fio-zram-LZ4
> 27.022598717 seconds time elapsed
> 29.580522717 seconds time elapsed
> 33.293463430 seconds time elapsed
> test-fio-zram-LZ4HC
> 56.393954615 seconds time elapsed
> 74.904659747 seconds time elapsed
> 101.940998564 seconds time elapsed
> test-fio-zram-LZO
> 28.155948075 seconds time elapsed
> 30.390036330 seconds time elapsed
> 34.455773159 seconds time elapsed

I'm having trouble understanding the benchmark results. What is being
compared to what and which was faster and how much?

[Minchan Kim]

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]
?

[Minchan Kim]

On Tue, May 31, 2016 at 09:20:15PM +0900, Sergey Senozhatsky wrote:
> Remove lzo/lz4 backends, we use crypto API now.
>
> Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
> Cc: Minchan Kim <min...@kernel.org>
> Cc: Joonsoo Kim <iamjoon...@lge.com>

Acked-by: Minchan Kim <min...@kernel.org>

[Sergey Senozhatsky]

On (05/31/16 12:07), Andrew Morton wrote:
> > test-fio-zram-842
> > 197.907655282 seconds time elapsed
> > 201.623142884 seconds time elapsed
> > 226.854291345 seconds time elapsed
> > test-fio-zram-DEFLATE
> > 253.259516155 seconds time elapsed
> > 258.148563401 seconds time elapsed
> > 290.251909365 seconds time elapsed
> > test-fio-zram-LZ4
> > 27.022598717 seconds time elapsed
> > 29.580522717 seconds time elapsed
> > 33.293463430 seconds time elapsed
> > test-fio-zram-LZ4HC
> > 56.393954615 seconds time elapsed
> > 74.904659747 seconds time elapsed
> > 101.940998564 seconds time elapsed
> > test-fio-zram-LZO
> > 28.155948075 seconds time elapsed
> > 30.390036330 seconds time elapsed
> > 34.455773159 seconds time elapsed
>
> I'm having trouble understanding the benchmark results. What is being
> compared to what and which was faster and how much?

Hello,

'benchmarking' was probably a bit too strong word to use here. basically,
I performed fio test with the increasing number of parallel jobs (max to 3)
on a 3G zram device, using `static' data and the following crypto comp
algorithms:
842, deflate, lz4, lz4hc, lzo

the output was:
- test running time (which can tell us what algorithms performs faster)
and
- zram mm_stat (which tells the compressed memory size, max used memory, etc).

it's just for information. for example, LZ4HC has twice the running time
of LZO, but the compressed memory size is: 23592960 vs 34603008 bytes.

-ss

[Sergey Senozhatsky]

Hello Minchan,

On (06/01/16 09:03), 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.

[..]

> 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]
> ?

yes, "lzo lz4 [out-of-tree-module-name]".

-ss

[Minchan Kim]

On Wed, Jun 01, 2016 at 10:07:07AM +0900, Sergey Senozhatsky wrote:
> Hello Minchan,
>
> On (06/01/16 09:03), 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.

What do you think about it?

>
> [..]
> > 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]
> > ?
>
> yes, "lzo lz4 [out-of-tree-module-name]".

Makes sense!

[Sergey Senozhatsky]

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.

> 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.

-ss

[Minchan Kim]

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.

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

[Sergey Senozhatsky]

On (06/01/16 15:47), Minchan Kim wrote:
[..]

> > 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.

have you ever seen this? really, why should we even bother?
if there is no requested algorithm we will fallback to LZO.

and how is that different from: user enabled LZO in .config (because it's
a prerequisite for zram) but forgot to install the module? do we have to
"fix" this as well?... implement our own LZO compression in zram?
or `cp lib/lzo/* drivers/block/zram/'?

> For solving the problem, if we check it with crypto_has_comp, again,
> it will load module into memory. :(

this will require a *VERY* non-standard behaviour from user

cat /sys/block/zram0/comp_algorithm
[lzo] lz4
# um...
echo 842 > /sys/block/zram0/comp_algorithm

and I'm quite confident that anyone who does this actually want
to init the device with the requested out-of-tree module right
after `echo FOO > comp_algorithm', rather than anything else.

-ss

[Austin S. Hemmelgarn]

On 2016-06-01 03:48, Sergey Senozhatsky wrote:
> On (06/01/16 15:47), Minchan Kim wrote:
> [..]
>>> 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.
>
> have you ever seen this? really, why should we even bother?
> if there is no requested algorithm we will fallback to LZO.
>
> and how is that different from: user enabled LZO in .config (because it's
> a prerequisite for zram) but forgot to install the module? do we have to
> "fix" this as well?... implement our own LZO compression in zram?
> or `cp lib/lzo/* drivers/block/zram/'?

Ideally, it should fall back to whatever algorithm it can find that is
supported, preferably in the following order:
lzo lz4 lz4hc deflate 842
LZO first will keep backwards compatibility, while the rest is roughly
in decreasing order of performance on most hardware (except on PPC
systems which have hardware support for 842 compression).

Handling not being able to find any algorithm gets trickier, and the
choices are pretty much use a null algorithm and just store flat data,
or refuse to store anything.

>
>> For solving the problem, if we check it with crypto_has_comp, again,
>> it will load module into memory. :(
>
> this will require a *VERY* non-standard behaviour from user
>
> cat /sys/block/zram0/comp_algorithm
> [lzo] lz4
> # um...
> echo 842 > /sys/block/zram0/comp_algorithm
>
> and I'm quite confident that anyone who does this actually want
> to init the device with the requested out-of-tree module right
> after `echo FOO > comp_algorithm', rather than anything else.

Just from the perspective of a system administrator, most people
probably aren't going to be directly touching the sysfs entries
themselves except possibly for testing, and I don't think I've ever seen
anything that actually reads zram/comp_algorithm except to verify that
it's set to the requested algorithm. Given that, the behavior I'd
expect from zram/comp_algorithm as an administrator would be:
1. List the currently used algorithm together with all algorithm's
supported in the mainline kernel.
2. If somebody writes an algorithm we don't know about, check it with
crypto_has_comp, and switch to it if successful.
3. Cache positive lookups of unknown algorithms so that you don't have
to check again on other devices, and list those algorithms in
zram/comp_algorithm even if they're not being used.
This would provide relative compatibility with the current behavior,
while still allowing people using unknown compression modules to use them.

[Minchan Kim]

On Wed, Jun 01, 2016 at 04:48:45PM +0900, Sergey Senozhatsky wrote:
> On (06/01/16 15:47), Minchan Kim wrote:
> [..]
> > > 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.
>
> have you ever seen this? really, why should we even bother?
> if there is no requested algorithm we will fallback to LZO.

Yeb, it seems I am too paranoid. Let's not take care about the case.
We can simple return -EINVAL and fallback lzo.

Thanks.

[Sergey Senozhatsky]

We don't perform any zstream idle list lookup anymore, so
zcomp_strm_find()/zcomp_strm_release() names are not
representative.

Rename to zcomp_stream_get()/zcomp_stream_put().

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>
Acked-by: Minchan Kim <min...@kernel.org>

---
drivers/block/zram/zcomp.c | 4 ++--
drivers/block/zram/zcomp.h | 4 ++--
drivers/block/zram/zram_drv.c | 8 ++++----
3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index b51a816..400f826 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c

@@ -95,12 +95,12 @@ bool zcomp_available_algorithm(const char *comp)
return find_backend(comp) != NULL;
}

-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)

+struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
{
return *get_cpu_ptr(comp->stream);
}

-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
+void zcomp_stream_put(struct zcomp *comp)
{
put_cpu_ptr(comp->stream);
}

diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index ffd88cb..944b8e6 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h

@@ -48,8 +48,8 @@ bool zcomp_available_algorithm(const char *comp);
struct zcomp *zcomp_create(const char *comp);
void zcomp_destroy(struct zcomp *comp);

-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
+struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
+void zcomp_stream_put(struct zcomp *comp);

int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src, size_t *dst_len);

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 8fcad8b..9361a5d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c

[Sergey Senozhatsky]

Hello,

This has started as a 'add zlib support' work, but after some
thinking I saw no blockers for a bigger change -- a switch to
crypto API.

We don't have an idle zstreams list anymore and our write path
now works absolutely differently, preventing preemption during
compression. This removes possibilities of read paths preempting
writes at wrong places and opens the door for a move from custom
LZO/LZ4 compression backends implementation to a more generic one,
using crypto compress API.

This patch set also eliminates the need of a new context-less
crypto API interface, which was quite hard to sell, so we can
move along faster.

v3:
-- use IS_ENABLED in the backends array, so crypto_has_comp()
can be avoided (saving some time and memory).

v2:
-- addressed Minchan's review points
-- allow out-of-tree comp algorithms, per Minchan
-- some other cleanups, reworks and improvements

Sergey Senozhatsky (7):

zram: rename zstrm find-release functions
zram: switch to crypto compress API

zram: use crypto api to check alg availability
zram: cosmetic: cleanup documentation
zram: delete custom lzo/lz4
zram: add more compression algorithms
zram: drop gfp_t from zcomp_strm_alloc()

Documentation/blockdev/zram.txt | 82 ++++++++++++----------
drivers/block/zram/Kconfig | 15 +---
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 150 +++++++++++++++++++++++++---------------

drivers/block/zram/zcomp.h | 36 +++-------
drivers/block/zram/zcomp_lz4.c | 56 ---------------

drivers/block/zram/zcomp_lz4.h | 17 -----
drivers/block/zram/zcomp_lzo.c | 56 ---------------

drivers/block/zram/zcomp_lzo.h | 17 -----
drivers/block/zram/zram_drv.c | 42 ++++++-----
drivers/block/zram/zram_drv.h | 5 +-
11 files changed, 180 insertions(+), 300 deletions(-)

delete mode 100644 drivers/block/zram/zcomp_lz4.c
delete mode 100644 drivers/block/zram/zcomp_lz4.h
delete mode 100644 drivers/block/zram/zcomp_lzo.c
delete mode 100644 drivers/block/zram/zcomp_lzo.h

--
2.8.3.394.g3916adf

[Sergey Senozhatsky]

We now allocate streams from CPU_UP hot-plug path, there are
no context-dependent stream allocations anymore and we can
schedule from zcomp_strm_alloc(). Use GFP_KERNEL directly and
drop a gfp_t parameter.

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>
Acked-by: Minchan Kim <min...@kernel.org>
---

drivers/block/zram/zcomp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 32e521a..4b5cd3a 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -47,9 +47,9 @@ static void zcomp_strm_free(struct zcomp_strm *zstrm)
* allocate new zcomp_strm structure with ->tfm initialized by
* backend, return NULL on error
*/
-static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)
+static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
{
- struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), flags);
+ struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), GFP_KERNEL);
if (!zstrm)
return NULL;

@@ -58,7 +58,7 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)
* allocate 2 pages. 1 for compressed data, plus 1 extra for the
* case when compressed size is larger than the original one
*/
- zstrm->buffer = (void *)__get_free_pages(flags | __GFP_ZERO, 1);
+ zstrm->buffer = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1);
if (IS_ERR_OR_NULL(zstrm->tfm) || !zstrm->buffer) {
zcomp_strm_free(zstrm);
zstrm = NULL;
@@ -169,7 +169,7 @@ static int __zcomp_cpu_notifier(struct zcomp *comp,
case CPU_UP_PREPARE:
if (WARN_ON(*per_cpu_ptr(comp->stream, cpu)))
break;
- zstrm = zcomp_strm_alloc(comp, GFP_KERNEL);
+ zstrm = zcomp_strm_alloc(comp);
if (IS_ERR_OR_NULL(zstrm)) {
pr_err("Can't allocate a compression stream\n");
return NOTIFY_BAD;
--
2.8.3.394.g3916adf

[Sergey Senozhatsky]

Add "deflate", "lz4hc", "842" algorithms to the list of
known compression backends. The real availability of those
algorithms, however, depends on the corresponding
CONFIG_CRYPTO_FOO config options.

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>

---
drivers/block/zram/zcomp.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 9ab45d4..32e521a 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -23,6 +23,15 @@ static const char * const backends[] = {
#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
};

--
2.8.3.394.g3916adf

[Sergey Senozhatsky]

Remove lzo/lz4 backends, we use crypto API now.

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>

Acked-by: Minchan Kim <min...@kernel.org>
---

drivers/block/zram/Kconfig | 9 -------
drivers/block/zram/Makefile | 4 +--

drivers/block/zram/zcomp.c | 2 +-

drivers/block/zram/zcomp.h | 15 -----------
drivers/block/zram/zcomp_lz4.c | 56 ------------------------------------------
drivers/block/zram/zcomp_lz4.h | 17 -------------
drivers/block/zram/zcomp_lzo.c | 56 ------------------------------------------
drivers/block/zram/zcomp_lzo.h | 17 -------------

8 files changed, 2 insertions(+), 174 deletions(-)

delete mode 100644 drivers/block/zram/zcomp_lz4.c
delete mode 100644 drivers/block/zram/zcomp_lz4.h
delete mode 100644 drivers/block/zram/zcomp_lzo.c
delete mode 100644 drivers/block/zram/zcomp_lzo.h

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index a2b4eb8..9ab45d4 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -20,7 +20,7 @@

static const char * const backends[] = {

"lzo",
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
+#if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
#endif
NULL
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index c914ab7..478cac2 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h

[Sergey Senozhatsky]

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 still use the `backends' array to determine if the requested
compression backend is known to crypto api. This array, however,
may not contain some entries, therefore as the last step we call
crypto_has_comp() function which attempts to insmod the requested
compression algorithm to determine if crypto api supports it. The
advantage of this method is that now we permit the usage of

out-of-tree crypto compression modules (implementing S/W or H/W
compression).

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>

---
Documentation/blockdev/zram.txt | 11 ++++++++
drivers/block/zram/zcomp.c | 61 +++++++++++++++++++++++++----------------
drivers/block/zram/zram_drv.c | 16 ++++++-----
drivers/block/zram/zram_drv.h | 5 ++--
4 files changed, 60 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..a2b4eb8 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,56 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)

return zstrm;
}

+bool zcomp_available_algorithm(const char *comp)
+{

+ int i = 0;
+
+ while (backends[i]) {
+ if (sysfs_streq(comp, backends[i]))
+ return true;
+ i++;
+ }
+
+ /*

+ * 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 permit zcomp initialisation
+ * 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 (!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 && crypto_has_comp(comp, 0, 0) == 1)

+ 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 +242,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

[Sergey Senozhatsky]

zram documentation is a mix of different
styles: spaces, tabs, tabs + spaces, etc.

clean it up.

Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
Cc: Minchan Kim <min...@kernel.org>
Cc: Joonsoo Kim <iamjoon...@lge.com>

Cc: Jonathan Corbet <cor...@lwn.net>

Acked-by: Minchan Kim <min...@kernel.org>
---

Documentation/blockdev/zram.txt | 91 ++++++++++++++++++++---------------------
1 file changed, 45 insertions(+), 46 deletions(-)

diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 7c05357..0535ae1 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -59,23 +59,23 @@ num_devices parameter is optional and tells zram how many devices should be
pre-created. Default: 1.

2) Set max number of compression streams
- Regardless the value passed to this attribute, ZRAM will always
- allocate multiple compression streams - one per online CPUs - thus
- allowing several concurrent compression operations. The number of
- allocated compression streams goes down when some of the CPUs
- become offline. There is no single-compression-stream mode anymore,
- unless you are running a UP system or has only 1 CPU online.
-
- To find out how many streams are currently available:
+Regardless the value passed to this attribute, ZRAM will always
+allocate multiple compression streams - one per online CPUs - thus
+allowing several concurrent compression operations. The number of
+allocated compression streams goes down when some of the CPUs
+become offline. There is no single-compression-stream mode anymore,
+unless you are running a UP system or has only 1 CPU online.
+
+To find out how many streams are currently available:
cat /sys/block/zram0/max_comp_streams

3) Select compression algorithm
- Using comp_algorithm device attribute one can see available and
- currently selected (shown in square brackets) compression algorithms,
- change selected compression algorithm (once the device is initialised
- there is no way to change compression algorithm).
+Using comp_algorithm device attribute one can see available and
+currently selected (shown in square brackets) compression algorithms,
+change selected compression algorithm (once the device is initialised
+there is no way to change compression algorithm).

- Examples:
+Examples:
#show supported compression algorithms
cat /sys/block/zram0/comp_algorithm
lzo [lz4]
@@ -83,28 +83,27 @@ 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).
+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.
- Examples:
- # Initialize /dev/zram0 with 50MB disksize
- echo $((50*1024*1024)) > /sys/block/zram0/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.
+Examples:
+ # Initialize /dev/zram0 with 50MB disksize
+ echo $((50*1024*1024)) > /sys/block/zram0/disksize

- # Using mem suffixes
- echo 256K > /sys/block/zram0/disksize
- echo 512M > /sys/block/zram0/disksize
- echo 1G > /sys/block/zram0/disksize
+ # Using mem suffixes
+ echo 256K > /sys/block/zram0/disksize
+ echo 512M > /sys/block/zram0/disksize
+ echo 1G > /sys/block/zram0/disksize

Note:
There is little point creating a zram of greater than twice the size of memory
@@ -112,20 +111,20 @@ since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of the
size of the disk when not in use so a huge zram is wasteful.

5) Set memory limit: Optional
- Set memory limit by writing the value to sysfs node 'mem_limit'.
- The value can be either in bytes or you can use mem suffixes.
- In addition, you could change the value in runtime.
- Examples:
- # limit /dev/zram0 with 50MB memory
- echo $((50*1024*1024)) > /sys/block/zram0/mem_limit
-
- # Using mem suffixes
- echo 256K > /sys/block/zram0/mem_limit
- echo 512M > /sys/block/zram0/mem_limit
- echo 1G > /sys/block/zram0/mem_limit
-
- # To disable memory limit
- echo 0 > /sys/block/zram0/mem_limit
+Set memory limit by writing the value to sysfs node 'mem_limit'.
+The value can be either in bytes or you can use mem suffixes.
+In addition, you could change the value in runtime.
+Examples:
+ # limit /dev/zram0 with 50MB memory
+ echo $((50*1024*1024)) > /sys/block/zram0/mem_limit
+
+ # Using mem suffixes
+ echo 256K > /sys/block/zram0/mem_limit
+ echo 512M > /sys/block/zram0/mem_limit
+ echo 1G > /sys/block/zram0/mem_limit
+
+ # To disable memory limit
+ echo 0 > /sys/block/zram0/mem_limit

6) Activate:
mkswap /dev/zram0
--
2.8.3.394.g3916adf

[Minchan Kim]

Acked-by: Minchan Kim <min...@kernel.org>

[Minchan Kim]

On Sat, Jun 04, 2016 at 11:49:01AM +0900, Sergey Senozhatsky wrote:
> Add "deflate", "lz4hc", "842" algorithms to the list of
> known compression backends. The real availability of those
> algorithms, however, depends on the corresponding
> CONFIG_CRYPTO_FOO config options.
>
> Signed-off-by: Sergey Senozhatsky <sergey.se...@gmail.com>
> Cc: Minchan Kim <min...@kernel.org>
> Cc: Joonsoo Kim <iamjoon...@lge.com>

Acked-by: Minchan Kim <min...@kernel.org>