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 (7):
crypto: introduce decompression API that can be called via sharable
tfm object
crypto/lzo: support decompress_noctx
crypyo/lz4: support decompress_noctx
crypto/lz4hc: support decompress_noctx
crypto/842: support decompress_noctx
zram: use crypto API for compression
zram: use crypto decompress_noctx API
Sergey Senozhatsky (2):
zram: make stream find and release functions static
zram: pass zstrm down to decompression path
crypto/842.c | 9 +++-
crypto/compress.c | 36 +++++++++++++++
crypto/crypto_null.c | 3 +-
crypto/deflate.c | 3 +-
crypto/lz4.c | 9 +++-
crypto/lz4hc.c | 9 +++-
crypto/lzo.c | 9 +++-
drivers/block/zram/Kconfig | 8 ++--
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 102 +++++++++++++++++++++++++++++++----------
drivers/block/zram/zcomp.h | 42 +++++++----------
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 | 32 +++++++++----
include/linux/crypto.h | 20 ++++++++
17 files changed, 211 insertions(+), 203 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
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 | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/crypto/lzo.c b/crypto/lzo.c
index ec0f7b3..3cc0ce7 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -80,6 +80,12 @@ 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)
+{
+ return lzo_decompress(NULL, src, slen, dst, dlen);
+}
+
static struct crypto_alg alg = {
.cra_name = "lzo",
.cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
@@ -90,7 +96,7 @@ static struct crypto_alg alg = {
.cra_u = { .compress = {
.coa_compress = lzo_compress,
.coa_decompress = lzo_decompress,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = lzo_decompress_noctx } }
};
static int __init lzo_mod_init(void)
--
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
decompression API that calls decompression function via sharable tfm
object. Concurrent calls to decompress_noctx function through sharable
tfm object will be okay because caller don't need any context in tfm and
tfm is only used for fetching function pointer to decompress_noctx
function. This can reduce overhead of maintaining multiple tfm
if decompression doesn't require any context to operate.
Signed-off-by: Joonsoo Kim <[email protected]>
---
crypto/842.c | 3 ++-
crypto/compress.c | 36 ++++++++++++++++++++++++++++++++++++
crypto/crypto_null.c | 3 ++-
crypto/deflate.c | 3 ++-
crypto/lz4.c | 3 ++-
crypto/lz4hc.c | 3 ++-
crypto/lzo.c | 3 ++-
include/linux/crypto.h | 20 ++++++++++++++++++++
8 files changed, 68 insertions(+), 6 deletions(-)
diff --git a/crypto/842.c b/crypto/842.c
index 98e387e..1b6cdab 100644
--- a/crypto/842.c
+++ b/crypto/842.c
@@ -61,7 +61,8 @@ 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_decompress_noctx = NULL } }
};
static int __init crypto842_mod_init(void)
diff --git a/crypto/compress.c b/crypto/compress.c
index c33f076..abb36a8 100644
--- a/crypto/compress.c
+++ b/crypto/compress.c
@@ -33,12 +33,21 @@ static int crypto_decompress(struct crypto_tfm *tfm,
dlen);
}
+static int crypto_decompress_noctx(struct crypto_tfm *tfm,
+ const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ return tfm->__crt_alg->cra_compress.coa_decompress_noctx(src, slen,
+ dst, dlen);
+}
+
int crypto_init_compress_ops(struct crypto_tfm *tfm)
{
struct compress_tfm *ops = &tfm->crt_compress;
ops->cot_compress = crypto_compress;
ops->cot_decompress = crypto_decompress;
+ ops->cot_decompress_noctx = NULL;
return 0;
}
@@ -46,3 +55,30 @@ int crypto_init_compress_ops(struct crypto_tfm *tfm)
void crypto_exit_compress_ops(struct crypto_tfm *tfm)
{
}
+
+struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
+ u32 type, u32 mask)
+{
+ struct crypto_comp *comp;
+ struct crypto_tfm *tfm;
+ struct compress_tfm *ops;
+
+ comp = crypto_alloc_comp(alg_name, type, mask);
+ if (IS_ERR(comp))
+ return comp;
+
+ tfm = crypto_comp_tfm(comp);
+ if (!tfm->__crt_alg->cra_compress.coa_decompress_noctx) {
+ crypto_free_comp(comp);
+ return ERR_PTR(-EINVAL);
+ }
+
+ ops = &tfm->crt_compress;
+
+ /* Only allow noctx ops to comp_noctx */
+ ops->cot_compress = NULL;
+ ops->cot_decompress = NULL;
+ ops->cot_decompress_noctx = crypto_decompress_noctx;
+
+ return comp;
+}
diff --git a/crypto/crypto_null.c b/crypto/crypto_null.c
index 941c9a4..3560b75 100644
--- a/crypto/crypto_null.c
+++ b/crypto/crypto_null.c
@@ -146,7 +146,8 @@ 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_decompress_noctx = NULL } }
} };
MODULE_ALIAS_CRYPTO("compress_null");
diff --git a/crypto/deflate.c b/crypto/deflate.c
index 95d8d37..c0b0a40 100644
--- a/crypto/deflate.c
+++ b/crypto/deflate.c
@@ -203,7 +203,8 @@ 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_decompress_noctx = NULL } }
};
static int __init deflate_mod_init(void)
diff --git a/crypto/lz4.c b/crypto/lz4.c
index aefbcea..d38ce2a 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -86,7 +86,8 @@ 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_decompress_noctx = NULL } }
};
static int __init lz4_mod_init(void)
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index a1d3b5b..0cb38a7 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -86,7 +86,8 @@ 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_decompress_noctx = NULL } }
};
static int __init lz4hc_mod_init(void)
diff --git a/crypto/lzo.c b/crypto/lzo.c
index 4b3e925..ec0f7b3 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -89,7 +89,8 @@ 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_decompress_noctx = NULL } }
};
static int __init lzo_mod_init(void)
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index e71cb70..31152b1 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -355,6 +355,8 @@ 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_decompress_noctx)(const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen);
};
@@ -538,6 +540,9 @@ struct compress_tfm {
int (*cot_decompress)(struct crypto_tfm *tfm,
const u8 *src, unsigned int slen,
u8 *dst, unsigned int *dlen);
+ int (*cot_decompress_noctx)(struct crypto_tfm *tfm,
+ const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen);
};
#define crt_ablkcipher crt_u.ablkcipher
@@ -1836,6 +1841,14 @@ static inline void crypto_free_comp(struct crypto_comp *tfm)
crypto_free_tfm(crypto_comp_tfm(tfm));
}
+struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
+ u32 type, u32 mask);
+
+static inline void crypto_free_comp_noctx(struct crypto_comp *tfm)
+{
+ crypto_free_comp(tfm);
+}
+
static inline int crypto_has_comp(const char *alg_name, u32 type, u32 mask)
{
type &= ~CRYPTO_ALG_TYPE_MASK;
@@ -1871,5 +1884,12 @@ static inline int crypto_comp_decompress(struct crypto_comp *tfm,
src, slen, dst, dlen);
}
+static inline int crypto_comp_decompress_noctx(struct crypto_comp *tfm,
+ const u8 *src, unsigned int slen,
+ u8 *dst, unsigned int *dlen)
+{
+ return crypto_comp_crt(tfm)->cot_decompress_noctx(crypto_comp_tfm(tfm),
+ src, slen, dst, dlen);
+}
#endif /* _LINUX_CRYPTO_H */
--
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 | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index 0cb38a7..0797065 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -76,6 +76,12 @@ 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)
+{
+ return lz4hc_decompress_crypto(NULL, src, slen, dst, dlen);
+}
+
static struct crypto_alg alg_lz4hc = {
.cra_name = "lz4hc",
.cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
@@ -87,7 +93,7 @@ static struct crypto_alg alg_lz4hc = {
.cra_u = { .compress = {
.coa_compress = lz4hc_compress_crypto,
.coa_decompress = lz4hc_decompress_crypto,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = lz4hc_decompress_noctx } }
};
static int __init lz4hc_mod_init(void)
--
1.9.1
From: Sergey Senozhatsky <[email protected]>
Hide (make static) zstrm find and release function and introduce
zcomp_compress_begin()/zcomp_compress_end(). We will have begin
and end functions around compression (this patch) and decompression
(next patch). So the work flow is evolving to:
zstrm = foo_begin();
foo(zstrm);
foo_end(zstrm);
where foo is compress or decompress zcomp functions.
This patch is a preparation to make crypto API-powered zcomp
possible. The reasoning is that some crypto compression backends
require zstrm for decompression.
Signed-off-by: Sergey Senozhatsky <[email protected]>
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/zcomp.c | 27 +++++++++++++++++++++++++--
drivers/block/zram/zcomp.h | 8 ++++++--
drivers/block/zram/zram_drv.c | 6 +++---
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 965d1af..fc13bf2 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -296,16 +296,39 @@ 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)
+static struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
{
return comp->strm_find(comp);
}
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
+static void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
{
comp->strm_release(comp, zstrm);
}
+/* Never return NULL, may sleep */
+struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp)
+{
+ return zcomp_strm_find(comp);
+}
+
+void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ zcomp_strm_release(comp, zstrm);
+}
+
+/* May return NULL, may sleep */
+struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
+{
+ return NULL;
+}
+
+void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ if (zstrm)
+ zcomp_strm_release(comp, zstrm);
+}
+
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src, size_t *dst_len)
{
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 46e2b9f..616e013 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -56,12 +56,16 @@ bool zcomp_available_algorithm(const char *comp);
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);
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
+
+struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp);
+void zcomp_compress_end(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);
+struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
+void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
+
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 9fa15bb..20c41ed 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -673,7 +673,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
goto out;
}
- zstrm = zcomp_strm_find(zram->comp);
+ zstrm = zcomp_compress_begin(zram->comp);
user_mem = kmap_atomic(page);
if (is_partial_io(bvec)) {
@@ -744,7 +744,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
memcpy(cmem, src, clen);
}
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_compress_end(zram->comp, zstrm);
zstrm = NULL;
zs_unmap_object(meta->mem_pool, handle);
@@ -764,7 +764,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
atomic64_inc(&zram->stats.pages_stored);
out:
if (zstrm)
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_compress_end(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
--
1.9.1
Crypto subsystem now supports decompress_noctx API that requires
special tfm_noctx. This tfm can be shared by multiple concurrent
decompress user because this API doesn't rely on this tfm object
except to fetch decompress function pointer.
Until changing to use crypto API, zram doesn't require any zstrm
on decompress so decompress is parallelized unlimitedly. But, previous
patch make zram to use crypto API and this requires one zstrm on every
decompress users so, in some zstrm contended situations, zram's
performance would be degraded.
This patch makes zram use decompress_noctx API and restore zram's
performance as the time that zram doesn't use crypto API.
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 | 24 +++++++++++++++++++++++-
drivers/block/zram/zcomp.h | 1 +
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index c2ed7c8..a020200 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -319,9 +319,12 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
zcomp_strm_release(comp, zstrm);
}
-/* Never return NULL, may sleep */
+/* May return NULL, may sleep */
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
{
+ if (comp->tfm_noctx)
+ return NULL;
+
return zcomp_strm_find(comp);
}
@@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
{
unsigned int size = PAGE_SIZE;
+ if (!zstrm) {
+ return crypto_comp_decompress_noctx(comp->tfm_noctx,
+ src, src_len, dst, &size);
+ }
+
return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
}
void zcomp_destroy(struct zcomp *comp)
{
+ if (comp->tfm_noctx)
+ crypto_free_comp_noctx(comp->tfm_noctx);
comp->destroy(comp);
kfree(comp);
}
@@ -366,6 +376,7 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
{
struct zcomp *comp;
const char *backend;
+ struct crypto_comp *tfm;
backend = find_backend(compress);
if (!backend)
@@ -384,5 +395,16 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
kfree(comp);
return ERR_PTR(-ENOMEM);
}
+
+ /*
+ * Prepare to use crypto decompress_noctx API. One tfm is required
+ * to initialize crypto algorithm properly and fetch corresponding
+ * function pointer. But, it is sharable for multiple concurrent
+ * decompress users.
+ */
+ tfm = crypto_alloc_comp_noctx(compress, 0, 0);
+ if (!IS_ERR(tfm))
+ comp->tfm_noctx = tfm;
+
return comp;
}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 4f9df8e..c76d8e4 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -26,6 +26,7 @@ struct zcomp_strm {
/* dynamic per-device compression frontend */
struct zcomp {
void *stream;
+ struct crypto_comp *tfm_noctx;
const char *backend;
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
--
1.9.1
From: Sergey Senozhatsky <[email protected]>
Introduce zcomp_decompress_begin()/zcomp_decompress_end() as a
preparation for crypto API-powered zcomp.
Change zcomp_decompress() signature to require zstrm parameter.
Unlike zcomp_compress_begin(), zcomp_decompress_begin() may return
zstrm if currently selected compression backend require zstrm for
decompression or NULL if it does not.
Signed-off-by: Sergey Senozhatsky <[email protected]>
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/zcomp.c | 3 ++-
drivers/block/zram/zcomp.h | 3 ++-
drivers/block/zram/zram_drv.c | 20 ++++++++++++++++----
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index fc13bf2..3456d5a 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -336,7 +336,8 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
zstrm->private);
}
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src,
size_t src_len, unsigned char *dst)
{
return comp->backend->decompress(src, src_len, dst);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 616e013..4c09c01 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -66,7 +66,8 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src,
size_t src_len, unsigned char *dst);
bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 20c41ed..55e09db1 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -560,7 +560,8 @@ 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;
@@ -582,7 +583,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);
@@ -602,6 +603,7 @@ 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 = NULL;
page = bvec->bv_page;
bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
@@ -617,6 +619,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_decompress_begin(zram->comp);
user_mem = kmap_atomic(page);
if (!is_partial_io(bvec))
uncmem = user_mem;
@@ -627,7 +630,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;
@@ -636,10 +639,14 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
memcpy(user_mem + bvec->bv_offset, uncmem + offset,
bvec->bv_len);
+ zcomp_decompress_end(zram->comp, zstrm);
+ zstrm = NULL;
+
flush_dcache_page(page);
ret = 0;
out_cleanup:
kunmap_atomic(user_mem);
+ zcomp_decompress_end(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
@@ -659,6 +666,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
page = bvec->bv_page;
if (is_partial_io(bvec)) {
+ struct zcomp_strm *zstrm;
/*
* This is a partial IO. We need to read the full page
* before to write the changes.
@@ -668,7 +676,11 @@ 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_decompress_begin(zram->comp);
+ ret = zram_decompress_page(zram, zstrm, uncmem, index);
+ zcomp_decompress_end(zram->comp, zstrm);
+
if (ret)
goto out;
}
--
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 decompress_noctx API and would restore the performance as is.
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/Kconfig | 8 +++----
drivers/block/zram/Makefile | 4 +---
drivers/block/zram/zcomp.c | 54 +++++++++++++++++++++++-------------------
drivers/block/zram/zcomp.h | 30 ++++++-----------------
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 | 6 ++---
9 files changed, 44 insertions(+), 186 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..7619bed 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, ...).
@@ -18,9 +17,8 @@ config ZRAM
config ZRAM_LZ4_COMPRESS
bool "Enable LZ4 algorithm support"
depends on ZRAM
- select LZ4_COMPRESS
- select LZ4_DECOMPRESS
+ select CRYPTO_LZ4
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
+ 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 3456d5a..c2ed7c8 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,10 +15,6 @@
#include <linux/sched.h>
#include "zcomp.h"
-#include "zcomp_lzo.h"
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
-#include "zcomp_lz4.h"
-#endif
/*
* single zcomp_strm backend
@@ -43,19 +39,20 @@ struct zcomp_strm_multi {
wait_queue_head_t strm_wait;
};
-static struct zcomp_backend *backends[] = {
- &zcomp_lzo,
+static const char * const backends[] = {
+ "lzo",
#ifdef CONFIG_ZRAM_LZ4_COMPRESS
- &zcomp_lz4,
+ "lz4",
#endif
NULL
};
-static struct zcomp_backend *find_backend(const char *compress)
+static const char *find_backend(const char *compress)
{
int i = 0;
while (backends[i]) {
- if (sysfs_streq(compress, backends[i]->name))
+ if (sysfs_streq(compress, backends[i]) &&
+ crypto_has_comp(compress, 0, 0))
break;
i++;
}
@@ -64,8 +61,8 @@ static struct zcomp_backend *find_backend(const char *compress)
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);
}
@@ -80,13 +77,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->backend, 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,12 +276,12 @@ 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");
@@ -317,10 +319,10 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
zcomp_strm_release(comp, zstrm);
}
-/* May return NULL, may sleep */
+/* Never return NULL, may sleep */
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
{
- return NULL;
+ return zcomp_strm_find(comp);
}
void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
@@ -330,17 +332,21 @@ void zcomp_decompress_end(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 int *dst_len)
{
- return comp->backend->compress(src, zstrm->buffer, dst_len,
- zstrm->private);
+ *dst_len = PAGE_SIZE << 1;
+
+ return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
+ zstrm->buffer, dst_len);
}
int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src,
- size_t src_len, unsigned char *dst)
+ 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)
@@ -359,7 +365,7 @@ void zcomp_destroy(struct zcomp *comp)
struct zcomp *zcomp_create(const char *compress, int max_strm)
{
struct zcomp *comp;
- struct zcomp_backend *backend;
+ const char *backend;
backend = find_backend(compress);
if (!backend)
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 4c09c01..4f9df8e 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -11,38 +11,22 @@
#define _ZCOMP_H_
#include <linux/mutex.h>
+#include <linux/crypto.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 *backend;
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
@@ -61,14 +45,14 @@ struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp);
void zcomp_compress_end(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 int *dst_len);
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src,
- size_t src_len, unsigned char *dst);
+ 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 55e09db1..834f452 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -567,7 +567,7 @@ static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
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;
@@ -656,7 +656,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;
@@ -731,7 +731,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_err("Error allocating memory for compressed page: %u, size=%zu\n",
+ pr_err("Error allocating memory for compressed page: %u, size=%u\n",
index, clen);
ret = -ENOMEM;
goto out;
--
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 1b6cdab..a933f71 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",
@@ -62,7 +68,7 @@ static struct crypto_alg alg = {
.cra_u = { .compress = {
.coa_compress = crypto842_compress,
.coa_decompress = crypto842_decompress,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = crypto842_decompress_noctx } }
};
static int __init crypto842_mod_init(void)
--
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 | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/crypto/lz4.c b/crypto/lz4.c
index d38ce2a..589fa251 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -76,6 +76,12 @@ 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)
+{
+ return lz4_decompress_crypto(NULL, src, slen, dst, dlen);
+}
+
static struct crypto_alg alg_lz4 = {
.cra_name = "lz4",
.cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
@@ -87,7 +93,7 @@ static struct crypto_alg alg_lz4 = {
.cra_u = { .compress = {
.coa_compress = lz4_compress_crypto,
.coa_decompress = lz4_decompress_crypto,
- .coa_decompress_noctx = NULL } }
+ .coa_decompress_noctx = lz4_decompress_noctx } }
};
static int __init lz4_mod_init(void)
--
1.9.1
On Fri, Sep 18, 2015 at 02:19:21PM +0900, Joonsoo Kim wrote:
> From: Sergey Senozhatsky <[email protected]>
>
> Hide (make static) zstrm find and release function and introduce
> zcomp_compress_begin()/zcomp_compress_end(). We will have begin
> and end functions around compression (this patch) and decompression
> (next patch). So the work flow is evolving to:
>
> zstrm = foo_begin();
> foo(zstrm);
> foo_end(zstrm);
>
> where foo is compress or decompress zcomp functions.
>
> This patch is a preparation to make crypto API-powered zcomp
> possible. The reasoning is that some crypto compression backends
> require zstrm for decompression.
>
> Signed-off-by: Sergey Senozhatsky <[email protected]>
> Signed-off-by: Joonsoo Kim <[email protected]>
Acked-by: Minchan Kim <[email protected]>
On Fri, Sep 18, 2015 at 02:19:22PM +0900, Joonsoo Kim wrote:
> From: Sergey Senozhatsky <[email protected]>
>
> Introduce zcomp_decompress_begin()/zcomp_decompress_end() as a
> preparation for crypto API-powered zcomp.
>
> Change zcomp_decompress() signature to require zstrm parameter.
>
> Unlike zcomp_compress_begin(), zcomp_decompress_begin() may return
> zstrm if currently selected compression backend require zstrm for
> decompression or NULL if it does not.
>
> Signed-off-by: Sergey Senozhatsky <[email protected]>
> Signed-off-by: Joonsoo Kim <[email protected]>
Acked-by: Minchan Kim <[email protected]>
There is a trivial comment below.
> ---
> drivers/block/zram/zcomp.c | 3 ++-
> drivers/block/zram/zcomp.h | 3 ++-
> drivers/block/zram/zram_drv.c | 20 ++++++++++++++++----
> 3 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index fc13bf2..3456d5a 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -336,7 +336,8 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> zstrm->private);
> }
>
> -int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> + const unsigned char *src,
> size_t src_len, unsigned char *dst)
> {
> return comp->backend->decompress(src, src_len, dst);
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 616e013..4c09c01 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -66,7 +66,8 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
> void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
>
> -int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> + const unsigned char *src,
> size_t src_len, unsigned char *dst);
>
> bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 20c41ed..55e09db1 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -560,7 +560,8 @@ 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;
> @@ -582,7 +583,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);
>
> @@ -602,6 +603,7 @@ 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 = NULL;
No need to initialize with NULL.
Hi Joonsoo,
On Fri, Sep 18, 2015 at 02:19:23PM +0900, Joonsoo Kim wrote:
> 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 decompress_noctx API and would restore the performance as is.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> drivers/block/zram/Kconfig | 8 +++----
> drivers/block/zram/Makefile | 4 +---
> drivers/block/zram/zcomp.c | 54 +++++++++++++++++++++++-------------------
> drivers/block/zram/zcomp.h | 30 ++++++-----------------
> 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 | 6 ++---
> 9 files changed, 44 insertions(+), 186 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..7619bed 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, ...).
> @@ -18,9 +17,8 @@ config ZRAM
> config ZRAM_LZ4_COMPRESS
> bool "Enable LZ4 algorithm support"
> depends on ZRAM
> - select LZ4_COMPRESS
> - select LZ4_DECOMPRESS
> + select CRYPTO_LZ4
It is out of my expectation.
When I heard crypto support for zRAM first, I imagined zram user can
use any crypto compressor algorithm easily if system has the algorithm.
IOW, I expect zRAM shouldn't bind CONFIG_CRYPTO_ALGONAME statically
except the default one(ie, CONFIG_CRYPTO_LZO) like zswap and It seems
you did in eariler version.
You seem to have a trouble to adapt crypto to current comp_algorithm
because crypto doesn't export any API to enumerate algorithm name
while zram should export supporting algorithm name via comp_algorithm
and I heard crypto maintainer doesn't want to export it. Instead,
user can use /proc/crypto to know what kinds of compressor system
can support.
Hmm,
At the first glance, I was about to say "let's sort it out with
futher patches" but I changed my mind. ;-).
We should sort it out before you are adding zlib support(ie, please
include zlib support patch with number data in this patchset). Otherwise,
we should add new hard-wired stuff for zlib like lzo, lz4 to
comp_algorithm and will depreate soon.
My idea is ABI change of comp_algorithm. Namely, let's deprecate it
and guide to use /proc/crypto to show available compressor.
Someday, we removes backends string array finally.
Welcome to any ideas.
> 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
> + 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 3456d5a..c2ed7c8 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -15,10 +15,6 @@
> #include <linux/sched.h>
>
> #include "zcomp.h"
> -#include "zcomp_lzo.h"
> -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> -#include "zcomp_lz4.h"
> -#endif
>
> /*
> * single zcomp_strm backend
> @@ -43,19 +39,20 @@ struct zcomp_strm_multi {
> wait_queue_head_t strm_wait;
> };
>
> -static struct zcomp_backend *backends[] = {
> - &zcomp_lzo,
> +static const char * const backends[] = {
> + "lzo",
> #ifdef CONFIG_ZRAM_LZ4_COMPRESS
> - &zcomp_lz4,
> + "lz4",
> #endif
> NULL
> };
>
> -static struct zcomp_backend *find_backend(const char *compress)
> +static const char *find_backend(const char *compress)
> {
> int i = 0;
> while (backends[i]) {
> - if (sysfs_streq(compress, backends[i]->name))
> + if (sysfs_streq(compress, backends[i]) &&
> + crypto_has_comp(compress, 0, 0))
> break;
> i++;
> }
> @@ -64,8 +61,8 @@ static struct zcomp_backend *find_backend(const char *compress)
>
> 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);
> }
> @@ -80,13 +77,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->backend, 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,12 +276,12 @@ 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");
> @@ -317,10 +319,10 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
> zcomp_strm_release(comp, zstrm);
> }
>
> -/* May return NULL, may sleep */
> +/* Never return NULL, may sleep */
> struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> {
> - return NULL;
> + return zcomp_strm_find(comp);
> }
>
> void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
> @@ -330,17 +332,21 @@ void zcomp_decompress_end(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 int *dst_len)
> {
> - return comp->backend->compress(src, zstrm->buffer, dst_len,
> - zstrm->private);
> + *dst_len = PAGE_SIZE << 1;
> +
> + return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
> + zstrm->buffer, dst_len);
> }
>
> int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> const unsigned char *src,
> - size_t src_len, unsigned char *dst)
> + 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)
> @@ -359,7 +365,7 @@ void zcomp_destroy(struct zcomp *comp)
> struct zcomp *zcomp_create(const char *compress, int max_strm)
> {
> struct zcomp *comp;
> - struct zcomp_backend *backend;
> + const char *backend;
>
> backend = find_backend(compress);
> if (!backend)
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 4c09c01..4f9df8e 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -11,38 +11,22 @@
> #define _ZCOMP_H_
>
> #include <linux/mutex.h>
> +#include <linux/crypto.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 *backend;
>
> struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> @@ -61,14 +45,14 @@ struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp);
> void zcomp_compress_end(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 int *dst_len);
>
> struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
> void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
>
> int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> const unsigned char *src,
> - size_t src_len, unsigned char *dst);
> + 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 55e09db1..834f452 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -567,7 +567,7 @@ static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
> 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;
> @@ -656,7 +656,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;
> @@ -731,7 +731,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_err("Error allocating memory for compressed page: %u, size=%zu\n",
> + pr_err("Error allocating memory for compressed page: %u, size=%u\n",
> index, clen);
> ret = -ENOMEM;
> goto out;
> --
> 1.9.1
>
On Fri, Sep 18, 2015 at 02:19:24PM +0900, Joonsoo Kim wrote:
> Crypto subsystem now supports decompress_noctx API that requires
> special tfm_noctx. This tfm can be shared by multiple concurrent
> decompress user because this API doesn't rely on this tfm object
> except to fetch decompress function pointer.
>
> Until changing to use crypto API, zram doesn't require any zstrm
> on decompress so decompress is parallelized unlimitedly. But, previous
> patch make zram to use crypto API and this requires one zstrm on every
> decompress users so, in some zstrm contended situations, zram's
> performance would be degraded.
>
> This patch makes zram use decompress_noctx API and restore zram's
> performance as the time that zram doesn't use crypto API.
>
> 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 | 24 +++++++++++++++++++++++-
> drivers/block/zram/zcomp.h | 1 +
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index c2ed7c8..a020200 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -319,9 +319,12 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
> zcomp_strm_release(comp, zstrm);
> }
>
> -/* Never return NULL, may sleep */
> +/* May return NULL, may sleep */
> struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> {
> + if (comp->tfm_noctx)
> + return NULL;
Hmm,, I understand why returns NULL but it seems to be awkward to use NULL
to express meaningful semantic and following functions relies on.
If I have a time, I will think over.
> +
> return zcomp_strm_find(comp);
> }
>
> @@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> {
> unsigned int size = PAGE_SIZE;
>
> + if (!zstrm) {
> + return crypto_comp_decompress_noctx(comp->tfm_noctx,
> + src, src_len, dst, &size);
> + }
> +
> return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
> }
>
> void zcomp_destroy(struct zcomp *comp)
> {
> + if (comp->tfm_noctx)
> + crypto_free_comp_noctx(comp->tfm_noctx);
> comp->destroy(comp);
> kfree(comp);
> }
> @@ -366,6 +376,7 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> {
> struct zcomp *comp;
> const char *backend;
> + struct crypto_comp *tfm;
>
> backend = find_backend(compress);
> if (!backend)
> @@ -384,5 +395,16 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> kfree(comp);
> return ERR_PTR(-ENOMEM);
> }
> +
> + /*
> + * Prepare to use crypto decompress_noctx API. One tfm is required
> + * to initialize crypto algorithm properly and fetch corresponding
> + * function pointer. But, it is sharable for multiple concurrent
> + * decompress users.
> + */
Please comment out that this API will return NULL if the compressor doesn't
support noctx mode.
Thanks.
On Fri, Sep 18, 2015 at 02:19:15PM +0900, Joonsoo Kim wrote:
> 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.
Fair enough. lzo and lz4 cannot cover all of workloads so surely, there are
workloads other compressor algorithm can win. As well, we could support
H/W compressor with crypto support so I think crypto is a way to go.
One thing I have a concern is I don't want to bind some of compressor
algorithm to zram statically. User can see what kinds of crypto compressor
support in his system via /proc/crypto and use it dynamically.
I hope zram supports it.
>
> 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
Nice catch from Sergey!
> 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
Another patch and number we need to merge this patchset is zlib support
patchset and number you had experiement.
>
> Thanks.
>
> Joonsoo Kim (7):
> crypto: introduce decompression API that can be called via sharable
> tfm object
> crypto/lzo: support decompress_noctx
> crypyo/lz4: support decompress_noctx
> crypto/lz4hc: support decompress_noctx
> crypto/842: support decompress_noctx
> zram: use crypto API for compression
> zram: use crypto decompress_noctx API
>
> Sergey Senozhatsky (2):
> zram: make stream find and release functions static
> zram: pass zstrm down to decompression path
>
> crypto/842.c | 9 +++-
> crypto/compress.c | 36 +++++++++++++++
> crypto/crypto_null.c | 3 +-
> crypto/deflate.c | 3 +-
> crypto/lz4.c | 9 +++-
> crypto/lz4hc.c | 9 +++-
> crypto/lzo.c | 9 +++-
> drivers/block/zram/Kconfig | 8 ++--
> drivers/block/zram/Makefile | 4 +-
> drivers/block/zram/zcomp.c | 102 +++++++++++++++++++++++++++++++----------
> drivers/block/zram/zcomp.h | 42 +++++++----------
> 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 | 32 +++++++++----
> include/linux/crypto.h | 20 ++++++++
> 17 files changed, 211 insertions(+), 203 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
>
On (09/18/15 14:19), Joonsoo Kim wrote:
[..]
> -static struct zcomp_backend *find_backend(const char *compress)
> +static const char *find_backend(const char *compress)
> {
> int i = 0;
> while (backends[i]) {
> - if (sysfs_streq(compress, backends[i]->name))
> + if (sysfs_streq(compress, backends[i]) &&
> + crypto_has_comp(compress, 0, 0))
ok, just for note. zcomp does sysfs_streq(), because sysfs data
usually contain a trailing new line, crypto_has_comp() does strcmp().
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> - const unsigned char *src, size_t *dst_len)
> + const unsigned char *src, unsigned int *dst_len)
> {
> - return comp->backend->compress(src, zstrm->buffer, dst_len,
> - zstrm->private);
> + *dst_len = PAGE_SIZE << 1;
> +
hm... wouldn't it be better to say crypto api that we have a PAGE_SIZE
buffer instead of PAGE_SIZE << 1, so in case of buffer overrun (or
whatever is the correct term here) it will stop compression earlier
(well, possibly)? zram will drop compressed data larger than PAGE_SIZE
anyway, so if passing a smaller buffer size can save us CPU time then
let's do it.
> + return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
> + zstrm->buffer, dst_len);
> }
>
> int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> const unsigned char *src,
> - size_t src_len, unsigned char *dst)
> + 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);
^^^^^^^^ tab?
> }
>
> void zcomp_destroy(struct zcomp *comp)
> @@ -359,7 +365,7 @@ void zcomp_destroy(struct zcomp *comp)
> struct zcomp *zcomp_create(const char *compress, int max_strm)
> {
> struct zcomp *comp;
> - struct zcomp_backend *backend;
> + const char *backend;
rebase against the current linux-next. this and the next patch do not
apply cleanly. the function was touched recently: '+int error'.
>
> backend = find_backend(compress);
> if (!backend)
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 4c09c01..4f9df8e 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -11,38 +11,22 @@
> #define _ZCOMP_H_
>
> #include <linux/mutex.h>
> +#include <linux/crypto.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 *backend;
^^ what for?
-ss
On (09/18/15 14:19), Joonsoo Kim wrote:
> -/* Never return NULL, may sleep */
> +/* May return NULL, may sleep */
> struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> {
> + if (comp->tfm_noctx)
> + return NULL;
> +
> return zcomp_strm_find(comp);
> }
>
> @@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> {
> unsigned int size = PAGE_SIZE;
>
> + if (!zstrm) {
> + return crypto_comp_decompress_noctx(comp->tfm_noctx,
> + src, src_len, dst, &size);
> + }
unneeded braces.
-ss
On (09/18/15 14:19), Joonsoo Kim wrote:
[..]
> @@ -61,7 +61,8 @@ 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_decompress_noctx = NULL } }
> };
>
> static int __init crypto842_mod_init(void)
> diff --git a/crypto/compress.c b/crypto/compress.c
> index c33f076..abb36a8 100644
> --- a/crypto/compress.c
> +++ b/crypto/compress.c
> @@ -33,12 +33,21 @@ static int crypto_decompress(struct crypto_tfm *tfm,
> dlen);
> }
>
> +static int crypto_decompress_noctx(struct crypto_tfm *tfm,
> + const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen)
> +{
> + return tfm->__crt_alg->cra_compress.coa_decompress_noctx(src, slen,
> + dst, dlen);
> +}
hm... well... sorry, if irrelevant.
if the algorithm can have a _noctx() decompression function, does it
automatically guarantee that this algorithm never dereferences a passed
`struct crypto_tfm *tfm' pointer in its decompress function? in other words,
can we simply pass that `shared tfm pointer' to the existing decompress
function instead of defining new symbols, new callbacks, etc.?
cot_decompress_noctx() == cot_decompress(shared_ftm) ?
just a thought.
[..]
> +struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
> + u32 type, u32 mask)
> +{
> + struct crypto_comp *comp;
> + struct crypto_tfm *tfm;
> + struct compress_tfm *ops;
> +
> + comp = crypto_alloc_comp(alg_name, type, mask);
> + if (IS_ERR(comp))
> + return comp;
> +
> + tfm = crypto_comp_tfm(comp);
> + if (!tfm->__crt_alg->cra_compress.coa_decompress_noctx) {
> + crypto_free_comp(comp);
> + return ERR_PTR(-EINVAL);
-ENOMEM?
-ss
On (09/18/15 14:19), Joonsoo Kim wrote:
[..]
> static int __init lzo_mod_init(void)
> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index e71cb70..31152b1 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -355,6 +355,8 @@ 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_decompress_noctx)(const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen);
> };
>
>
> @@ -538,6 +540,9 @@ struct compress_tfm {
> int (*cot_decompress)(struct crypto_tfm *tfm,
> const u8 *src, unsigned int slen,
> u8 *dst, unsigned int *dlen);
> + int (*cot_decompress_noctx)(struct crypto_tfm *tfm,
> + const u8 *src, unsigned int slen,
> + u8 *dst, unsigned int *dlen);
> };
>
> #define crt_ablkcipher crt_u.ablkcipher
> @@ -1836,6 +1841,14 @@ static inline void crypto_free_comp(struct crypto_comp *tfm)
> crypto_free_tfm(crypto_comp_tfm(tfm));
> }
>
> +struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
> + u32 type, u32 mask);
> +
this should be EXPORT_SYMBOL_GPL().
-ss
On (09/18/15 14:19), Joonsoo Kim wrote:
[..]
> + /*
> + * Prepare to use crypto decompress_noctx API. One tfm is required
> + * to initialize crypto algorithm properly and fetch corresponding
> + * function pointer. But, it is sharable for multiple concurrent
> + * decompress users.
> + */
if comp algorithm require zstrm for both compression and decompression,
then there seems to be no need in allocating sharable ->tfm_noctx, we
will never use it.
-ss
> + tfm = crypto_alloc_comp_noctx(compress, 0, 0);
> + if (!IS_ERR(tfm))
> + comp->tfm_noctx = tfm;
> +
> return comp;
> }
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 4f9df8e..c76d8e4 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -26,6 +26,7 @@ struct zcomp_strm {
> /* dynamic per-device compression frontend */
> struct zcomp {
> void *stream;
> + struct crypto_comp *tfm_noctx;
> const char *backend;
>
> struct zcomp_strm *(*strm_find)(struct zcomp *comp);
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
crypto/lzo.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crypto/lzo.c b/crypto/lzo.c
index 4b3e925..994667e 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -82,7 +82,8 @@ static int lzo_decompress(struct crypto_tfm *tfm, const u8 *src,
static struct crypto_alg alg = {
.cra_name = "lzo",
- .cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
+ .cra_flags = CRYPTO_ALG_TYPE_COMPRESS |
+ CRYPTO_ALG_TFM_MAY_SHARE,
.cra_ctxsize = sizeof(struct lzo_ctx),
.cra_module = THIS_MODULE,
.cra_init = lzo_init,
--
2.6.0.rc2.10.gf4d9753
RFC
resend reason:
git (2.6.0-rc2) has crashed and spme patches were not sent out.
Sorry for the noise.
===================
This patch set implements a bit different approach to shared tfm. It
defines a new CRYPTO_ALG_TFM_MAY_SHARE ->cra_flag which each algorithm
setups in its `static struct crypto_alg'. Crypto API provides
crypto_tfm_may_share() function that returns true if the algorithm
has CRYPTO_ALG_TFM_MAY_SHARE set and, thus, we can supply a shared
`struct crypto_comp *tfm'.
Not properly tested yet.
Sergey Senozhatsky (9):
crypto: introduce CRYPTO_ALG_TFM_MAY_SHARE flag
crypto/lzo: set CRYPTO_ALG_TFM_MAY_SHARE
crypto/lz4: set CRYPTO_ALG_TFM_MAY_SHARE
crypto/lz4hc: set CRYPTO_ALG_TFM_MAY_SHARE
crypto/842: set CRYPTO_ALG_TFM_MAY_SHARE
zram: make stream find and release functions static
zram: pass zstrm down to decompression path
zram: use crypto API for compression
zram: use crypto CRYPTO_ALG_TFM_MAY_SHARE API
crypto/842.c | 3 +-
crypto/lz4.c | 3 +-
crypto/lz4hc.c | 3 +-
crypto/lzo.c | 3 +-
drivers/block/zram/Kconfig | 8 ++--
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 103 +++++++++++++++++++++++++++++++----------
drivers/block/zram/zcomp.h | 42 +++++++----------
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 | 32 +++++++++----
include/linux/crypto.h | 10 ++++
14 files changed, 138 insertions(+), 201 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.6.0.rc2.10.gf4d9753
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
crypto/lz4.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crypto/lz4.c b/crypto/lz4.c
index aefbcea..ca48de1 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -78,7 +78,8 @@ static int lz4_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
static struct crypto_alg alg_lz4 = {
.cra_name = "lz4",
- .cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
+ .cra_flags = CRYPTO_ALG_TYPE_COMPRESS |
+ CRYPTO_ALG_TFM_MAY_SHARE,
.cra_ctxsize = sizeof(struct lz4_ctx),
.cra_module = THIS_MODULE,
.cra_list = LIST_HEAD_INIT(alg_lz4.cra_list),
--
2.6.0.rc2.10.gf4d9753
Set CRYPTO_ALG_TFM_MAY_SHARE ->cra_flags when algorithm support
shared tfm.
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
include/linux/crypto.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index e71cb70..66f10f8 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -102,6 +102,11 @@
#define CRYPTO_ALG_INTERNAL 0x00002000
/*
+ * Mark that algorithm can use shared tfm
+ */
+#define CRYPTO_ALG_TFM_MAY_SHARE 0x00004000
+
+/*
* Transform masks and values (for crt_flags).
*/
#define CRYPTO_TFM_REQ_MASK 0x000fff00
@@ -648,6 +653,11 @@ static inline u32 crypto_tfm_alg_type(struct crypto_tfm *tfm)
return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK;
}
+static inline u32 crypto_tfm_may_share(struct crypto_tfm *tfm)
+{
+ return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TFM_MAY_SHARE;
+}
+
static inline unsigned int crypto_tfm_alg_blocksize(struct crypto_tfm *tfm)
{
return tfm->__crt_alg->cra_blocksize;
--
2.6.0.rc2.10.gf4d9753
Hide (make static) zstrm find and release function and introduce
zcomp_compress_begin()/zcomp_compress_end(). We will have begin
and end functions around compression (this patch) and decompression
(next patch). So the work flow is evolving to:
zstrm = foo_begin();
foo(zstrm);
foo_end(zstrm);
where foo is compress or decompress zcomp functions.
This patch is a preparation to make crypto API-powered zcomp
possible. The reasoning is that some crypto compression backends
require zstrm for decompression.
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
drivers/block/zram/zcomp.c | 27 +++++++++++++++++++++++++--
drivers/block/zram/zcomp.h | 8 ++++++--
drivers/block/zram/zram_drv.c | 6 +++---
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 5cb13ca..52d5b04 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -296,16 +296,39 @@ 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)
+static struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
{
return comp->strm_find(comp);
}
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
+static void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
{
comp->strm_release(comp, zstrm);
}
+/* Never return NULL, may sleep */
+struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp)
+{
+ return zcomp_strm_find(comp);
+}
+
+void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ zcomp_strm_release(comp, zstrm);
+}
+
+/* May return NULL, may sleep */
+struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
+{
+ return NULL;
+}
+
+void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ if (zstrm)
+ zcomp_strm_release(comp, zstrm);
+}
+
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src, size_t *dst_len)
{
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 46e2b9f..616e013 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -56,12 +56,16 @@ bool zcomp_available_algorithm(const char *comp);
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);
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
+
+struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp);
+void zcomp_compress_end(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);
+struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
+void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
+
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 c93aeb8..7dda8a5 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -673,7 +673,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
goto out;
}
- zstrm = zcomp_strm_find(zram->comp);
+ zstrm = zcomp_compress_begin(zram->comp);
user_mem = kmap_atomic(page);
if (is_partial_io(bvec)) {
@@ -744,7 +744,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
memcpy(cmem, src, clen);
}
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_compress_end(zram->comp, zstrm);
zstrm = NULL;
zs_unmap_object(meta->mem_pool, handle);
@@ -764,7 +764,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
atomic64_inc(&zram->stats.pages_stored);
out:
if (zstrm)
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_compress_end(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
--
2.6.0.rc2.10.gf4d9753
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
crypto/lz4hc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index a1d3b5b..34ef3de 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -78,7 +78,8 @@ static int lz4hc_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
static struct crypto_alg alg_lz4hc = {
.cra_name = "lz4hc",
- .cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
+ .cra_flags = CRYPTO_ALG_TYPE_COMPRESS |
+ CRYPTO_ALG_TFM_MAY_SHARE,
.cra_ctxsize = sizeof(struct lz4hc_ctx),
.cra_module = THIS_MODULE,
.cra_list = LIST_HEAD_INIT(alg_lz4hc.cra_list),
--
2.6.0.rc2.10.gf4d9753
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
crypto/842.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crypto/842.c b/crypto/842.c
index 98e387e..579be68 100644
--- a/crypto/842.c
+++ b/crypto/842.c
@@ -56,7 +56,8 @@ static struct crypto_alg alg = {
.cra_name = "842",
.cra_driver_name = "842-generic",
.cra_priority = 100,
- .cra_flags = CRYPTO_ALG_TYPE_COMPRESS,
+ .cra_flags = CRYPTO_ALG_TYPE_COMPRESS |
+ CRYPTO_ALG_TFM_MAY_SHARE,
.cra_ctxsize = sizeof(struct crypto842_ctx),
.cra_module = THIS_MODULE,
.cra_u = { .compress = {
--
2.6.0.rc2.10.gf4d9753
Introduce zcomp_decompress_begin()/zcomp_decompress_end() as a
preparation for crypto API-powered zcomp.
Change zcomp_decompress() signature to require zstrm parameter.
Unlike zcomp_compress_begin(), zcomp_decompress_begin() may return
zstrm if currently selected compression backend require zstrm for
decompression or NULL if it does not.
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
drivers/block/zram/zcomp.c | 3 ++-
drivers/block/zram/zcomp.h | 3 ++-
drivers/block/zram/zram_drv.c | 20 ++++++++++++++++----
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 52d5b04..e3016da 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -336,7 +336,8 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
zstrm->private);
}
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src,
size_t src_len, unsigned char *dst)
{
return comp->backend->decompress(src, src_len, dst);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 616e013..4c09c01 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -66,7 +66,8 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src,
size_t src_len, unsigned char *dst);
bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 7dda8a5..74cbb81 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -560,7 +560,8 @@ 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;
@@ -582,7 +583,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);
@@ -602,6 +603,7 @@ 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);
@@ -617,6 +619,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_decompress_begin(zram->comp);
user_mem = kmap_atomic(page);
if (!is_partial_io(bvec))
uncmem = user_mem;
@@ -627,7 +630,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;
@@ -636,10 +639,14 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
memcpy(user_mem + bvec->bv_offset, uncmem + offset,
bvec->bv_len);
+ zcomp_decompress_end(zram->comp, zstrm);
+ zstrm = NULL;
+
flush_dcache_page(page);
ret = 0;
out_cleanup:
kunmap_atomic(user_mem);
+ zcomp_decompress_end(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
@@ -659,6 +666,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
page = bvec->bv_page;
if (is_partial_io(bvec)) {
+ struct zcomp_strm *zstrm;
/*
* This is a partial IO. We need to read the full page
* before to write the changes.
@@ -668,7 +676,11 @@ 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_decompress_begin(zram->comp);
+ ret = zram_decompress_page(zram, zstrm, uncmem, index);
+ zcomp_decompress_end(zram->comp, zstrm);
+
if (ret)
goto out;
}
--
2.6.0.rc2.10.gf4d9753
On (09/21/15 22:13), Sergey Senozhatsky wrote:
> RFC
>
> resend reason:
> git (2.6.0-rc2) has crashed and spme patches were not sent out.
>
>
> Sorry for the noise.
>
> ===================
what a day... really sorry.
(release 2.5.3) git has crashed again
[Net::SMTP::SSL] Connection closed at /usr/lib/git-core/git-send-email line 1351.
fatal: 'send-email' appears to be a git command, but we were not
able to execute it. Maybe git-send-email is broken?
hm...
-ss
>
> This patch set implements a bit different approach to shared tfm. It
> defines a new CRYPTO_ALG_TFM_MAY_SHARE ->cra_flag which each algorithm
> setups in its `static struct crypto_alg'. Crypto API provides
> crypto_tfm_may_share() function that returns true if the algorithm
> has CRYPTO_ALG_TFM_MAY_SHARE set and, thus, we can supply a shared
> `struct crypto_comp *tfm'.
>
> Not properly tested yet.
>
>
> Sergey Senozhatsky (9):
> crypto: introduce CRYPTO_ALG_TFM_MAY_SHARE flag
> crypto/lzo: set CRYPTO_ALG_TFM_MAY_SHARE
> crypto/lz4: set CRYPTO_ALG_TFM_MAY_SHARE
> crypto/lz4hc: set CRYPTO_ALG_TFM_MAY_SHARE
> crypto/842: set CRYPTO_ALG_TFM_MAY_SHARE
> zram: make stream find and release functions static
> zram: pass zstrm down to decompression path
> zram: use crypto API for compression
> zram: use crypto CRYPTO_ALG_TFM_MAY_SHARE API
>
> crypto/842.c | 3 +-
> crypto/lz4.c | 3 +-
> crypto/lz4hc.c | 3 +-
> crypto/lzo.c | 3 +-
> drivers/block/zram/Kconfig | 8 ++--
> drivers/block/zram/Makefile | 4 +-
> drivers/block/zram/zcomp.c | 103 +++++++++++++++++++++++++++++++----------
> drivers/block/zram/zcomp.h | 42 +++++++----------
> 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 | 32 +++++++++----
> include/linux/crypto.h | 10 ++++
> 14 files changed, 138 insertions(+), 201 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.6.0.rc2.10.gf4d9753
>
Crypto subsystem now supports CRYPTO_ALG_TFM_MAY_SHARE API that
requires special tfm_noctx. This tfm can be shared by multiple
concurrent decompress user because this API doesn't rely on this
tfm object except to fetch decompress function pointer.
Until changing to use crypto API, zram doesn't require any zstrm
on decompress so decompress is parallelized unlimitedly. But, previous
patch make zram to use crypto API and this requires one zstrm on every
decompress users so, in some zstrm contended situations, zram's
performance would be degraded.
This patch makes zram use CRYPTO_ALG_TFM_MAY_SHARE API and
restore zram's performance as the time that zram doesn't use
crypto API.
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-CRYPTO_ALG_TFM_MAY_SHARE
Read 10411701.88 6426911.62 9423894.38
Re-read 10017386.62 6428218.88 11000063.50
Signed-off-by: Joonsoo Kim <[email protected]>
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
drivers/block/zram/zcomp.c | 27 +++++++++++++++++++++++++--
drivers/block/zram/zcomp.h | 1 +
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 6219d4d..b768dc9 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -319,9 +319,14 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
zcomp_strm_release(comp, zstrm);
}
-/* Never return NULL, may sleep */
+/* May return NULL, may sleep */
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
{
+ struct crypto_comp *tfm = comp->tfm_noctx;
+
+ if (tfm && crypto_tfm_may_share(crypto_comp_tfm(tfm)))
+ return NULL;
+
return zcomp_strm_find(comp);
}
@@ -345,12 +350,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
unsigned int src_len, unsigned char *dst)
{
unsigned int size = PAGE_SIZE;
+ struct crypto_comp *tfm = comp->tfm_noctx;
+
+ if (tfm && crypto_tfm_may_share(crypto_comp_tfm(tfm)))
+ return crypto_comp_decompress(tfm, src, src_len, dst, &size);
- return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
+ return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
}
void zcomp_destroy(struct zcomp *comp)
{
+ if (comp->tfm_noctx)
+ crypto_free_comp(comp->tfm_noctx);
comp->destroy(comp);
kfree(comp);
}
@@ -367,6 +378,7 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
{
struct zcomp *comp;
const char *backend;
+ struct crypto_comp *tfm;
int error;
backend = find_backend(compress);
@@ -386,5 +398,16 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
kfree(comp);
return ERR_PTR(error);
}
+
+ /*
+ * Prepare to use crypto decompress_noctx API. One tfm is required
+ * to initialize crypto algorithm properly and fetch corresponding
+ * function pointer. But, it is sharable for multiple concurrent
+ * decompress users.
+ */
+ tfm = crypto_alloc_comp(compress, 0, 0);
+ if (!IS_ERR(tfm))
+ comp->tfm_noctx = tfm;
+
return comp;
}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 4f9df8e..c76d8e4 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -26,6 +26,7 @@ struct zcomp_strm {
/* dynamic per-device compression frontend */
struct zcomp {
void *stream;
+ struct crypto_comp *tfm_noctx;
const char *backend;
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
--
2.5.3
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 decompress_noctx API and would restore the performance as is.
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/Kconfig | 8 +++----
drivers/block/zram/Makefile | 4 +---
drivers/block/zram/zcomp.c | 54 +++++++++++++++++++++++-------------------
drivers/block/zram/zcomp.h | 30 ++++++-----------------
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 | 6 ++---
9 files changed, 44 insertions(+), 186 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..7619bed 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, ...).
@@ -18,9 +17,8 @@ config ZRAM
config ZRAM_LZ4_COMPRESS
bool "Enable LZ4 algorithm support"
depends on ZRAM
- select LZ4_COMPRESS
- select LZ4_DECOMPRESS
+ select CRYPTO_LZ4
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
+ 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 e3016da..6219d4d 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,10 +15,6 @@
#include <linux/sched.h>
#include "zcomp.h"
-#include "zcomp_lzo.h"
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
-#include "zcomp_lz4.h"
-#endif
/*
* single zcomp_strm backend
@@ -43,19 +39,20 @@ struct zcomp_strm_multi {
wait_queue_head_t strm_wait;
};
-static struct zcomp_backend *backends[] = {
- &zcomp_lzo,
+static const char * const backends[] = {
+ "lzo",
#ifdef CONFIG_ZRAM_LZ4_COMPRESS
- &zcomp_lz4,
+ "lz4",
#endif
NULL
};
-static struct zcomp_backend *find_backend(const char *compress)
+static const char *find_backend(const char *compress)
{
int i = 0;
while (backends[i]) {
- if (sysfs_streq(compress, backends[i]->name))
+ if (sysfs_streq(compress, backends[i]) &&
+ crypto_has_comp(compress, 0, 0))
break;
i++;
}
@@ -64,8 +61,8 @@ static struct zcomp_backend *find_backend(const char *compress)
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);
}
@@ -80,13 +77,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->backend, 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,12 +276,12 @@ 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");
@@ -317,10 +319,10 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
zcomp_strm_release(comp, zstrm);
}
-/* May return NULL, may sleep */
+/* Never return NULL, may sleep */
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
{
- return NULL;
+ return zcomp_strm_find(comp);
}
void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
@@ -330,17 +332,21 @@ void zcomp_decompress_end(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 int *dst_len)
{
- return comp->backend->compress(src, zstrm->buffer, dst_len,
- zstrm->private);
+ *dst_len = PAGE_SIZE << 1;
+
+ return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
+ zstrm->buffer, dst_len);
}
int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src,
- size_t src_len, unsigned char *dst)
+ 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)
@@ -360,7 +366,7 @@ void zcomp_destroy(struct zcomp *comp)
struct zcomp *zcomp_create(const char *compress, int max_strm)
{
struct zcomp *comp;
- struct zcomp_backend *backend;
+ const char *backend;
int error;
backend = find_backend(compress);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 4c09c01..4f9df8e 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -11,38 +11,22 @@
#define _ZCOMP_H_
#include <linux/mutex.h>
+#include <linux/crypto.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 *backend;
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
@@ -61,14 +45,14 @@ struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp);
void zcomp_compress_end(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 int *dst_len);
struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src,
- size_t src_len, unsigned char *dst);
+ 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 74cbb81..cb478ca 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -567,7 +567,7 @@ static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
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;
@@ -656,7 +656,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;
@@ -731,7 +731,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_err("Error allocating memory for compressed page: %u, size=%zu\n",
+ pr_err("Error allocating memory for compressed page: %u, size=%u\n",
index, clen);
ret = -ENOMEM;
goto out;
--
2.5.3
On Fri, Sep 18, 2015 at 02:19:16PM +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
> decompression API that calls decompression function via sharable tfm
> object. Concurrent calls to decompress_noctx function through sharable
> tfm object will be okay because caller don't need any context in tfm and
> tfm is only used for fetching function pointer to decompress_noctx
> function. This can reduce overhead of maintaining multiple tfm
> if decompression doesn't require any context to operate.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
Have you looked into include/crypto/compress.h?
The compress type itself is obsolete and should be replaced with
the pcomp interface.
However, we made some mistakes with the pcomp interface. First
of all it doesn't have a non-partial interface like the digest
function for crypto_shash.
But the biggest problem is that it should be completely stateless
but is not. IOW we should not store anything in the tfm that
is per-request. That should all go into the request structure.
Fortunately it seems that pcomp doesn't actually have any users
so we can just rip it out and start from scratch and redo it
properly.
So to recap, please abandon any efforts on the crypto_compress
interface as it is old and obsolete. Instead reshape crypto_pcomp
into a proper stateless interface which should then give you what
you want.
When you do so, just keep in mind that we need to find a way to
support IPComp. That means the ability to allocate requests in
thread context and then use them to compress/decompress in IRQ
context.
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 Mon, Sep 21, 2015 at 03:18:17PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > static int __init lzo_mod_init(void)
> > diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> > index e71cb70..31152b1 100644
> > --- a/include/linux/crypto.h
> > +++ b/include/linux/crypto.h
> > @@ -355,6 +355,8 @@ 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_decompress_noctx)(const u8 *src, unsigned int slen,
> > + u8 *dst, unsigned int *dlen);
> > };
> >
> >
> > @@ -538,6 +540,9 @@ struct compress_tfm {
> > int (*cot_decompress)(struct crypto_tfm *tfm,
> > const u8 *src, unsigned int slen,
> > u8 *dst, unsigned int *dlen);
> > + int (*cot_decompress_noctx)(struct crypto_tfm *tfm,
> > + const u8 *src, unsigned int slen,
> > + u8 *dst, unsigned int *dlen);
> > };
> >
> > #define crt_ablkcipher crt_u.ablkcipher
> > @@ -1836,6 +1841,14 @@ static inline void crypto_free_comp(struct crypto_comp *tfm)
> > crypto_free_tfm(crypto_comp_tfm(tfm));
> > }
> >
> > +struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
> > + u32 type, u32 mask);
> > +
>
> this should be EXPORT_SYMBOL_GPL().
>
Will do in next version.
Thanks.
On Tue, Sep 22, 2015 at 08:43:46PM +0800, Herbert Xu wrote:
> On Fri, Sep 18, 2015 at 02:19:16PM +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
> > decompression API that calls decompression function via sharable tfm
> > object. Concurrent calls to decompress_noctx function through sharable
> > tfm object will be okay because caller don't need any context in tfm and
> > tfm is only used for fetching function pointer to decompress_noctx
> > function. This can reduce overhead of maintaining multiple tfm
> > if decompression doesn't require any context to operate.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
>
> Have you looked into include/crypto/compress.h?
Okay. Now, I have looked it.
>
> The compress type itself is obsolete and should be replaced with
> the pcomp interface.
>
> However, we made some mistakes with the pcomp interface. First
> of all it doesn't have a non-partial interface like the digest
> function for crypto_shash.
>
> But the biggest problem is that it should be completely stateless
> but is not. IOW we should not store anything in the tfm that
> is per-request. That should all go into the request structure.
I'm not a expert on this area but I have different opinion.
IIUC, what partial compression aims at is to support partial data
compression. It isn't for stateless compression and they are
different. Current implementation would work well for it's own
purpose.
And, making it stateless looks not good to me. If we make it
stateless, we should include algorithm's state in request object.
It enforces much memory to request object and, unlike crypto_shash
which can be allocated in stack, user needs to manage life time of
request objects and it requires additional complexity to
compression user.
Moreover, to use sharable tfm concurrently from my original intend,
many request objects would be needed and if it cannot be allocated
in the stack, it requires much memory. It's not suitable
for my purpose.
If compression is obsolete, what I think the best is leaving pcomp
as is and implementing sharable tfm in pcomp, And then, make lzo
and others support pcomp interface and sharable tfm.
Maybe, I'm wrong so please correct me.
Thanks.
> Fortunately it seems that pcomp doesn't actually have any users
> so we can just rip it out and start from scratch and redo it
> properly.
>
> So to recap, please abandon any efforts on the crypto_compress
> interface as it is old and obsolete. Instead reshape crypto_pcomp
> into a proper stateless interface which should then give you what
> you want.
>
> When you do so, just keep in mind that we need to find a way to
> support IPComp. That means the ability to allocate requests in
> thread context and then use them to compress/decompress in IRQ
> context.
>
> Cheers,
> --
> Email: Herbert Xu <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Mon, Sep 21, 2015 at 02:38:59PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > @@ -61,7 +61,8 @@ 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_decompress_noctx = NULL } }
> > };
> >
> > static int __init crypto842_mod_init(void)
> > diff --git a/crypto/compress.c b/crypto/compress.c
> > index c33f076..abb36a8 100644
> > --- a/crypto/compress.c
> > +++ b/crypto/compress.c
> > @@ -33,12 +33,21 @@ static int crypto_decompress(struct crypto_tfm *tfm,
> > dlen);
> > }
> >
> > +static int crypto_decompress_noctx(struct crypto_tfm *tfm,
> > + const u8 *src, unsigned int slen,
> > + u8 *dst, unsigned int *dlen)
> > +{
> > + return tfm->__crt_alg->cra_compress.coa_decompress_noctx(src, slen,
> > + dst, dlen);
> > +}
>
>
> hm... well... sorry, if irrelevant.
> if the algorithm can have a _noctx() decompression function, does it
> automatically guarantee that this algorithm never dereferences a passed
> `struct crypto_tfm *tfm' pointer in its decompress function? in other words,
> can we simply pass that `shared tfm pointer' to the existing decompress
> function instead of defining new symbols, new callbacks, etc.?
>
> cot_decompress_noctx() == cot_decompress(shared_ftm) ?
>
> just a thought.
Will think it if I implement next version in this way. Due to Herbert
comment, interface will be changed so much in next version.
>
> [..]
> > +struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
> > + u32 type, u32 mask)
> > +{
> > + struct crypto_comp *comp;
> > + struct crypto_tfm *tfm;
> > + struct compress_tfm *ops;
> > +
> > + comp = crypto_alloc_comp(alg_name, type, mask);
> > + if (IS_ERR(comp))
> > + return comp;
> > +
> > + tfm = crypto_comp_tfm(comp);
> > + if (!tfm->__crt_alg->cra_compress.coa_decompress_noctx) {
> > + crypto_free_comp(comp);
> > + return ERR_PTR(-EINVAL);
>
> -ENOMEM?
No, it's failure isn't related to NOMEM. Just not support it.
Thanks.
On Mon, Sep 21, 2015 at 12:58:12PM +0900, Minchan Kim wrote:
> On Fri, Sep 18, 2015 at 02:19:15PM +0900, Joonsoo Kim wrote:
> > 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.
>
> Fair enough. lzo and lz4 cannot cover all of workloads so surely, there are
> workloads other compressor algorithm can win. As well, we could support
> H/W compressor with crypto support so I think crypto is a way to go.
Hello, Minchan.
Good!
>
> One thing I have a concern is I don't want to bind some of compressor
> algorithm to zram statically. User can see what kinds of crypto compressor
> support in his system via /proc/crypto and use it dynamically.
> I hope zram supports it.
Okay. I will handle it next version.
> >
> > 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
>
> Nice catch from Sergey!
>
> > 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
>
> Another patch and number we need to merge this patchset is zlib support
> patchset and number you had experiement.
Okay. Will include.
Thanks.
> >
> > Thanks.
> >
> > Joonsoo Kim (7):
> > crypto: introduce decompression API that can be called via sharable
> > tfm object
> > crypto/lzo: support decompress_noctx
> > crypyo/lz4: support decompress_noctx
> > crypto/lz4hc: support decompress_noctx
> > crypto/842: support decompress_noctx
> > zram: use crypto API for compression
> > zram: use crypto decompress_noctx API
> >
> > Sergey Senozhatsky (2):
> > zram: make stream find and release functions static
> > zram: pass zstrm down to decompression path
> >
> > crypto/842.c | 9 +++-
> > crypto/compress.c | 36 +++++++++++++++
> > crypto/crypto_null.c | 3 +-
> > crypto/deflate.c | 3 +-
> > crypto/lz4.c | 9 +++-
> > crypto/lz4hc.c | 9 +++-
> > crypto/lzo.c | 9 +++-
> > drivers/block/zram/Kconfig | 8 ++--
> > drivers/block/zram/Makefile | 4 +-
> > drivers/block/zram/zcomp.c | 102 +++++++++++++++++++++++++++++++----------
> > drivers/block/zram/zcomp.h | 42 +++++++----------
> > 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 | 32 +++++++++----
> > include/linux/crypto.h | 20 ++++++++
> > 17 files changed, 211 insertions(+), 203 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
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Mon, Sep 21, 2015 at 02:19:03PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > -static struct zcomp_backend *find_backend(const char *compress)
> > +static const char *find_backend(const char *compress)
> > {
> > int i = 0;
> > while (backends[i]) {
> > - if (sysfs_streq(compress, backends[i]->name))
> > + if (sysfs_streq(compress, backends[i]) &&
> > + crypto_has_comp(compress, 0, 0))
>
> ok, just for note. zcomp does sysfs_streq(), because sysfs data
> usually contain a trailing new line, crypto_has_comp() does strcmp().
Okay. Will check.
>
>
> > int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > - const unsigned char *src, size_t *dst_len)
> > + const unsigned char *src, unsigned int *dst_len)
> > {
> > - return comp->backend->compress(src, zstrm->buffer, dst_len,
> > - zstrm->private);
> > + *dst_len = PAGE_SIZE << 1;
> > +
>
> hm... wouldn't it be better to say crypto api that we have a PAGE_SIZE
> buffer instead of PAGE_SIZE << 1, so in case of buffer overrun (or
> whatever is the correct term here) it will stop compression earlier
> (well, possibly)? zram will drop compressed data larger than PAGE_SIZE
> anyway, so if passing a smaller buffer size can save us CPU time then
> let's do it.
It can be implemented and maybe good way to go. But, in this patchset,
it isn't needed. It is better to do in separate patch.
>
> > + return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
> > + zstrm->buffer, dst_len);
> > }
> >
> > int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > const unsigned char *src,
> > - size_t src_len, unsigned char *dst)
> > + 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);
>
> ^^^^^^^^ tab?
>
> > }
> >
> > void zcomp_destroy(struct zcomp *comp)
> > @@ -359,7 +365,7 @@ void zcomp_destroy(struct zcomp *comp)
> > struct zcomp *zcomp_create(const char *compress, int max_strm)
> > {
> > struct zcomp *comp;
> > - struct zcomp_backend *backend;
> > + const char *backend;
>
> rebase against the current linux-next. this and the next patch do not
> apply cleanly. the function was touched recently: '+int error'.
Okay.
>
> >
> > backend = find_backend(compress);
> > if (!backend)
> > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> > index 4c09c01..4f9df8e 100644
> > --- a/drivers/block/zram/zcomp.h
> > +++ b/drivers/block/zram/zcomp.h
> > @@ -11,38 +11,22 @@
> > #define _ZCOMP_H_
> >
> > #include <linux/mutex.h>
> > +#include <linux/crypto.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 *backend;
>
> ^^ what for?
Will remove.
Thanks.
On Mon, Sep 21, 2015 at 12:45:12PM +0900, Minchan Kim wrote:
> Hi Joonsoo,
>
> On Fri, Sep 18, 2015 at 02:19:23PM +0900, Joonsoo Kim wrote:
> > 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 decompress_noctx API and would restore the performance as is.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
> > ---
> > drivers/block/zram/Kconfig | 8 +++----
> > drivers/block/zram/Makefile | 4 +---
> > drivers/block/zram/zcomp.c | 54 +++++++++++++++++++++++-------------------
> > drivers/block/zram/zcomp.h | 30 ++++++-----------------
> > 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 | 6 ++---
> > 9 files changed, 44 insertions(+), 186 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..7619bed 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, ...).
> > @@ -18,9 +17,8 @@ config ZRAM
> > config ZRAM_LZ4_COMPRESS
> > bool "Enable LZ4 algorithm support"
> > depends on ZRAM
> > - select LZ4_COMPRESS
> > - select LZ4_DECOMPRESS
> > + select CRYPTO_LZ4
>
> It is out of my expectation.
>
> When I heard crypto support for zRAM first, I imagined zram user can
> use any crypto compressor algorithm easily if system has the algorithm.
> IOW, I expect zRAM shouldn't bind CONFIG_CRYPTO_ALGONAME statically
> except the default one(ie, CONFIG_CRYPTO_LZO) like zswap and It seems
> you did in eariler version.
>
> You seem to have a trouble to adapt crypto to current comp_algorithm
> because crypto doesn't export any API to enumerate algorithm name
> while zram should export supporting algorithm name via comp_algorithm
> and I heard crypto maintainer doesn't want to export it. Instead,
> user can use /proc/crypto to know what kinds of compressor system
> can support.
>
> Hmm,
> At the first glance, I was about to say "let's sort it out with
> futher patches" but I changed my mind. ;-).
>
> We should sort it out before you are adding zlib support(ie, please
> include zlib support patch with number data in this patchset). Otherwise,
> we should add new hard-wired stuff for zlib like lzo, lz4 to
> comp_algorithm and will depreate soon.
>
> My idea is ABI change of comp_algorithm. Namely, let's deprecate it
> and guide to use /proc/crypto to show available compressor.
> Someday, we removes backends string array finally.
Okay! That's also what I want so I will follow your comment.
Thanks.
>
> Welcome to any ideas.
>
> > 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
> > + 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 3456d5a..c2ed7c8 100644
> > --- a/drivers/block/zram/zcomp.c
> > +++ b/drivers/block/zram/zcomp.c
> > @@ -15,10 +15,6 @@
> > #include <linux/sched.h>
> >
> > #include "zcomp.h"
> > -#include "zcomp_lzo.h"
> > -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> > -#include "zcomp_lz4.h"
> > -#endif
> >
> > /*
> > * single zcomp_strm backend
> > @@ -43,19 +39,20 @@ struct zcomp_strm_multi {
> > wait_queue_head_t strm_wait;
> > };
> >
> > -static struct zcomp_backend *backends[] = {
> > - &zcomp_lzo,
> > +static const char * const backends[] = {
> > + "lzo",
> > #ifdef CONFIG_ZRAM_LZ4_COMPRESS
> > - &zcomp_lz4,
> > + "lz4",
> > #endif
> > NULL
> > };
> >
> > -static struct zcomp_backend *find_backend(const char *compress)
> > +static const char *find_backend(const char *compress)
> > {
> > int i = 0;
> > while (backends[i]) {
> > - if (sysfs_streq(compress, backends[i]->name))
> > + if (sysfs_streq(compress, backends[i]) &&
> > + crypto_has_comp(compress, 0, 0))
> > break;
> > i++;
> > }
> > @@ -64,8 +61,8 @@ static struct zcomp_backend *find_backend(const char *compress)
> >
> > 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);
> > }
> > @@ -80,13 +77,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->backend, 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,12 +276,12 @@ 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");
> > @@ -317,10 +319,10 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
> > zcomp_strm_release(comp, zstrm);
> > }
> >
> > -/* May return NULL, may sleep */
> > +/* Never return NULL, may sleep */
> > struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> > {
> > - return NULL;
> > + return zcomp_strm_find(comp);
> > }
> >
> > void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
> > @@ -330,17 +332,21 @@ void zcomp_decompress_end(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 int *dst_len)
> > {
> > - return comp->backend->compress(src, zstrm->buffer, dst_len,
> > - zstrm->private);
> > + *dst_len = PAGE_SIZE << 1;
> > +
> > + return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
> > + zstrm->buffer, dst_len);
> > }
> >
> > int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > const unsigned char *src,
> > - size_t src_len, unsigned char *dst)
> > + 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)
> > @@ -359,7 +365,7 @@ void zcomp_destroy(struct zcomp *comp)
> > struct zcomp *zcomp_create(const char *compress, int max_strm)
> > {
> > struct zcomp *comp;
> > - struct zcomp_backend *backend;
> > + const char *backend;
> >
> > backend = find_backend(compress);
> > if (!backend)
> > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> > index 4c09c01..4f9df8e 100644
> > --- a/drivers/block/zram/zcomp.h
> > +++ b/drivers/block/zram/zcomp.h
> > @@ -11,38 +11,22 @@
> > #define _ZCOMP_H_
> >
> > #include <linux/mutex.h>
> > +#include <linux/crypto.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 *backend;
> >
> > struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> > void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> > @@ -61,14 +45,14 @@ struct zcomp_strm *zcomp_compress_begin(struct zcomp *comp);
> > void zcomp_compress_end(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 int *dst_len);
> >
> > struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp);
> > void zcomp_decompress_end(struct zcomp *comp, struct zcomp_strm *zstrm);
> >
> > int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > const unsigned char *src,
> > - size_t src_len, unsigned char *dst);
> > + 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 55e09db1..834f452 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -567,7 +567,7 @@ static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
> > 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;
> > @@ -656,7 +656,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;
> > @@ -731,7 +731,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_err("Error allocating memory for compressed page: %u, size=%zu\n",
> > + pr_err("Error allocating memory for compressed page: %u, size=%u\n",
> > index, clen);
> > ret = -ENOMEM;
> > goto out;
> > --
> > 1.9.1
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Mon, Sep 21, 2015 at 12:51:28PM +0900, Minchan Kim wrote:
> On Fri, Sep 18, 2015 at 02:19:24PM +0900, Joonsoo Kim wrote:
> > Crypto subsystem now supports decompress_noctx API that requires
> > special tfm_noctx. This tfm can be shared by multiple concurrent
> > decompress user because this API doesn't rely on this tfm object
> > except to fetch decompress function pointer.
> >
> > Until changing to use crypto API, zram doesn't require any zstrm
> > on decompress so decompress is parallelized unlimitedly. But, previous
> > patch make zram to use crypto API and this requires one zstrm on every
> > decompress users so, in some zstrm contended situations, zram's
> > performance would be degraded.
> >
> > This patch makes zram use decompress_noctx API and restore zram's
> > performance as the time that zram doesn't use crypto API.
> >
> > 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 | 24 +++++++++++++++++++++++-
> > drivers/block/zram/zcomp.h | 1 +
> > 2 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> > index c2ed7c8..a020200 100644
> > --- a/drivers/block/zram/zcomp.c
> > +++ b/drivers/block/zram/zcomp.c
> > @@ -319,9 +319,12 @@ void zcomp_compress_end(struct zcomp *comp, struct zcomp_strm *zstrm)
> > zcomp_strm_release(comp, zstrm);
> > }
> >
> > -/* Never return NULL, may sleep */
> > +/* May return NULL, may sleep */
> > struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> > {
> > + if (comp->tfm_noctx)
> > + return NULL;
>
> Hmm,, I understand why returns NULL but it seems to be awkward to use NULL
> to express meaningful semantic and following functions relies on.
> If I have a time, I will think over.
I also think that it's not good thing but I can't think any better
idea. Please let me know if you have better one.
>
> > +
> > return zcomp_strm_find(comp);
> > }
> >
> > @@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > {
> > unsigned int size = PAGE_SIZE;
> >
> > + if (!zstrm) {
> > + return crypto_comp_decompress_noctx(comp->tfm_noctx,
> > + src, src_len, dst, &size);
> > + }
> > +
> > return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size);
> > }
> >
> > void zcomp_destroy(struct zcomp *comp)
> > {
> > + if (comp->tfm_noctx)
> > + crypto_free_comp_noctx(comp->tfm_noctx);
> > comp->destroy(comp);
> > kfree(comp);
> > }
> > @@ -366,6 +376,7 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> > {
> > struct zcomp *comp;
> > const char *backend;
> > + struct crypto_comp *tfm;
> >
> > backend = find_backend(compress);
> > if (!backend)
> > @@ -384,5 +395,16 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
> > kfree(comp);
> > return ERR_PTR(-ENOMEM);
> > }
> > +
> > + /*
> > + * Prepare to use crypto decompress_noctx API. One tfm is required
> > + * to initialize crypto algorithm properly and fetch corresponding
> > + * function pointer. But, it is sharable for multiple concurrent
> > + * decompress users.
> > + */
>
> Please comment out that this API will return NULL if the compressor doesn't
> support noctx mode.
Will do.
Thanks.
On Mon, Sep 21, 2015 at 04:56:00PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > + /*
> > + * Prepare to use crypto decompress_noctx API. One tfm is required
> > + * to initialize crypto algorithm properly and fetch corresponding
> > + * function pointer. But, it is sharable for multiple concurrent
> > + * decompress users.
> > + */
>
> if comp algorithm require zstrm for both compression and decompression,
> then there seems to be no need in allocating sharable ->tfm_noctx, we
> will never use it.
>
Yes, in that case, NULL will be returned. I should describe it on
somewhere.
Thanks.
On Mon, Sep 21, 2015 at 02:29:18PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> > -/* Never return NULL, may sleep */
> > +/* May return NULL, may sleep */
> > struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> > {
> > + if (comp->tfm_noctx)
> > + return NULL;
> > +
> > return zcomp_strm_find(comp);
> > }
> >
> > @@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > {
> > unsigned int size = PAGE_SIZE;
> >
> > + if (!zstrm) {
> > + return crypto_comp_decompress_noctx(comp->tfm_noctx,
> > + src, src_len, dst, &size);
> > + }
>
> unneeded braces.
It's more readable for me. But, if you don't like it, I will remove brace.
Thanks.
On (09/25/15 14:43), Joonsoo Kim wrote:
[..]
> > > /* dynamic per-device compression frontend */
> > > struct zcomp {
> > > void *stream;
> > > - struct zcomp_backend *backend;
> > > + const char *backend;
> >
> > ^^ what for?
>
> Will remove.
I think that was my mistake, I realized why do you keep it
later -- to allocate new zstreams. let's keep it.
-ss
On (09/25/15 14:46), Joonsoo Kim wrote:
[..]
> > > struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> > > {
> > > + if (comp->tfm_noctx)
> > > + return NULL;
> >
> > Hmm,, I understand why returns NULL but it seems to be awkward to use NULL
> > to express meaningful semantic and following functions relies on.
> > If I have a time, I will think over.
>
> I also think that it's not good thing but I can't think any better
> idea. Please let me know if you have better one.
yeah, me neither.
-ss
On (09/25/15 14:26), Joonsoo Kim wrote:
[..]
> > > +struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
> > > + u32 type, u32 mask);
> > > +
> >
> > this should be EXPORT_SYMBOL_GPL().
> >
>
> Will do in next version.
>
so you want to go with _noctx() callbacks implementation?
that CRYPTO_ALG_TFM_MAY_SHARE flag looks quite simple to
me. or you guys hate it?
-ss
On Fri, Sep 25, 2015 at 04:56:10PM +0900, Sergey Senozhatsky wrote:
>
> so you want to go with _noctx() callbacks implementation?
> that CRYPTO_ALG_TFM_MAY_SHARE flag looks quite simple to
> me. or you guys hate it?
I think we should just replace crypto_pcomp with a new interface
that does what you guys want. The current crypto_compress interface
is simply broken because it stores per-request state in the tfm.
This runs counter to every other crypto type, e.g., hash or aead.
The tfm should only hold shared data, e.g., compression algorithm
parameters but not per-request state.
As the original pcomp author has disappeared I think you could
even drop the partial stuff and just do a straight compression
interface.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt