This patchset makes zram to use crypto API in order to support
more compression algorithm.
The reason we need to support vairous compression algorithms is that
zram's performance is highly depend on workload and compression algorithm
and architecture. Every compression algorithm has it's own strong point.
For example, zlib is the best for compression ratio, but, it's
(de)compression speed is rather slow. Against my expectation, in my kernel
build test with zram swap, in low-memory condition on x86, zlib shows best
performance than others. In this case, I guess that compression ratio is
the most important factor. Unlike this situation, on ARM, maybe fast
(de)compression speed is the most important because it's computation speed
is slower than x86.
Anyway, there is a concern from Sergey to use crypto API in zram. Current
crypto API has a limitation that always require tfm object to (de)compress
something because some of (de)compression function requires scratch buffer
embedded on tfm even if some of (de)compression function doesn't
require it. Due to above reason, using crypto API rather than calling
compression library directly causes more memory footprint and this is
why zram doesn't use crypto API before.
In this patchset, crypto compress noctx API is introduced to reduce memory
footprint caused by maintaining multiple tfm and zram uses it. Before
noctx API, zram's performace is down-graded if tfm is insufficient. But,
after applying noctx API, performace is restored.
This addresses Sergey's concern perfectly and provides possibility to use
various compression algorithm in zram.
Following is zram's read performance number.
* iozone -t 4 -R -r 16K -s 60M -I +Z -i 0 -i 1
* max_stream is set to 1
* Output is in Kbytes/sec
zram-base vs zram-crypto vs zram-crypto-noctx
Read 10411701.88 6426911.62 9423894.38
Re-read 10017386.62 6428218.88 11000063.50
Thanks.
Joonsoo Kim (8):
crypto: support (de)compression API that doesn't require tfm object
crypto/lzo: support decompress_noctx
crypyo/lz4: support decompress_noctx
crypto/lz4hc: support decompress_noctx
crypto/842: support decompress_noctx
zram: change zcomp_compress interface
zram: use crypto API for compression
zram: use decompress_noctx
crypto/842.c | 10 +++-
crypto/compress.c | 20 ++++++++
crypto/crypto_null.c | 4 +-
crypto/deflate.c | 4 +-
crypto/lz4.c | 19 ++++++-
crypto/lz4hc.c | 19 ++++++-
crypto/lzo.c | 20 +++++++-
drivers/block/zram/Kconfig | 13 +----
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 113 +++++++++++++++++++++++++----------------
drivers/block/zram/zcomp.h | 44 +++++++---------
drivers/block/zram/zcomp_lz4.c | 47 -----------------
drivers/block/zram/zcomp_lz4.h | 17 -------
drivers/block/zram/zcomp_lzo.c | 47 -----------------
drivers/block/zram/zcomp_lzo.h | 17 -------
drivers/block/zram/zram_drv.c | 29 +++++++----
include/linux/crypto.h | 31 +++++++++++
17 files changed, 229 insertions(+), 229 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
--
1.9.1
Until now, tfm object embeds (de)compression context in it and
(de)compression in crypto API requires tfm object to use
this context. But, there are some algorithms that doesn't need
such context to operate. Therefore, this patch introduce new crypto
compression API that call (de)compression function via crypto_alg,
instead of tfm object. crypto_alg is required to get appropriate
(de)compression function pointer. This can reduce overhead of
maintaining multiple tfm if (de)compression doesn't require
any context to operate.
Signed-off-by: Joonsoo Kim <[email protected]>
---
crypto/842.c | 4 +++-
crypto/compress.c | 20 ++++++++++++++++++++
crypto/crypto_null.c | 4 +++-
crypto/deflate.c | 4 +++-
crypto/lz4.c | 4 +++-
crypto/lz4hc.c | 4 +++-
crypto/lzo.c | 4 +++-
include/linux/crypto.h | 31 +++++++++++++++++++++++++++++++
8 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/crypto/842.c b/crypto/842.c
index 98e387e..b6503ea 100644
--- a/crypto/842.c
+++ b/crypto/842.c
@@ -61,7 +61,9 @@ static struct crypto_alg alg = {
.cra_module = THIS_MODULE,
.cra_u = { .compress = {
.coa_compress = crypto842_compress,
- .coa_decompress = crypto842_decompress } }
+ .coa_decompress = crypto842_decompress,
+ .coa_compress_noctx = NULL,
+ .coa_decompress_noctx = NULL } }
};
static int __init crypto842_mod_init(void)
diff --git a/crypto/compress.c b/crypto/compress.c
index c33f076..f27eee0 100644
--- a/crypto/compress.c
+++ b/crypto/compress.c
@@ -33,6 +33,26 @@ static int crypto_decompress(struct crypto_tfm *tfm,
dlen);
}
+struct crypto_alg *crypto_get_comp(const char *alg_name, u32 type, u32 mask)
+{
+ struct crypto_alg *alg;
+
+ type &= ~CRYPTO_ALG_TYPE_MASK;
+ type |= CRYPTO_ALG_TYPE_COMPRESS;
+ mask |= CRYPTO_ALG_TYPE_MASK;
+
+ alg = crypto_alg_mod_lookup(alg_name, type, mask);
+ if (!IS_ERR(alg))
+ return alg;
+
+ return NULL;
+}
+
+void crypto_put_comp(struct crypto_alg *alg)
+{
+ crypto_mod_put(alg);
+}
+
int crypto_init_compress_ops(struct crypto_tfm *tfm)
{
struct compress_tfm *ops = &tfm->crt_compress;
diff --git a/crypto/crypto_null.c b/crypto/crypto_null.c
index 941c9a4..e397d1c 100644
--- a/crypto/crypto_null.c
+++ b/crypto/crypto_null.c
@@ -146,7 +146,9 @@ static struct crypto_alg null_algs[3] = { {
.cra_module = THIS_MODULE,
.cra_u = { .compress = {
.coa_compress = null_compress,
- .coa_decompress = null_compress } }
+ .coa_decompress = null_compress,
+ .coa_compress_noctx = NULL,
+ .coa_decompress_noctx = NULL } }
} };
MODULE_ALIAS_CRYPTO("compress_null");
diff --git a/crypto/deflate.c b/crypto/deflate.c
index 95d8d37..dee4daf 100644
--- a/crypto/deflate.c
+++ b/crypto/deflate.c
@@ -203,7 +203,9 @@ static struct crypto_alg alg = {
.cra_exit = deflate_exit,
.cra_u = { .compress = {
.coa_compress = deflate_compress,
- .coa_decompress = deflate_decompress } }
+ .coa_decompress = deflate_decompress,
+ .coa_compress_noctx = NULL,
+ .coa_decompress_noctx = NULL } }
};
static int __init deflate_mod_init(void)
diff --git a/crypto/lz4.c b/crypto/lz4.c
index aefbcea..1848435 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -86,7 +86,9 @@ static struct crypto_alg alg_lz4 = {
.cra_exit = lz4_exit,
.cra_u = { .compress = {
.coa_compress = lz4_compress_crypto,
- .coa_decompress = lz4_decompress_crypto } }
+ .coa_decompress = lz4_decompress_crypto,
+ .coa_compress_noctx = NULL,
+ .coa_decompress_noctx = NULL } }
};
static int __init lz4_mod_init(void)
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index a1d3b5b..bcf0baa 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -86,7 +86,9 @@ static struct crypto_alg alg_lz4hc = {
.cra_exit = lz4hc_exit,
.cra_u = { .compress = {
.coa_compress = lz4hc_compress_crypto,
- .coa_decompress = lz4hc_decompress_crypto } }
+ .coa_decompress = lz4hc_decompress_crypto,
+ .coa_compress_noctx = NULL,
+ .coa_decompress_noctx = NULL } }
};
static int __init lz4hc_mod_init(void)
diff --git a/crypto/lzo.c b/crypto/lzo.c
index 4b3e925..9ca516b 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -89,7 +89,9 @@ static struct crypto_alg alg = {
.cra_exit = lzo_exit,
.cra_u = { .compress = {
.coa_compress = lzo_compress,
- .coa_decompress = lzo_decompress } }
+ .coa_decompress = lzo_decompress,
+ .coa_compress_noctx = NULL,
+ .coa_decompress_noctx = NULL } }
};
static int __init lzo_mod_init(void)
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 81ef938..9e68eb0 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -405,6 +405,10 @@ struct compress_alg {
unsigned int slen, u8 *dst, unsigned int *dlen);
int (*coa_decompress)(struct crypto_tfm *tfm, const u8 *src,
unsigned int slen, u8 *dst, unsigned int *dlen);
+ int (*coa_compress_noctx)(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen);
+ int (*coa_decompress_noctx)(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen);
};
@@ -1888,6 +1892,9 @@ static inline void crypto_free_comp(struct crypto_comp *tfm)
crypto_free_tfm(crypto_comp_tfm(tfm));
}
+struct crypto_alg *crypto_get_comp(const char *alg_name, u32 type, u32 mask);
+void crypto_put_comp(struct crypto_alg *alg);
+
static inline int crypto_has_comp(const char *alg_name, u32 type, u32 mask)
{
type &= ~CRYPTO_ALG_TYPE_MASK;
@@ -1923,5 +1930,29 @@ static inline int crypto_comp_decompress(struct crypto_comp *tfm,
src, slen, dst, dlen);
}
+static inline int crypto_has_compress_noctx(struct crypto_alg *alg)
+{
+ return !!alg->cra_compress.coa_compress_noctx;
+}
+
+static inline int crypto_has_decompress_noctx(struct crypto_alg *alg)
+{
+ return !!alg->cra_compress.coa_decompress_noctx;
+}
+
+static inline int crypto_comp_compress_noctx(struct crypto_alg *alg,
+ const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ return alg->cra_compress.coa_compress_noctx(src, slen, dst, dlen);
+}
+
+static inline int crypto_comp_decompress_noctx(struct crypto_alg *alg,
+ const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ return alg->cra_compress.coa_decompress_noctx(src, slen, dst, dlen);
+}
+
#endif /* _LINUX_CRYPTO_H */
--
1.9.1
lz4's decompression doesn't requires any scratch buffer so
it doesn't need tfm context. Hence, it can support
crypto compression noctx API and this patch implements it.
Signed-off-by: Joonsoo Kim <[email protected]>
---
crypto/lz4.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/crypto/lz4.c b/crypto/lz4.c
index 1848435..aa23da3 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -76,6 +76,21 @@ static int lz4_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
return err;
}
+static int lz4_decompress_noctx(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ int err;
+ size_t tmp_len = *dlen;
+ size_t __slen = slen;
+
+ err = lz4_decompress_unknownoutputsize(src, __slen, dst, &tmp_len);
+ if (err < 0)
+ return -EINVAL;
+
+ *dlen = tmp_len;
+ return err;
+}
+
static struct crypto_alg alg_lz4 = {
.cra_name = "lz4",
.cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
@@ -88,7 +103,7 @@ static struct crypto_alg alg_lz4 = {
.coa_compress = lz4_compress_crypto,
.coa_decompress = lz4_decompress_crypto,
.coa_compress_noctx = NULL,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = lz4_decompress_noctx } }
};
static int __init lz4_mod_init(void)
--
1.9.1
lz4hc's decompression doesn't requires any scratch buffer so
it doesn't need tfm context. Hence, it can support
crypto compression noctx API and this patch implements it.
Signed-off-by: Joonsoo Kim <[email protected]>
---
crypto/lz4hc.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index bcf0baa..a529620 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -76,6 +76,21 @@ static int lz4hc_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
return err;
}
+static int lz4hc_decompress_noctx(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ int err;
+ size_t tmp_len = *dlen;
+ size_t __slen = slen;
+
+ err = lz4_decompress_unknownoutputsize(src, __slen, dst, &tmp_len);
+ if (err < 0)
+ return -EINVAL;
+
+ *dlen = tmp_len;
+ return err;
+}
+
static struct crypto_alg alg_lz4hc = {
.cra_name = "lz4hc",
.cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
@@ -88,7 +103,7 @@ static struct crypto_alg alg_lz4hc = {
.coa_compress = lz4hc_compress_crypto,
.coa_decompress = lz4hc_decompress_crypto,
.coa_compress_noctx = NULL,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = lz4hc_decompress_noctx } }
};
static int __init lz4hc_mod_init(void)
--
1.9.1
842's decompression doesn't requires any scratch buffer so
it doesn't need tfm context. Hence, it can support
crypto compression noctx API and this patch implements it.
Signed-off-by: Joonsoo Kim <[email protected]>
---
crypto/842.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/crypto/842.c b/crypto/842.c
index b6503ea..9acb595 100644
--- a/crypto/842.c
+++ b/crypto/842.c
@@ -52,6 +52,12 @@ static int crypto842_decompress(struct crypto_tfm *tfm,
return sw842_decompress(src, slen, dst, dlen);
}
+static int crypto842_decompress_noctx(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ return sw842_decompress(src, slen, dst, dlen);
+}
+
static struct crypto_alg alg = {
.cra_name = "842",
.cra_driver_name = "842-generic",
@@ -63,7 +69,7 @@ static struct crypto_alg alg = {
.coa_compress = crypto842_compress,
.coa_decompress = crypto842_decompress,
.coa_compress_noctx = NULL,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = crypto842_decompress_noctx } }
};
static int __init crypto842_mod_init(void)
--
1.9.1
Until now, zram uses compression algorithm through direct call
to core algorithm function, but, it has drawback that we need to add
compression algorithm manually to zram if needed. Without this work,
we cannot utilize various compression algorithms in the system.
Moreover, to add new compression algorithm, we need to know how to use it
and this is somewhat time-consuming.
When I tested new algorithms such as zlib, these problems make me hard
to test them. To prevent these problem in the future, I think that
using crypto API for compression is better approach and this patch
implements it.
The reason we need to support vairous compression algorithms is that
zram's performance is highly depend on workload and compression algorithm
and architecture. Every compression algorithm has it's own strong point.
For example, zlib is the best for compression ratio, but, it's
(de)compression speed is rather slow. Against my expectation, in my kernel
build test with zram swap, in low-memory condition on x86, zlib shows best
performance than others. In this case, I guess that compression ratio is
the most important factor. Unlike this situation, on ARM, maybe fast
(de)compression speed is the most important because it's computation speed
is slower than x86.
We can't expect what algorithm is the best fit for one's system, because
it needs too complex calculation. We need to test it in case by case and
easy to use new compression algorithm by this patch will help
that situation.
There is one problem that crypto API requires tfm object to (de)compress
something and zram abstract it on zstrm which is very limited resource.
If number of zstrm is set to low and is contended, zram's performace could
be down-graded due to this change. But, following patch support to use
crypto noctx API and would restore the performance as is.
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/Kconfig | 13 +------
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 84 +++++++++++++++++++++---------------------
drivers/block/zram/zcomp.h | 35 +++++-------------
drivers/block/zram/zcomp_lz4.c | 47 -----------------------
drivers/block/zram/zcomp_lz4.h | 17 ---------
drivers/block/zram/zcomp_lzo.c | 47 -----------------------
drivers/block/zram/zcomp_lzo.h | 17 ---------
drivers/block/zram/zram_drv.c | 26 ++++++++-----
9 files changed, 71 insertions(+), 219 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 386ba3d..36ec96f 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -1,8 +1,7 @@
config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC
- select LZO_COMPRESS
- select LZO_DECOMPRESS
+ select CRYPTO_LZO
default n
help
Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
@@ -14,13 +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 LZ4_COMPRESS
- select LZ4_DECOMPRESS
- default n
- help
- This option enables LZ4 compression algorithm support. Compression
- algorithm can be changed using `comp_algorithm' device attribute.
\ No newline at end of file
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 2ad504b..d2734f2 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -13,12 +13,9 @@
#include <linux/slab.h>
#include <linux/wait.h>
#include <linux/sched.h>
+#include <linux/crypto.h>
#include "zcomp.h"
-#include "zcomp_lzo.h"
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
-#include "zcomp_lz4.h"
-#endif
/*
* single zcomp_strm backend
@@ -43,36 +40,34 @@ struct zcomp_strm_multi {
wait_queue_head_t strm_wait;
};
-static struct zcomp_backend *backends[] = {
- &zcomp_lzo,
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
- &zcomp_lz4,
+static const char * const backends[] = {
+ "lzo",
+#ifdef CONFIG_CRYPTO_LZ4
+ "lz4",
+#endif
+#ifdef CONFIG_CRYPTO_LZ4HC
+ "lz4hc",
+#endif
+#ifdef CONFIG_CRYPTO_DEFLATE
+ "deflate",
+#endif
+#ifdef CONFIG_CRYPTO_842
+ "842",
#endif
NULL
};
-static struct zcomp_backend *find_backend(const char *compress)
-{
- int i = 0;
- while (backends[i]) {
- if (sysfs_streq(compress, backends[i]->name))
- break;
- i++;
- }
- return backends[i];
-}
-
static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
{
- if (zstrm->private)
- comp->backend->destroy(zstrm->private);
+ if (zstrm->tfm)
+ crypto_free_comp(zstrm->tfm);
free_pages((unsigned long)zstrm->buffer, 1);
kfree(zstrm);
}
/*
- * allocate new zcomp_strm structure with ->private initialized by
- * backend, return NULL on error
+ * allocate new zcomp_strm structure with initializing crypto data structure,
+ * return NULL on error
*/
static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
{
@@ -80,13 +75,18 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
if (!zstrm)
return NULL;
- zstrm->private = comp->backend->create();
+ zstrm->tfm = crypto_alloc_comp(comp->name, 0, 0);
+ if (IS_ERR(zstrm->tfm)) {
+ kfree(zstrm);
+ return NULL;
+ }
+
/*
* 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(GFP_KERNEL | __GFP_ZERO, 1);
- if (!zstrm->private || !zstrm->buffer) {
+ if (!zstrm->buffer) {
zcomp_strm_free(comp, zstrm);
zstrm = NULL;
}
@@ -274,23 +274,18 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
int i = 0;
while (backends[i]) {
- if (!strcmp(comp, backends[i]->name))
+ if (!strcmp(comp, backends[i]))
sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
- "[%s] ", backends[i]->name);
+ "[%s] ", backends[i]);
else
sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
- "%s ", backends[i]->name);
+ "%s ", backends[i]);
i++;
}
sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
return sz;
}
-bool zcomp_available_algorithm(const char *comp)
-{
- return find_backend(comp) != NULL;
-}
-
bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
{
return comp->set_max_streams(comp, num_strm);
@@ -307,15 +302,21 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
}
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, unsigned char *dst, size_t *dst_len)
+ const unsigned char *src, unsigned char *dst,
+ unsigned int *dst_len)
{
- return comp->backend->compress(src, dst, dst_len, zstrm->private);
+ *dst_len = PAGE_SIZE << 1;
+
+ return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE, dst, dst_len);
}
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
- size_t src_len, unsigned char *dst)
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src, unsigned int src_len,
+ unsigned char *dst)
{
- return comp->backend->decompress(src, src_len, dst);
+ unsigned int size = PAGE_SIZE;
+
+ return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
}
void zcomp_destroy(struct zcomp *comp)
@@ -334,17 +335,16 @@ void zcomp_destroy(struct zcomp *comp)
struct zcomp *zcomp_create(const char *compress, int max_strm)
{
struct zcomp *comp;
- struct zcomp_backend *backend;
- backend = find_backend(compress);
- if (!backend)
+ if (!crypto_has_comp(compress, 0, 0))
return ERR_PTR(-EINVAL);
comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
if (!comp)
return ERR_PTR(-ENOMEM);
- comp->backend = backend;
+ comp->name = compress;
+
if (max_strm > 1)
zcomp_strm_multi_create(comp, max_strm);
else
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index b2388e0..c47db4d 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -10,39 +10,23 @@
#ifndef _ZCOMP_H_
#define _ZCOMP_H_
+#include <linux/crypto.h>
#include <linux/mutex.h>
struct zcomp_strm {
+ struct crypto_comp *tfm;
+
/* compression/decompression buffer */
void *buffer;
- /*
- * The private data of the compression stream, only compression
- * stream backend can touch this (e.g. compression algorithm
- * working memory)
- */
- void *private;
+
/* used in multi stream backend, protected by backend strm_lock */
struct list_head list;
};
-/* 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)(void);
- void (*destroy)(void *private);
-
- const char *name;
-};
-
/* dynamic per-device compression frontend */
struct zcomp {
void *stream;
- struct zcomp_backend *backend;
+ const char *name;
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
@@ -51,7 +35,6 @@ struct zcomp {
};
ssize_t zcomp_available_show(const char *comp, char *buf);
-bool zcomp_available_algorithm(const char *comp);
struct zcomp *zcomp_create(const char *comp, int max_strm);
void zcomp_destroy(struct zcomp *comp);
@@ -60,10 +43,12 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, unsigned char *dst, size_t *dst_len);
+ const unsigned char *src, unsigned char *dst,
+ unsigned int *dst_len);
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
- size_t src_len, unsigned char *dst);
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src, unsigned int src_len,
+ unsigned char *dst);
bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
#endif /* _ZCOMP_H_ */
diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
deleted file mode 100644
index f2afb7e..0000000
--- a/drivers/block/zram/zcomp_lz4.c
+++ /dev/null
@@ -1,47 +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 "zcomp_lz4.h"
-
-static void *zcomp_lz4_create(void)
-{
- return kzalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
-}
-
-static void zcomp_lz4_destroy(void *private)
-{
- kfree(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 da1bc47..0000000
--- a/drivers/block/zram/zcomp_lzo.c
+++ /dev/null
@@ -1,47 +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 "zcomp_lzo.h"
-
-static void *lzo_create(void)
-{
- return kzalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
-}
-
-static void lzo_destroy(void *private)
-{
- kfree(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_ */
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 4801e4d..b3044d3 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -30,6 +30,7 @@
#include <linux/err.h>
#include <linux/idr.h>
#include <linux/sysfs.h>
+#include <linux/crypto.h>
#include "zram_drv.h"
@@ -378,7 +379,7 @@ static ssize_t comp_algorithm_store(struct device *dev,
if (sz > 0 && zram->compressor[sz - 1] == '\n')
zram->compressor[sz - 1] = 0x00;
- if (!zcomp_available_algorithm(zram->compressor))
+ if (!crypto_has_comp(zram->compressor, 0, 0))
len = -EINVAL;
up_write(&zram->init_lock);
@@ -562,13 +563,14 @@ static void zram_free_page(struct zram *zram, size_t index)
zram_set_obj_size(meta, index, 0);
}
-static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
+static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
+ char *mem, u32 index)
{
int ret = 0;
unsigned char *cmem;
struct zram_meta *meta = zram->meta;
unsigned long handle;
- size_t size;
+ unsigned int size;
bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
handle = meta->table[index].handle;
@@ -584,7 +586,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
if (size == PAGE_SIZE)
copy_page(mem, cmem);
else
- ret = zcomp_decompress(zram->comp, cmem, size, mem);
+ ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
zs_unmap_object(meta->mem_pool, handle);
bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
@@ -604,6 +606,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
struct page *page;
unsigned char *user_mem, *uncmem = NULL;
struct zram_meta *meta = zram->meta;
+ struct zcomp_strm *zstrm;
+
page = bvec->bv_page;
bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
@@ -619,6 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
/* Use a temporary buffer to decompress the page */
uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
+ zstrm = zcomp_strm_find(zram->comp);
user_mem = kmap_atomic(page);
if (!is_partial_io(bvec))
uncmem = user_mem;
@@ -629,7 +634,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
goto out_cleanup;
}
- ret = zram_decompress_page(zram, uncmem, index);
+ ret = zram_decompress_page(zram, zstrm, uncmem, index);
/* Should NEVER happen. Return bio error if it does. */
if (unlikely(ret))
goto out_cleanup;
@@ -642,6 +647,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
ret = 0;
out_cleanup:
kunmap_atomic(user_mem);
+ zcomp_strm_release(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
@@ -651,7 +657,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
int offset)
{
int ret = 0;
- size_t clen;
+ unsigned int clen;
unsigned long handle;
struct page *page;
unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
@@ -670,12 +676,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
- ret = zram_decompress_page(zram, uncmem, index);
+ zstrm = zcomp_strm_find(zram->comp);
+ ret = zram_decompress_page(zram, zstrm, uncmem, index);
if (ret)
goto out;
}
- zstrm = zcomp_strm_find(zram->comp);
+ if (!zstrm)
+ zstrm = zcomp_strm_find(zram->comp);
user_mem = kmap_atomic(page);
if (is_partial_io(bvec)) {
@@ -721,7 +729,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
handle = zs_malloc(meta->mem_pool, clen);
if (!handle) {
- pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
+ pr_info("Error allocating memory for compressed page: %u, size=%u\n",
index, clen);
ret = -ENOMEM;
goto out;
--
1.9.1
zram regards zstrm's buffer as compression destination buffer, but,
it is not intuitive and there is no document about it. Providing
destination buffer to zcomp_compress() directly seems more intuitive
interface to me so this patch changes zcomp_compress interface.
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/zcomp.c | 5 ++---
drivers/block/zram/zcomp.h | 2 +-
drivers/block/zram/zram_drv.c | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 965d1af..2ad504b 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -307,10 +307,9 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
}
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, size_t *dst_len)
+ const unsigned char *src, unsigned char *dst, size_t *dst_len)
{
- return comp->backend->compress(src, zstrm->buffer, dst_len,
- zstrm->private);
+ return comp->backend->compress(src, dst, dst_len, zstrm->private);
}
int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 46e2b9f..b2388e0 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -60,7 +60,7 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, size_t *dst_len);
+ const unsigned char *src, unsigned char *dst, size_t *dst_len);
int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
size_t src_len, unsigned char *dst);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index b088ca9..4801e4d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -701,7 +701,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
goto out;
}
- ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen);
+ ret = zcomp_compress(zram->comp, zstrm, uncmem, zstrm->buffer, &clen);
if (!is_partial_io(bvec)) {
kunmap_atomic(user_mem);
user_mem = NULL;
--
1.9.1
Crypto subsystem supports noctx API which doesn't require tfm.
To get tfm in zram, we need zstrm and it is limited resource.
If we uses noctx API, we don't need to get zstrm so that
we get much better performance when zstrm is insufficient.
This patch restores zram's performance to the time that zram
doesn't use crypto subsystem.
Following is zram's read performance number.
* iozone -t 4 -R -r 16K -s 60M -I +Z -i 0 -i 1
* max_stream is set to 1
* Output is in Kbytes/sec
zram-base vs zram-crypto vs zram-crypto-noctx
Read 10411701.88 6426911.62 9423894.38
Re-read 10017386.62 6428218.88 11000063.50
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/zcomp.c | 28 +++++++++++++++++++++++++++-
drivers/block/zram/zcomp.h | 9 ++++++++-
drivers/block/zram/zram_drv.c | 9 +++++----
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index d2734f2..86b0c9b 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -291,8 +291,12 @@ bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
return comp->set_max_streams(comp, num_strm);
}
-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
+struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp)
{
+ /* We don't need decompression context so zstrm neither */
+ if (decomp && test_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags))
+ return NULL;
+
return comp->strm_find(comp);
}
@@ -307,6 +311,11 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
{
*dst_len = PAGE_SIZE << 1;
+ if (!zstrm) {
+ return crypto_comp_compress_noctx(comp->alg, src, PAGE_SIZE,
+ dst, dst_len);
+ }
+
return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE, dst, dst_len);
}
@@ -316,12 +325,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
{
unsigned int size = PAGE_SIZE;
+ if (!zstrm) {
+ return crypto_comp_decompress_noctx(comp->alg, src, src_len,
+ dst, &size);
+ }
+
return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
}
void zcomp_destroy(struct zcomp *comp)
{
comp->destroy(comp);
+ crypto_put_comp(comp->alg);
kfree(comp);
}
@@ -344,12 +359,23 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
return ERR_PTR(-ENOMEM);
comp->name = compress;
+ comp->alg = crypto_get_comp(compress, 0, 0);
+ if (!comp->alg) {
+ kfree(comp);
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (crypto_has_compress_noctx(comp->alg))
+ set_bit(ZCOMP_COMPRESS_NOCTX, &comp->flags);
+ if (crypto_has_decompress_noctx(comp->alg))
+ set_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags);
if (max_strm > 1)
zcomp_strm_multi_create(comp, max_strm);
else
zcomp_strm_single_create(comp);
if (!comp->stream) {
+ crypto_put_comp(comp->alg);
kfree(comp);
return ERR_PTR(-ENOMEM);
}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index c47db4d..6c137c8 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -13,6 +13,11 @@
#include <linux/crypto.h>
#include <linux/mutex.h>
+enum {
+ ZCOMP_COMPRESS_NOCTX,
+ ZCOMP_DECOMPRESS_NOCTX,
+};
+
struct zcomp_strm {
struct crypto_comp *tfm;
@@ -27,6 +32,8 @@ struct zcomp_strm {
struct zcomp {
void *stream;
const char *name;
+ struct crypto_alg *alg;
+ unsigned long flags;
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
@@ -39,7 +46,7 @@ ssize_t zcomp_available_show(const char *comp, char *buf);
struct zcomp *zcomp_create(const char *comp, int max_strm);
void zcomp_destroy(struct zcomp *comp);
-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
+struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp);
void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index b3044d3..8283bd3 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -623,7 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
/* Use a temporary buffer to decompress the page */
uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
- zstrm = zcomp_strm_find(zram->comp);
+ zstrm = zcomp_strm_find(zram->comp, true);
user_mem = kmap_atomic(page);
if (!is_partial_io(bvec))
uncmem = user_mem;
@@ -647,7 +647,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
ret = 0;
out_cleanup:
kunmap_atomic(user_mem);
- zcomp_strm_release(zram->comp, zstrm);
+ if (zstrm)
+ zcomp_strm_release(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
@@ -676,14 +677,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
- zstrm = zcomp_strm_find(zram->comp);
+ zstrm = zcomp_strm_find(zram->comp, true);
ret = zram_decompress_page(zram, zstrm, uncmem, index);
if (ret)
goto out;
}
if (!zstrm)
- zstrm = zcomp_strm_find(zram->comp);
+ zstrm = zcomp_strm_find(zram->comp, false);
user_mem = kmap_atomic(page);
if (is_partial_io(bvec)) {
--
1.9.1
lzo's decompression doesn't requires any scratch buffer so
it doesn't need tfm context. Hence, it can support
crypto compression noctx API and this patch implements it.
Signed-off-by: Joonsoo Kim <[email protected]>
---
crypto/lzo.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/crypto/lzo.c b/crypto/lzo.c
index 9ca516b..f1844dd 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -80,6 +80,22 @@ static int lzo_decompress(struct crypto_tfm *tfm, const u8 *src,
}
+static int lzo_decompress_noctx(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ int err;
+ size_t tmp_len = *dlen; /* size_t(ulong) <-> uint on 64 bit */
+
+ err = lzo1x_decompress_safe(src, slen, dst, &tmp_len);
+
+ if (err != LZO_E_OK)
+ return -EINVAL;
+
+ *dlen = tmp_len;
+ return 0;
+
+}
+
static struct crypto_alg alg = {
.cra_name = "lzo",
.cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
@@ -91,7 +107,7 @@ static struct crypto_alg alg = {
.coa_compress = lzo_compress,
.coa_decompress = lzo_decompress,
.coa_compress_noctx = NULL,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = lzo_decompress_noctx } }
};
static int __init lzo_mod_init(void)
--
1.9.1
On Thu, Aug 20, 2015 at 03:34:57PM +0900, Joonsoo Kim wrote:
> Until now, tfm object embeds (de)compression context in it and
> (de)compression in crypto API requires tfm object to use
> this context. But, there are some algorithms that doesn't need
> such context to operate. Therefore, this patch introduce new crypto
> compression API that call (de)compression function via crypto_alg,
> instead of tfm object. crypto_alg is required to get appropriate
> (de)compression function pointer. This can reduce overhead of
> maintaining multiple tfm if (de)compression doesn't require
> any context to operate.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
This isn't going to fly I'm afraid. The main purpose of a tfm
is not to allocate memory but to track the crypto_alg object
which could be a hardware device.
So you're not going to get away with not allocating it.
What you can do for these contextless algorithms (and by definition
every compression algorithm is conxteless) is to allocate a system-
wide tfm that is used by everybody, or at least by every one within
your subsystem.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Aug 20, 2015 at 02:47:28PM +0800, Herbert Xu wrote:
> On Thu, Aug 20, 2015 at 03:34:57PM +0900, Joonsoo Kim wrote:
> > Until now, tfm object embeds (de)compression context in it and
> > (de)compression in crypto API requires tfm object to use
> > this context. But, there are some algorithms that doesn't need
> > such context to operate. Therefore, this patch introduce new crypto
> > compression API that call (de)compression function via crypto_alg,
> > instead of tfm object. crypto_alg is required to get appropriate
> > (de)compression function pointer. This can reduce overhead of
> > maintaining multiple tfm if (de)compression doesn't require
> > any context to operate.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
>
> This isn't going to fly I'm afraid. The main purpose of a tfm
> is not to allocate memory but to track the crypto_alg object
> which could be a hardware device.
>
> So you're not going to get away with not allocating it.
Hmm... I guess there is no problem. crypto_alg object fetched by
crypto_get_comp() introduced in this patch could be hardware device
algorithm which is same one that we can eventually fetch from tfm object.
So, this approach would correctly track the crypto_alg regardless
it is a hardware one or not. If there is some dependency between
algorithm and tfm, it can't support _noctx API. Am I missing
something?
>
> What you can do for these contextless algorithms (and by definition
> every compression algorithm is conxteless) is to allocate a system-
> wide tfm that is used by everybody, or at least by every one within
> your subsystem.
Yes, I thought this way before, but, current way is much simpler so
I try it first. If it is not acceptable, I will implement this
approach.
Thanks.
On Thu, Aug 20, 2015 at 04:52:17PM +0900, Joonsoo Kim wrote:
>
> Hmm... I guess there is no problem. crypto_alg object fetched by
> crypto_get_comp() introduced in this patch could be hardware device
> algorithm which is same one that we can eventually fetch from tfm object.
> So, this approach would correctly track the crypto_alg regardless
> it is a hardware one or not. If there is some dependency between
> algorithm and tfm, it can't support _noctx API. Am I missing
> something?
Your approach limits what hardware devices we can support in
future. It is fairly common for hardware drivers to only allocate
resources when a tfm is created. With your tfmless interface,
the driver would have to unconditionally allocate resources.
It is essentially a global tfm without the tfm.
> Yes, I thought this way before, but, current way is much simpler so
> I try it first. If it is not acceptable, I will implement this
> approach.
Please go with a global tfm.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Aug 20, 2015 at 03:50:35PM +0800, Herbert Xu wrote:
> On Thu, Aug 20, 2015 at 04:52:17PM +0900, Joonsoo Kim wrote:
> >
> > Hmm... I guess there is no problem. crypto_alg object fetched by
> > crypto_get_comp() introduced in this patch could be hardware device
> > algorithm which is same one that we can eventually fetch from tfm object.
> > So, this approach would correctly track the crypto_alg regardless
> > it is a hardware one or not. If there is some dependency between
> > algorithm and tfm, it can't support _noctx API. Am I missing
> > something?
>
> Your approach limits what hardware devices we can support in
> future. It is fairly common for hardware drivers to only allocate
> resources when a tfm is created. With your tfmless interface,
> the driver would have to unconditionally allocate resources.
Ah...Okay. thanks for clarifying.
>
> It is essentially a global tfm without the tfm.
>
> > Yes, I thought this way before, but, current way is much simpler so
> > I try it first. If it is not acceptable, I will implement this
> > approach.
>
> Please go with a global tfm.
Okay. Will do that in next spin.
Thanks.
On (08/20/15 15:34), Joonsoo Kim wrote:
> lzo's decompression doesn't requires any scratch buffer so
> it doesn't need tfm context. Hence, it can support
> crypto compression noctx API and this patch implements it.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> crypto/lzo.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/lzo.c b/crypto/lzo.c
> index 9ca516b..f1844dd 100644
> --- a/crypto/lzo.c
> +++ b/crypto/lzo.c
> @@ -80,6 +80,22 @@ static int lzo_decompress(struct crypto_tfm *tfm, const u8 *src,
>
> }
>
> +static int lzo_decompress_noctx(const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen)
> +{
> + int err;
> + size_t tmp_len = *dlen; /* size_t(ulong) <-> uint on 64 bit */
> +
> + err = lzo1x_decompress_safe(src, slen, dst, &tmp_len);
> +
> + if (err != LZO_E_OK)
> + return -EINVAL;
> +
> + *dlen = tmp_len;
> + return 0;
> +
just do
static int lzo_decompress_noctx(const u8 *src, unsigned int slen,
u8 *dst, unsigned int *dlen)
{
return lzo_decompress(NULL, src, len, dst, dlen);
}
?
-ss
> +}
> +
> static struct crypto_alg alg = {
> .cra_name = "lzo",
> .cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
> @@ -91,7 +107,7 @@ static struct crypto_alg alg = {
> .coa_compress = lzo_compress,
> .coa_decompress = lzo_decompress,
> .coa_compress_noctx = NULL,
> - .coa_decompress_noctx = NULL } }
> + .coa_decompress_noctx = lzo_decompress_noctx } }
> };
>
> static int __init lzo_mod_init(void)
> --
> 1.9.1
>
On (08/20/15 15:34), Joonsoo Kim wrote:
> lz4's decompression doesn't requires any scratch buffer so
> it doesn't need tfm context. Hence, it can support
> crypto compression noctx API and this patch implements it.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> crypto/lz4.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/lz4.c b/crypto/lz4.c
> index 1848435..aa23da3 100644
> --- a/crypto/lz4.c
> +++ b/crypto/lz4.c
> @@ -76,6 +76,21 @@ static int lz4_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
> return err;
> }
>
> +static int lz4_decompress_noctx(const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen)
> +{
> + int err;
> + size_t tmp_len = *dlen;
> + size_t __slen = slen;
> +
> + err = lz4_decompress_unknownoutputsize(src, __slen, dst, &tmp_len);
> + if (err < 0)
> + return -EINVAL;
> +
> + *dlen = tmp_len;
> + return err;
> +}
> +
same,
static int lz4_decompress_noctx(const u8 *src, unsigned int slen,
u8 *dst, unsigned int *dlen)
{
return lz4_decompress_crypto(NULL, ....);
}
?
> static struct crypto_alg alg_lz4 = {
> .cra_name = "lz4",
> .cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
> @@ -88,7 +103,7 @@ static struct crypto_alg alg_lz4 = {
> .coa_compress = lz4_compress_crypto,
> .coa_decompress = lz4_decompress_crypto,
> .coa_compress_noctx = NULL,
> - .coa_decompress_noctx = NULL } }
> + .coa_decompress_noctx = lz4_decompress_noctx } }
> };
>
> static int __init lz4_mod_init(void)
> --
> 1.9.1
>
On (08/20/15 15:35), Joonsoo Kim wrote:
>
> lz4hc's decompression doesn't requires any scratch buffer so
> it doesn't need tfm context. Hence, it can support
> crypto compression noctx API and this patch implements it.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> crypto/lz4hc.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
> index bcf0baa..a529620 100644
> --- a/crypto/lz4hc.c
> +++ b/crypto/lz4hc.c
> @@ -76,6 +76,21 @@ static int lz4hc_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
> return err;
> }
>
> +static int lz4hc_decompress_noctx(const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen)
> +{
> + int err;
> + size_t tmp_len = *dlen;
> + size_t __slen = slen;
> +
> + err = lz4_decompress_unknownoutputsize(src, __slen, dst, &tmp_len);
> + if (err < 0)
> + return -EINVAL;
> +
> + *dlen = tmp_len;
> + return err;
> +}
> +
same,
static int lz4hc_decompress_noctx(const u8 *src, unsigned int slen,
u8 *dst, unsigned int *dlen)
{
return lz4hc_decompress_crypto(NULL, ....);
}
?
-ss
> static struct crypto_alg alg_lz4hc = {
> .cra_name = "lz4hc",
> .cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
> @@ -88,7 +103,7 @@ static struct crypto_alg alg_lz4hc = {
> .coa_compress = lz4hc_compress_crypto,
> .coa_decompress = lz4hc_decompress_crypto,
> .coa_compress_noctx = NULL,
> - .coa_decompress_noctx = NULL } }
> + .coa_decompress_noctx = lz4hc_decompress_noctx } }
> };
>
> static int __init lz4hc_mod_init(void)
> --
> 1.9.1
>
On (08/20/15 15:35), Joonsoo Kim wrote:
> zram regards zstrm's buffer as compression destination buffer, but,
> it is not intuitive and there is no document about it. Providing
> destination buffer to zcomp_compress() directly seems more intuitive
> interface to me so this patch changes zcomp_compress interface.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> drivers/block/zram/zcomp.c | 5 ++---
> drivers/block/zram/zcomp.h | 2 +-
> drivers/block/zram/zram_drv.c | 2 +-
> 3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index 965d1af..2ad504b 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -307,10 +307,9 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
> }
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> - const unsigned char *src, size_t *dst_len)
> + const unsigned char *src, unsigned char *dst, size_t *dst_len)
> {
> - return comp->backend->compress(src, zstrm->buffer, dst_len,
> - zstrm->private);
> + return comp->backend->compress(src, dst, dst_len, zstrm->private);
> }
>
> int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 46e2b9f..b2388e0 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -60,7 +60,7 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
> void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> - const unsigned char *src, size_t *dst_len);
> + const unsigned char *src, unsigned char *dst, size_t *dst_len);
>
> int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> size_t src_len, unsigned char *dst);
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index b088ca9..4801e4d 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -701,7 +701,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> goto out;
> }
>
> - ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen);
> + ret = zcomp_compress(zram->comp, zstrm, uncmem, zstrm->buffer, &clen);
No, this change is unreasonable. zcomp might want to do with zstrm
anything it wants to, because zstrm belongs there. Besides, you change
zcomp_decompress() to require `struct zcomp_strm *zstrm', so let's
stick with this -- pass `struct zcomp_strm *zstrm' to both compress and
decompress functions. It's up to zcomp to ignore passed zstrm or do
something sane; not up to zram_drv.
-ss
The patch contains too much noise and unrelated changes. Its goal is
to switch zcomp to use Crypto api.
I really would love to see zram_drv aligned with
https://lkml.org/lkml/2015/8/13/343
IOW, only zcomp_decompress_begin()/zcomp_decompress_end() changes in
zram; the rest is purely zcomp related.
[..]
> -static struct zcomp_backend *backends[] = {
> - &zcomp_lzo,
> -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> - &zcomp_lz4,
> +static const char * const backends[] = {
> + "lzo",
> +#ifdef CONFIG_CRYPTO_LZ4
> + "lz4",
> +#endif
> +#ifdef CONFIG_CRYPTO_LZ4HC
> + "lz4hc",
> +#endif
> +#ifdef CONFIG_CRYPTO_DEFLATE
> + "deflate",
> +#endif
> +#ifdef CONFIG_CRYPTO_842
> + "842",
> #endif
> NULL
> };
why this change is in this patch?
> -static struct zcomp_backend *find_backend(const char *compress)
> -{
> - int i = 0;
> - while (backends[i]) {
> - if (sysfs_streq(compress, backends[i]->name))
> - break;
> - i++;
> - }
> - return backends[i];
> -}
No, find_backend() and zcomp_available_algorithm() must stay. Don't call
crypto API functions from zram_drv directly. This functionality belongs to
zcomp.
> static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> {
> - if (zstrm->private)
> - comp->backend->destroy(zstrm->private);
> + if (zstrm->tfm)
> + crypto_free_comp(zstrm->tfm);
> free_pages((unsigned long)zstrm->buffer, 1);
> kfree(zstrm);
> }
>
> /*
> - * allocate new zcomp_strm structure with ->private initialized by
> - * backend, return NULL on error
> + * allocate new zcomp_strm structure with initializing crypto data structure,
> + * return NULL on error
> */
> static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> {
> @@ -80,13 +75,18 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> if (!zstrm)
> return NULL;
>
> - zstrm->private = comp->backend->create();
> + zstrm->tfm = crypto_alloc_comp(comp->name, 0, 0);
> + if (IS_ERR(zstrm->tfm)) {
> + kfree(zstrm);
> + return NULL;
> + }
> +
> /*
> * 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(GFP_KERNEL | __GFP_ZERO, 1);
> - if (!zstrm->private || !zstrm->buffer) {
> + if (!zstrm->buffer) {
> zcomp_strm_free(comp, zstrm);
> zstrm = NULL;
> }
> @@ -274,23 +274,18 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
> int i = 0;
>
> while (backends[i]) {
> - if (!strcmp(comp, backends[i]->name))
> + if (!strcmp(comp, backends[i]))
> sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
> - "[%s] ", backends[i]->name);
> + "[%s] ", backends[i]);
> else
> sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
> - "%s ", backends[i]->name);
> + "%s ", backends[i]);
> i++;
> }
> sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
> return sz;
> }
>
> -bool zcomp_available_algorithm(const char *comp)
> -{
> - return find_backend(comp) != NULL;
> -}
> -
No.
> bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
> {
> return comp->set_max_streams(comp, num_strm);
> @@ -307,15 +302,21 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
> }
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> - const unsigned char *src, unsigned char *dst, size_t *dst_len)
> + const unsigned char *src, unsigned char *dst,
> + unsigned int *dst_len)
> {
> - return comp->backend->compress(src, dst, dst_len, zstrm->private);
> + *dst_len = PAGE_SIZE << 1;
> +
> + return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE, dst, dst_len);
> }
No, see later.
> -int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> - size_t src_len, unsigned char *dst)
> +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> + const unsigned char *src, unsigned int src_len,
> + unsigned char *dst)
> {
> - return comp->backend->decompress(src, src_len, dst);
> + unsigned int size = PAGE_SIZE;
> +
> + return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
> }
No, no direct Crypto API calls from zram_drv.
compression/decompression should be handled in zcomp. period.
What's the point of having zcomp in the first place then?
>
> void zcomp_destroy(struct zcomp *comp)
> @@ -334,17 +335,16 @@ void zcomp_destroy(struct zcomp *comp)
> struct zcomp *zcomp_create(const char *compress, int max_strm)
> {
> struct zcomp *comp;
> - struct zcomp_backend *backend;
>
> - backend = find_backend(compress);
> - if (!backend)
> + if (!crypto_has_comp(compress, 0, 0))
> return ERR_PTR(-EINVAL);
No. Crypto API calls stay in zcomp.
zram_drv should know nothing about it.
> comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
> if (!comp)
> return ERR_PTR(-ENOMEM);
>
> - comp->backend = backend;
> + comp->name = compress;
No. I don't like that zram now use `struct zcomp' internals. Besides,
->tfm keeps alg name, zram keeps alg name.
> if (max_strm > 1)
> zcomp_strm_multi_create(comp, max_strm);
> else
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index b2388e0..c47db4d 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -10,39 +10,23 @@
> #ifndef _ZCOMP_H_
> #define _ZCOMP_H_
>
> +#include <linux/crypto.h>
> #include <linux/mutex.h>
>
> struct zcomp_strm {
> + struct crypto_comp *tfm;
> +
> /* compression/decompression buffer */
> void *buffer;
> - /*
> - * The private data of the compression stream, only compression
> - * stream backend can touch this (e.g. compression algorithm
> - * working memory)
> - */
> - void *private;
> +
> /* used in multi stream backend, protected by backend strm_lock */
> struct list_head list;
> };
>
> -/* 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)(void);
> - void (*destroy)(void *private);
> -
> - const char *name;
> -};
> -
> /* dynamic per-device compression frontend */
> struct zcomp {
> void *stream;
> - struct zcomp_backend *backend;
> + const char *name;
>
> struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> @@ -51,7 +35,6 @@ struct zcomp {
> };
>
> ssize_t zcomp_available_show(const char *comp, char *buf);
> -bool zcomp_available_algorithm(const char *comp);
>
> struct zcomp *zcomp_create(const char *comp, int max_strm);
> void zcomp_destroy(struct zcomp *comp);
> @@ -60,10 +43,12 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
> void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> - const unsigned char *src, unsigned char *dst, size_t *dst_len);
> + const unsigned char *src, unsigned char *dst,
> + unsigned int *dst_len);
>
> -int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> - size_t src_len, unsigned char *dst);
> +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> + const unsigned char *src, unsigned int src_len,
> + unsigned char *dst);
>
> bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
> #endif /* _ZCOMP_H_ */
> diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
> deleted file mode 100644
> index f2afb7e..0000000
> --- a/drivers/block/zram/zcomp_lz4.c
> +++ /dev/null
> @@ -1,47 +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 "zcomp_lz4.h"
> -
> -static void *zcomp_lz4_create(void)
> -{
> - return kzalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
> -}
> -
> -static void zcomp_lz4_destroy(void *private)
> -{
> - kfree(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 da1bc47..0000000
> --- a/drivers/block/zram/zcomp_lzo.c
> +++ /dev/null
> @@ -1,47 +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 "zcomp_lzo.h"
> -
> -static void *lzo_create(void)
> -{
> - return kzalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
> -}
> -
> -static void lzo_destroy(void *private)
> -{
> - kfree(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_ */
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 4801e4d..b3044d3 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -30,6 +30,7 @@
> #include <linux/err.h>
> #include <linux/idr.h>
> #include <linux/sysfs.h>
> +#include <linux/crypto.h>
>
> #include "zram_drv.h"
>
> @@ -378,7 +379,7 @@ static ssize_t comp_algorithm_store(struct device *dev,
> if (sz > 0 && zram->compressor[sz - 1] == '\n')
> zram->compressor[sz - 1] = 0x00;
>
> - if (!zcomp_available_algorithm(zram->compressor))
> + if (!crypto_has_comp(zram->compressor, 0, 0))
> len = -EINVAL;
No.
> up_write(&zram->init_lock);
> @@ -562,13 +563,14 @@ static void zram_free_page(struct zram *zram, size_t index)
> zram_set_obj_size(meta, index, 0);
> }
>
> -static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> +static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
> + char *mem, u32 index)
> {
> int ret = 0;
> unsigned char *cmem;
> struct zram_meta *meta = zram->meta;
> unsigned long handle;
> - size_t size;
> + unsigned int size;
>
> bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> handle = meta->table[index].handle;
> @@ -584,7 +586,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> if (size == PAGE_SIZE)
> copy_page(mem, cmem);
> else
> - ret = zcomp_decompress(zram->comp, cmem, size, mem);
> + ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
> zs_unmap_object(meta->mem_pool, handle);
> bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
>
> @@ -604,6 +606,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> struct page *page;
> unsigned char *user_mem, *uncmem = NULL;
> struct zram_meta *meta = zram->meta;
> + struct zcomp_strm *zstrm;
> +
> page = bvec->bv_page;
>
> bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> @@ -619,6 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> /* Use a temporary buffer to decompress the page */
> uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
>
> + zstrm = zcomp_strm_find(zram->comp);
No.
zcomp_decompress_begin()/zcomp_decompress_end() please, and then just
change them to return NULL or zstrm when needed.
don't change zcomp_strm_find() to return NULL. that completely brakes
zcomp design. it always return zstream -- immediately (if there is an
idle zstrm) or after sleep.
> user_mem = kmap_atomic(page);
> if (!is_partial_io(bvec))
> uncmem = user_mem;
> @@ -629,7 +634,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> goto out_cleanup;
> }
>
> - ret = zram_decompress_page(zram, uncmem, index);
> + ret = zram_decompress_page(zram, zstrm, uncmem, index);
> /* Should NEVER happen. Return bio error if it does. */
> if (unlikely(ret))
> goto out_cleanup;
> @@ -642,6 +647,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> ret = 0;
> out_cleanup:
> kunmap_atomic(user_mem);
> + zcomp_strm_release(zram->comp, zstrm);
> if (is_partial_io(bvec))
> kfree(uncmem);
> return ret;
> @@ -651,7 +657,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> int offset)
> {
> int ret = 0;
> - size_t clen;
> + unsigned int clen;
> unsigned long handle;
> struct page *page;
> unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
> @@ -670,12 +676,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> ret = -ENOMEM;
> goto out;
> }
> - ret = zram_decompress_page(zram, uncmem, index);
> + zstrm = zcomp_strm_find(zram->comp);
> + ret = zram_decompress_page(zram, zstrm, uncmem, index);
> if (ret)
> goto out;
> }
>
> - zstrm = zcomp_strm_find(zram->comp);
> + if (!zstrm)
> + zstrm = zcomp_strm_find(zram->comp);
> user_mem = kmap_atomic(page);
>
> if (is_partial_io(bvec)) {
> @@ -721,7 +729,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
>
> handle = zs_malloc(meta->mem_pool, clen);
> if (!handle) {
> - pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
> + pr_info("Error allocating memory for compressed page: %u, size=%u\n",
> index, clen);
Please rebase against the linux-next. I do believe that I have changed
that line a couple of weeks ago.
-ss
On (08/20/15 15:35), Joonsoo Kim wrote:
> Crypto subsystem supports noctx API which doesn't require tfm.
> To get tfm in zram, we need zstrm and it is limited resource.
> If we uses noctx API, we don't need to get zstrm so that
> we get much better performance when zstrm is insufficient.
>
> This patch restores zram's performance to the time that zram
> doesn't use crypto subsystem.
>
> Following is zram's read performance number.
>
> * iozone -t 4 -R -r 16K -s 60M -I +Z -i 0 -i 1
> * max_stream is set to 1
> * Output is in Kbytes/sec
>
> zram-base vs zram-crypto vs zram-crypto-noctx
>
> Read 10411701.88 6426911.62 9423894.38
> Re-read 10017386.62 6428218.88 11000063.50
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> drivers/block/zram/zcomp.c | 28 +++++++++++++++++++++++++++-
> drivers/block/zram/zcomp.h | 9 ++++++++-
> drivers/block/zram/zram_drv.c | 9 +++++----
> 3 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index d2734f2..86b0c9b 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -291,8 +291,12 @@ bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
> return comp->set_max_streams(comp, num_strm);
> }
>
> -struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
> +struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp)
> {
> + /* We don't need decompression context so zstrm neither */
> + if (decomp && test_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags))
> + return NULL;
> +
> return comp->strm_find(comp);
> }
No. zcomp_strm_find() should never return NULL. And no, I don't like
"if (decomp)" change.
>
> @@ -307,6 +311,11 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> {
> *dst_len = PAGE_SIZE << 1;
>
> + if (!zstrm) {
> + return crypto_comp_compress_noctx(comp->alg, src, PAGE_SIZE,
> + dst, dst_len);
> + }
> +
> return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE, dst, dst_len);
> }
>
> @@ -316,12 +325,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> {
> unsigned int size = PAGE_SIZE;
>
> + if (!zstrm) {
> + return crypto_comp_decompress_noctx(comp->alg, src, src_len,
> + dst, &size);
> + }
> +
> return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
> }
>
> void zcomp_destroy(struct zcomp *comp)
> {
> comp->destroy(comp);
> + crypto_put_comp(comp->alg);
> kfree(comp);
> }
>
> @@ -344,12 +359,23 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> return ERR_PTR(-ENOMEM);
>
> comp->name = compress;
> + comp->alg = crypto_get_comp(compress, 0, 0);
> + if (!comp->alg) {
> + kfree(comp);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (crypto_has_compress_noctx(comp->alg))
> + set_bit(ZCOMP_COMPRESS_NOCTX, &comp->flags);
do you use ZCOMP_COMPRESS_NOCTX algs in this patch set?
> + if (crypto_has_decompress_noctx(comp->alg))
> + set_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags);
>
> if (max_strm > 1)
> zcomp_strm_multi_create(comp, max_strm);
> else
> zcomp_strm_single_create(comp);
> if (!comp->stream) {
> + crypto_put_comp(comp->alg);
> kfree(comp);
> return ERR_PTR(-ENOMEM);
> }
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index c47db4d..6c137c8 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -13,6 +13,11 @@
> #include <linux/crypto.h>
> #include <linux/mutex.h>
>
> +enum {
> + ZCOMP_COMPRESS_NOCTX,
> + ZCOMP_DECOMPRESS_NOCTX,
> +};
Can it be handled via crypto api? check if ->foo_noctx() is !NULL ?
> struct zcomp_strm {
> struct crypto_comp *tfm;
>
> @@ -27,6 +32,8 @@ struct zcomp_strm {
> struct zcomp {
> void *stream;
> const char *name;
> + struct crypto_alg *alg;
> + unsigned long flags;
>
> struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> @@ -39,7 +46,7 @@ ssize_t zcomp_available_show(const char *comp, char *buf);
> struct zcomp *zcomp_create(const char *comp, int max_strm);
> void zcomp_destroy(struct zcomp *comp);
>
> -struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
> +struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp);
> void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index b3044d3..8283bd3 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -623,7 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> /* Use a temporary buffer to decompress the page */
> uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
>
> - zstrm = zcomp_strm_find(zram->comp);
> + zstrm = zcomp_strm_find(zram->comp, true);
No, I don't like this.
> user_mem = kmap_atomic(page);
> if (!is_partial_io(bvec))
> uncmem = user_mem;
> @@ -647,7 +647,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> ret = 0;
> out_cleanup:
> kunmap_atomic(user_mem);
> - zcomp_strm_release(zram->comp, zstrm);
> + if (zstrm)
> + zcomp_strm_release(zram->comp, zstrm);
> if (is_partial_io(bvec))
> kfree(uncmem);
> return ret;
> @@ -676,14 +677,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> ret = -ENOMEM;
> goto out;
> }
> - zstrm = zcomp_strm_find(zram->comp);
> + zstrm = zcomp_strm_find(zram->comp, true);
> ret = zram_decompress_page(zram, zstrm, uncmem, index);
> if (ret)
> goto out;
> }
>
> if (!zstrm)
> - zstrm = zcomp_strm_find(zram->comp);
> + zstrm = zcomp_strm_find(zram->comp, false);
No. I don't like this.
-ss
On (08/27/15 13:07), Sergey Senozhatsky wrote:
[..]
> > -struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
> > +struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp)
> >
I think we can hide zcomp_strm_find()/_release (make them static),
and instead introduce:
a) zcomp_decompress_begin()/zcomp_decompress_end()
calls zcomp_strm_find()/zcomp_strm_release() for tfms that
require context for decompression; may return NULL, may sleep.
b) zcomp_compress_begin()/zcomp_compress_end()
always calls zcomp_strm_find()/zcomp_strm_release();
never return NULL; may sleep.
-ss
> > {
> > + /* We don't need decompression context so zstrm neither */
> > + if (decomp && test_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags))
> > + return NULL;
> > +
> > return comp->strm_find(comp);
> > }
>
> No. zcomp_strm_find() should never return NULL. And no, I don't like
> "if (decomp)" change.
>
>
> >
> > @@ -307,6 +311,11 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > {
> > *dst_len = PAGE_SIZE << 1;
> >
> > + if (!zstrm) {
> > + return crypto_comp_compress_noctx(comp->alg, src, PAGE_SIZE,
> > + dst, dst_len);
> > + }
> > +
> > return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE, dst, dst_len);
> > }
> >
> > @@ -316,12 +325,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > {
> > unsigned int size = PAGE_SIZE;
> >
> > + if (!zstrm) {
> > + return crypto_comp_decompress_noctx(comp->alg, src, src_len,
> > + dst, &size);
> > + }
> > +
> > return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
> > }
> >
> > void zcomp_destroy(struct zcomp *comp)
> > {
> > comp->destroy(comp);
> > + crypto_put_comp(comp->alg);
> > kfree(comp);
> > }
> >
> > @@ -344,12 +359,23 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> > return ERR_PTR(-ENOMEM);
> >
> > comp->name = compress;
> > + comp->alg = crypto_get_comp(compress, 0, 0);
> > + if (!comp->alg) {
> > + kfree(comp);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + if (crypto_has_compress_noctx(comp->alg))
> > + set_bit(ZCOMP_COMPRESS_NOCTX, &comp->flags);
>
> do you use ZCOMP_COMPRESS_NOCTX algs in this patch set?
>
>
> > + if (crypto_has_decompress_noctx(comp->alg))
> > + set_bit(ZCOMP_DECOMPRESS_NOCTX, &comp->flags);
> >
> > if (max_strm > 1)
> > zcomp_strm_multi_create(comp, max_strm);
> > else
> > zcomp_strm_single_create(comp);
> > if (!comp->stream) {
> > + crypto_put_comp(comp->alg);
> > kfree(comp);
> > return ERR_PTR(-ENOMEM);
> > }
> > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> > index c47db4d..6c137c8 100644
> > --- a/drivers/block/zram/zcomp.h
> > +++ b/drivers/block/zram/zcomp.h
> > @@ -13,6 +13,11 @@
> > #include <linux/crypto.h>
> > #include <linux/mutex.h>
> >
> > +enum {
> > + ZCOMP_COMPRESS_NOCTX,
> > + ZCOMP_DECOMPRESS_NOCTX,
> > +};
>
> Can it be handled via crypto api? check if ->foo_noctx() is !NULL ?
>
> > struct zcomp_strm {
> > struct crypto_comp *tfm;
> >
> > @@ -27,6 +32,8 @@ struct zcomp_strm {
> > struct zcomp {
> > void *stream;
> > const char *name;
> > + struct crypto_alg *alg;
> > + unsigned long flags;
> >
> > struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> > void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> > @@ -39,7 +46,7 @@ ssize_t zcomp_available_show(const char *comp, char *buf);
> > struct zcomp *zcomp_create(const char *comp, int max_strm);
> > void zcomp_destroy(struct zcomp *comp);
> >
> > -struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
> > +struct zcomp_strm *zcomp_strm_find(struct zcomp *comp, bool decomp);
> > void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
> >
> > int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index b3044d3..8283bd3 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -623,7 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> > /* Use a temporary buffer to decompress the page */
> > uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
> >
> > - zstrm = zcomp_strm_find(zram->comp);
> > + zstrm = zcomp_strm_find(zram->comp, true);
>
> No, I don't like this.
>
> > user_mem = kmap_atomic(page);
> > if (!is_partial_io(bvec))
> > uncmem = user_mem;
> > @@ -647,7 +647,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> > ret = 0;
> > out_cleanup:
> > kunmap_atomic(user_mem);
> > - zcomp_strm_release(zram->comp, zstrm);
> > + if (zstrm)
> > + zcomp_strm_release(zram->comp, zstrm);
> > if (is_partial_io(bvec))
> > kfree(uncmem);
> > return ret;
> > @@ -676,14 +677,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> > ret = -ENOMEM;
> > goto out;
> > }
> > - zstrm = zcomp_strm_find(zram->comp);
> > + zstrm = zcomp_strm_find(zram->comp, true);
> > ret = zram_decompress_page(zram, zstrm, uncmem, index);
> > if (ret)
> > goto out;
> > }
> >
> > if (!zstrm)
> > - zstrm = zcomp_strm_find(zram->comp);
> > + zstrm = zcomp_strm_find(zram->comp, false);
>
> No. I don't like this.
>
> -ss
>