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 we don't do that, we cannot
utilize various compression algorithms in the system. To improve this
situation, zram will be changed to use crypto subsystem in following
patch. There is one problem with this change. Zram has a interface
that what compression algorithm it can support. Although crypto subsystem
has /proc interface to search all of crypto algorithm, but, there is
no way to get just compression algorithm in cryto subsystem. To implement
it on zram-side, crypto_alg_list and rwsem should be exported so
this patch do it.
Signed-off-by: Joonsoo Kim <[email protected]>
---
crypto/internal.h | 2 --
include/linux/crypto.h | 4 ++++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/crypto/internal.h b/crypto/internal.h
index 00e42a3..806f17a 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -45,8 +45,6 @@ struct crypto_larval {
u32 mask;
};
-extern struct list_head crypto_alg_list;
-extern struct rw_semaphore crypto_alg_sem;
extern struct blocking_notifier_head crypto_chain;
#ifdef CONFIG_PROC_FS
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 81ef938..ab39f4b 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -24,6 +24,10 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/uaccess.h>
+#include <linux/rwsem.h>
+
+extern struct list_head crypto_alg_list;
+extern struct rw_semaphore crypto_alg_sem;
/*
* Autoloaded crypto modules should only use a prefixed name to avoid allowing
--
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 to compress the page 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.
Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/block/zram/Kconfig | 13 +------
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 87 ++++++++++++++++++------------------------
drivers/block/zram/zcomp.h | 34 +++++------------
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 | 28 +++++++++-----
9 files changed, 66 insertions(+), 228 deletions(-)
delete mode 100644 drivers/block/zram/zcomp_lz4.c
delete mode 100644 drivers/block/zram/zcomp_lz4.h
delete mode 100644 drivers/block/zram/zcomp_lzo.c
delete mode 100644 drivers/block/zram/zcomp_lzo.h
diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 386ba3d..36ec96f 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -1,8 +1,7 @@
config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC
- select LZO_COMPRESS
- select LZO_DECOMPRESS
+ select CRYPTO_LZO
default n
help
Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
@@ -14,13 +13,3 @@ config ZRAM
disks and maybe many more.
See zram.txt for more information.
-
-config ZRAM_LZ4_COMPRESS
- bool "Enable LZ4 algorithm support"
- depends on ZRAM
- select LZ4_COMPRESS
- select LZ4_DECOMPRESS
- default n
- help
- This option enables LZ4 compression algorithm support. Compression
- algorithm can be changed using `comp_algorithm' device attribute.
\ No newline at end of file
diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
index be0763f..9e2b79e 100644
--- a/drivers/block/zram/Makefile
+++ b/drivers/block/zram/Makefile
@@ -1,5 +1,3 @@
-zram-y := zcomp_lzo.o zcomp.o zram_drv.o
-
-zram-$(CONFIG_ZRAM_LZ4_COMPRESS) += zcomp_lz4.o
+zram-y := zcomp.o zram_drv.o
obj-$(CONFIG_ZRAM) += zram.o
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 965d1af..3dc9bfa 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -13,12 +13,9 @@
#include <linux/slab.h>
#include <linux/wait.h>
#include <linux/sched.h>
+#include <linux/crypto.h>
#include "zcomp.h"
-#include "zcomp_lzo.h"
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
-#include "zcomp_lz4.h"
-#endif
/*
* single zcomp_strm backend
@@ -43,36 +40,17 @@ struct zcomp_strm_multi {
wait_queue_head_t strm_wait;
};
-static struct zcomp_backend *backends[] = {
- &zcomp_lzo,
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
- &zcomp_lz4,
-#endif
- NULL
-};
-
-static struct zcomp_backend *find_backend(const char *compress)
-{
- int i = 0;
- while (backends[i]) {
- if (sysfs_streq(compress, backends[i]->name))
- break;
- i++;
- }
- return backends[i];
-}
-
static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
{
- if (zstrm->private)
- comp->backend->destroy(zstrm->private);
+ if (zstrm->tfm)
+ crypto_free_comp(zstrm->tfm);
free_pages((unsigned long)zstrm->buffer, 1);
kfree(zstrm);
}
/*
- * allocate new zcomp_strm structure with ->private initialized by
- * backend, return NULL on error
+ * allocate new zcomp_strm structure with initializing crypto data structure,
+ * return NULL on error
*/
static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
{
@@ -80,13 +58,18 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
if (!zstrm)
return NULL;
- zstrm->private = comp->backend->create();
+ zstrm->tfm = crypto_alloc_comp(comp->name, 0, 0);
+ if (IS_ERR(zstrm->tfm)) {
+ kfree(zstrm);
+ return NULL;
+ }
+
/*
* allocate 2 pages. 1 for compressed data, plus 1 extra for the
* case when compressed size is larger than the original one
*/
zstrm->buffer = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1);
- if (!zstrm->private || !zstrm->buffer) {
+ if (!zstrm->buffer) {
zcomp_strm_free(comp, zstrm);
zstrm = NULL;
}
@@ -270,25 +253,26 @@ static int zcomp_strm_single_create(struct zcomp *comp)
/* show available compressors */
ssize_t zcomp_available_show(const char *comp, char *buf)
{
+ struct crypto_alg *alg;
ssize_t sz = 0;
- int i = 0;
- while (backends[i]) {
- if (!strcmp(comp, backends[i]->name))
+ down_read(&crypto_alg_sem);
+ list_for_each_entry(alg, &crypto_alg_list, cra_list) {
+ if ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK)
+ != CRYPTO_ALG_TYPE_COMPRESS)
+ continue;
+
+ if (sysfs_streq(comp, alg->cra_name))
sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
- "[%s] ", backends[i]->name);
+ "[%s] ", alg->cra_name);
else
sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
- "%s ", backends[i]->name);
- i++;
+ "%s ", alg->cra_name);
}
sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
- return sz;
-}
+ up_read(&crypto_alg_sem);
-bool zcomp_available_algorithm(const char *comp)
-{
- return find_backend(comp) != NULL;
+ return sz;
}
bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
@@ -307,16 +291,21 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
}
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, 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, const unsigned char *src,
- size_t src_len, unsigned char *dst)
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src, unsigned int src_len,
+ unsigned char *dst)
{
- return comp->backend->decompress(src, src_len, dst);
+ unsigned int size = PAGE_SIZE;
+
+ return crypto_comp_decompress(zstrm->tfm, src, src_len,
+ dst, &size);
}
void zcomp_destroy(struct zcomp *comp)
@@ -335,17 +324,15 @@ void zcomp_destroy(struct zcomp *comp)
struct zcomp *zcomp_create(const char *compress, int max_strm)
{
struct zcomp *comp;
- struct zcomp_backend *backend;
- backend = find_backend(compress);
- if (!backend)
+ if (!crypto_has_comp(compress, 0, 0))
return ERR_PTR(-EINVAL);
comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
if (!comp)
return ERR_PTR(-ENOMEM);
- comp->backend = backend;
+ comp->name = compress;
if (max_strm > 1)
zcomp_strm_multi_create(comp, max_strm);
else
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 46e2b9f..35f7f10 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -10,39 +10,23 @@
#ifndef _ZCOMP_H_
#define _ZCOMP_H_
+#include <linux/crypto.h>
#include <linux/mutex.h>
struct zcomp_strm {
+ struct crypto_comp *tfm;
+
/* compression/decompression buffer */
void *buffer;
- /*
- * The private data of the compression stream, only compression
- * stream backend can touch this (e.g. compression algorithm
- * working memory)
- */
- void *private;
+
/* used in multi stream backend, protected by backend strm_lock */
struct list_head list;
};
-/* static compression backend */
-struct zcomp_backend {
- int (*compress)(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private);
-
- int (*decompress)(const unsigned char *src, size_t src_len,
- unsigned char *dst);
-
- void *(*create)(void);
- void (*destroy)(void *private);
-
- const char *name;
-};
-
/* dynamic per-device compression frontend */
struct zcomp {
void *stream;
- struct zcomp_backend *backend;
+ const char *name;
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
@@ -51,7 +35,6 @@ struct zcomp {
};
ssize_t zcomp_available_show(const char *comp, char *buf);
-bool zcomp_available_algorithm(const char *comp);
struct zcomp *zcomp_create(const char *comp, int max_strm);
void zcomp_destroy(struct zcomp *comp);
@@ -60,10 +43,11 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, size_t *dst_len);
+ const unsigned char *src, unsigned int *dst_len);
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
- size_t src_len, unsigned char *dst);
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src, unsigned int src_len,
+ unsigned char *dst);
bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
#endif /* _ZCOMP_H_ */
diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
deleted file mode 100644
index f2afb7e..0000000
--- a/drivers/block/zram/zcomp_lz4.c
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/lz4.h>
-
-#include "zcomp_lz4.h"
-
-static void *zcomp_lz4_create(void)
-{
- return kzalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
-}
-
-static void zcomp_lz4_destroy(void *private)
-{
- kfree(private);
-}
-
-static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private)
-{
- /* return : Success if return 0 */
- return lz4_compress(src, PAGE_SIZE, dst, dst_len, private);
-}
-
-static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
-{
- size_t dst_len = PAGE_SIZE;
- /* return : Success if return 0 */
- return lz4_decompress_unknownoutputsize(src, src_len, dst, &dst_len);
-}
-
-struct zcomp_backend zcomp_lz4 = {
- .compress = zcomp_lz4_compress,
- .decompress = zcomp_lz4_decompress,
- .create = zcomp_lz4_create,
- .destroy = zcomp_lz4_destroy,
- .name = "lz4",
-};
diff --git a/drivers/block/zram/zcomp_lz4.h b/drivers/block/zram/zcomp_lz4.h
deleted file mode 100644
index 60613fb..0000000
--- a/drivers/block/zram/zcomp_lz4.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _ZCOMP_LZ4_H_
-#define _ZCOMP_LZ4_H_
-
-#include "zcomp.h"
-
-extern struct zcomp_backend zcomp_lz4;
-
-#endif /* _ZCOMP_LZ4_H_ */
diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
deleted file mode 100644
index da1bc47..0000000
--- a/drivers/block/zram/zcomp_lzo.c
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/lzo.h>
-
-#include "zcomp_lzo.h"
-
-static void *lzo_create(void)
-{
- return kzalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
-}
-
-static void lzo_destroy(void *private)
-{
- kfree(private);
-}
-
-static int lzo_compress(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private)
-{
- int ret = lzo1x_1_compress(src, PAGE_SIZE, dst, dst_len, private);
- return ret == LZO_E_OK ? 0 : ret;
-}
-
-static int lzo_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
-{
- size_t dst_len = PAGE_SIZE;
- int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len);
- return ret == LZO_E_OK ? 0 : ret;
-}
-
-struct zcomp_backend zcomp_lzo = {
- .compress = lzo_compress,
- .decompress = lzo_decompress,
- .create = lzo_create,
- .destroy = lzo_destroy,
- .name = "lzo",
-};
diff --git a/drivers/block/zram/zcomp_lzo.h b/drivers/block/zram/zcomp_lzo.h
deleted file mode 100644
index 128c580..0000000
--- a/drivers/block/zram/zcomp_lzo.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _ZCOMP_LZO_H_
-#define _ZCOMP_LZO_H_
-
-#include "zcomp.h"
-
-extern struct zcomp_backend zcomp_lzo;
-
-#endif /* _ZCOMP_LZO_H_ */
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index b088ca9..84704d8 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -30,6 +30,7 @@
#include <linux/err.h>
#include <linux/idr.h>
#include <linux/sysfs.h>
+#include <linux/crypto.h>
#include "zram_drv.h"
@@ -349,7 +350,7 @@ out:
static ssize_t comp_algorithm_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- size_t sz;
+ size_t sz = 0;
struct zram *zram = dev_to_zram(dev);
down_read(&zram->init_lock);
@@ -378,7 +379,7 @@ static ssize_t comp_algorithm_store(struct device *dev,
if (sz > 0 && zram->compressor[sz - 1] == '\n')
zram->compressor[sz - 1] = 0x00;
- if (!zcomp_available_algorithm(zram->compressor))
+ if (!crypto_has_comp(zram->compressor, 0, 0))
len = -EINVAL;
up_write(&zram->init_lock);
@@ -562,13 +563,14 @@ static void zram_free_page(struct zram *zram, size_t index)
zram_set_obj_size(meta, index, 0);
}
-static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
+static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
+ char *mem, u32 index)
{
int ret = 0;
unsigned char *cmem;
struct zram_meta *meta = zram->meta;
unsigned long handle;
- size_t size;
+ unsigned int size;
bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
handle = meta->table[index].handle;
@@ -584,7 +586,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
if (size == PAGE_SIZE)
copy_page(mem, cmem);
else
- ret = zcomp_decompress(zram->comp, cmem, size, mem);
+ ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
zs_unmap_object(meta->mem_pool, handle);
bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
@@ -604,6 +606,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
struct page *page;
unsigned char *user_mem, *uncmem = NULL;
struct zram_meta *meta = zram->meta;
+ struct zcomp_strm *zstrm = NULL;
+
page = bvec->bv_page;
bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
@@ -619,6 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
/* Use a temporary buffer to decompress the page */
uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
+ zstrm = zcomp_strm_find(zram->comp);
user_mem = kmap_atomic(page);
if (!is_partial_io(bvec))
uncmem = user_mem;
@@ -629,7 +634,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
goto out_cleanup;
}
- ret = zram_decompress_page(zram, uncmem, index);
+ ret = zram_decompress_page(zram, zstrm, uncmem, index);
/* Should NEVER happen. Return bio error if it does. */
if (unlikely(ret))
goto out_cleanup;
@@ -642,6 +647,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
ret = 0;
out_cleanup:
kunmap_atomic(user_mem);
+ zcomp_strm_release(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
return ret;
@@ -651,7 +657,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
int offset)
{
int ret = 0;
- size_t clen;
+ unsigned int clen;
unsigned long handle;
struct page *page;
unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
@@ -670,12 +676,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
- ret = zram_decompress_page(zram, uncmem, index);
+ zstrm = zcomp_strm_find(zram->comp);
+ ret = zram_decompress_page(zram, zstrm, uncmem, index);
if (ret)
goto out;
}
- zstrm = zcomp_strm_find(zram->comp);
+ if (!zstrm)
+ zstrm = zcomp_strm_find(zram->comp);
user_mem = kmap_atomic(page);
if (is_partial_io(bvec)) {
@@ -721,7 +729,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
handle = zs_malloc(meta->mem_pool, clen);
if (!handle) {
- pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
+ pr_info("Error allocating memory for compressed page: %u, size=%u\n",
index, clen);
ret = -ENOMEM;
goto out;
--
1.9.1
On Thu, Aug 13, 2015 at 11:24:13AM +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 we don't do that, we cannot
> utilize various compression algorithms in the system. To improve this
> situation, zram will be changed to use crypto subsystem in following
> patch. There is one problem with this change. Zram has a interface
> that what compression algorithm it can support. Although crypto subsystem
> has /proc interface to search all of crypto algorithm, but, there is
> no way to get just compression algorithm in cryto subsystem. To implement
> it on zram-side, crypto_alg_list and rwsem should be exported so
> this patch do it.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
Nack. We already have a netlink interface that can be used to
query algorithms and the type information is present in the report.
The interface is crypto_user and should be used instead of exporting
the raw list.
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 (08/13/15 11:24), 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.
>
I don't like this change, sorry.
Here is why. One of the main reasons why I haven't implemented it using
crypto API (and it was an option and I thought about it) was and still
is the fact that crypto API requires tfm for both compress and decompress
operations. And this is a show stopper. No matter how you implement it,
it is slower than what we have by definition.
Now zram_bvec_read() operations depends on:
a) other read operations
because read() path now use limited in size idle stream list
b) write operations
because write() path uses same idle streams list
Literally, you change zram_bvec_read() from a fast
zram_decompress_page(zram, uncmem, index);
to a slow
zstrm = zcomp_strm_find(zram->comp);
zram_decompress_page(zram, zstrm, uncmem, index);
zcomp_strm_release(zram->comp, zstrm);
you either slow down both write() and read() if you use a single idle
stream list for both read and write, or double the amount of memory
used by idle streams if you decide to use separate read and write
idle stream lists. I see no real reason to do either of those. I like
that the existing read() is as fast as probably possible.
-ss
> 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 to compress the page 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.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> drivers/block/zram/Kconfig | 13 +------
> drivers/block/zram/Makefile | 4 +-
> drivers/block/zram/zcomp.c | 87 ++++++++++++++++++------------------------
> drivers/block/zram/zcomp.h | 34 +++++------------
> 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 | 28 +++++++++-----
> 9 files changed, 66 insertions(+), 228 deletions(-)
> delete mode 100644 drivers/block/zram/zcomp_lz4.c
> delete mode 100644 drivers/block/zram/zcomp_lz4.h
> delete mode 100644 drivers/block/zram/zcomp_lzo.c
> delete mode 100644 drivers/block/zram/zcomp_lzo.h
>
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index 386ba3d..36ec96f 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -1,8 +1,7 @@
> config ZRAM
> tristate "Compressed RAM block device support"
> depends on BLOCK && SYSFS && ZSMALLOC
> - select LZO_COMPRESS
> - select LZO_DECOMPRESS
> + select CRYPTO_LZO
> default n
> help
> Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
> @@ -14,13 +13,3 @@ config ZRAM
> disks and maybe many more.
>
> See zram.txt for more information.
> -
> -config ZRAM_LZ4_COMPRESS
> - bool "Enable LZ4 algorithm support"
> - depends on ZRAM
> - select LZ4_COMPRESS
> - select LZ4_DECOMPRESS
> - default n
> - help
> - This option enables LZ4 compression algorithm support. Compression
> - algorithm can be changed using `comp_algorithm' device attribute.
> \ No newline at end of file
> diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
> index be0763f..9e2b79e 100644
> --- a/drivers/block/zram/Makefile
> +++ b/drivers/block/zram/Makefile
> @@ -1,5 +1,3 @@
> -zram-y := zcomp_lzo.o zcomp.o zram_drv.o
> -
> -zram-$(CONFIG_ZRAM_LZ4_COMPRESS) += zcomp_lz4.o
> +zram-y := zcomp.o zram_drv.o
>
> obj-$(CONFIG_ZRAM) += zram.o
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index 965d1af..3dc9bfa 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -13,12 +13,9 @@
> #include <linux/slab.h>
> #include <linux/wait.h>
> #include <linux/sched.h>
> +#include <linux/crypto.h>
>
> #include "zcomp.h"
> -#include "zcomp_lzo.h"
> -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> -#include "zcomp_lz4.h"
> -#endif
>
> /*
> * single zcomp_strm backend
> @@ -43,36 +40,17 @@ struct zcomp_strm_multi {
> wait_queue_head_t strm_wait;
> };
>
> -static struct zcomp_backend *backends[] = {
> - &zcomp_lzo,
> -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> - &zcomp_lz4,
> -#endif
> - NULL
> -};
> -
> -static struct zcomp_backend *find_backend(const char *compress)
> -{
> - int i = 0;
> - while (backends[i]) {
> - if (sysfs_streq(compress, backends[i]->name))
> - break;
> - i++;
> - }
> - return backends[i];
> -}
> -
> static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> {
> - if (zstrm->private)
> - comp->backend->destroy(zstrm->private);
> + if (zstrm->tfm)
> + crypto_free_comp(zstrm->tfm);
> free_pages((unsigned long)zstrm->buffer, 1);
> kfree(zstrm);
> }
>
> /*
> - * allocate new zcomp_strm structure with ->private initialized by
> - * backend, return NULL on error
> + * allocate new zcomp_strm structure with initializing crypto data structure,
> + * return NULL on error
> */
> static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> {
> @@ -80,13 +58,18 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> if (!zstrm)
> return NULL;
>
> - zstrm->private = comp->backend->create();
> + zstrm->tfm = crypto_alloc_comp(comp->name, 0, 0);
> + if (IS_ERR(zstrm->tfm)) {
> + kfree(zstrm);
> + return NULL;
> + }
> +
> /*
> * allocate 2 pages. 1 for compressed data, plus 1 extra for the
> * case when compressed size is larger than the original one
> */
> zstrm->buffer = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1);
> - if (!zstrm->private || !zstrm->buffer) {
> + if (!zstrm->buffer) {
> zcomp_strm_free(comp, zstrm);
> zstrm = NULL;
> }
> @@ -270,25 +253,26 @@ static int zcomp_strm_single_create(struct zcomp *comp)
> /* show available compressors */
> ssize_t zcomp_available_show(const char *comp, char *buf)
> {
> + struct crypto_alg *alg;
> ssize_t sz = 0;
> - int i = 0;
>
> - while (backends[i]) {
> - if (!strcmp(comp, backends[i]->name))
> + down_read(&crypto_alg_sem);
> + list_for_each_entry(alg, &crypto_alg_list, cra_list) {
> + if ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK)
> + != CRYPTO_ALG_TYPE_COMPRESS)
> + continue;
> +
> + if (sysfs_streq(comp, alg->cra_name))
> sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
> - "[%s] ", backends[i]->name);
> + "[%s] ", alg->cra_name);
> else
> sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
> - "%s ", backends[i]->name);
> - i++;
> + "%s ", alg->cra_name);
> }
> sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
> - return sz;
> -}
> + up_read(&crypto_alg_sem);
>
> -bool zcomp_available_algorithm(const char *comp)
> -{
> - return find_backend(comp) != NULL;
> + return sz;
> }
>
> bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
> @@ -307,16 +291,21 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
> }
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> - const unsigned char *src, 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, const unsigned char *src,
> - size_t src_len, unsigned char *dst)
> +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> + const unsigned char *src, unsigned int src_len,
> + unsigned char *dst)
> {
> - return comp->backend->decompress(src, src_len, dst);
> + unsigned int size = PAGE_SIZE;
> +
> + return crypto_comp_decompress(zstrm->tfm, src, src_len,
> + dst, &size);
> }
>
> void zcomp_destroy(struct zcomp *comp)
> @@ -335,17 +324,15 @@ void zcomp_destroy(struct zcomp *comp)
> struct zcomp *zcomp_create(const char *compress, int max_strm)
> {
> struct zcomp *comp;
> - struct zcomp_backend *backend;
>
> - backend = find_backend(compress);
> - if (!backend)
> + if (!crypto_has_comp(compress, 0, 0))
> return ERR_PTR(-EINVAL);
>
> comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
> if (!comp)
> return ERR_PTR(-ENOMEM);
>
> - comp->backend = backend;
> + comp->name = compress;
> if (max_strm > 1)
> zcomp_strm_multi_create(comp, max_strm);
> else
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> index 46e2b9f..35f7f10 100644
> --- a/drivers/block/zram/zcomp.h
> +++ b/drivers/block/zram/zcomp.h
> @@ -10,39 +10,23 @@
> #ifndef _ZCOMP_H_
> #define _ZCOMP_H_
>
> +#include <linux/crypto.h>
> #include <linux/mutex.h>
>
> struct zcomp_strm {
> + struct crypto_comp *tfm;
> +
> /* compression/decompression buffer */
> void *buffer;
> - /*
> - * The private data of the compression stream, only compression
> - * stream backend can touch this (e.g. compression algorithm
> - * working memory)
> - */
> - void *private;
> +
> /* used in multi stream backend, protected by backend strm_lock */
> struct list_head list;
> };
>
> -/* static compression backend */
> -struct zcomp_backend {
> - int (*compress)(const unsigned char *src, unsigned char *dst,
> - size_t *dst_len, void *private);
> -
> - int (*decompress)(const unsigned char *src, size_t src_len,
> - unsigned char *dst);
> -
> - void *(*create)(void);
> - void (*destroy)(void *private);
> -
> - const char *name;
> -};
> -
> /* dynamic per-device compression frontend */
> struct zcomp {
> void *stream;
> - struct zcomp_backend *backend;
> + const char *name;
>
> struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> @@ -51,7 +35,6 @@ struct zcomp {
> };
>
> ssize_t zcomp_available_show(const char *comp, char *buf);
> -bool zcomp_available_algorithm(const char *comp);
>
> struct zcomp *zcomp_create(const char *comp, int max_strm);
> void zcomp_destroy(struct zcomp *comp);
> @@ -60,10 +43,11 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
> void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
>
> int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> - const unsigned char *src, size_t *dst_len);
> + const unsigned char *src, unsigned int *dst_len);
>
> -int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> - size_t src_len, unsigned char *dst);
> +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> + const unsigned char *src, unsigned int src_len,
> + unsigned char *dst);
>
> bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
> #endif /* _ZCOMP_H_ */
> diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
> deleted file mode 100644
> index f2afb7e..0000000
> --- a/drivers/block/zram/zcomp_lz4.c
> +++ /dev/null
> @@ -1,47 +0,0 @@
> -/*
> - * Copyright (C) 2014 Sergey Senozhatsky.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * as published by the Free Software Foundation; either version
> - * 2 of the License, or (at your option) any later version.
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/lz4.h>
> -
> -#include "zcomp_lz4.h"
> -
> -static void *zcomp_lz4_create(void)
> -{
> - return kzalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
> -}
> -
> -static void zcomp_lz4_destroy(void *private)
> -{
> - kfree(private);
> -}
> -
> -static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst,
> - size_t *dst_len, void *private)
> -{
> - /* return : Success if return 0 */
> - return lz4_compress(src, PAGE_SIZE, dst, dst_len, private);
> -}
> -
> -static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len,
> - unsigned char *dst)
> -{
> - size_t dst_len = PAGE_SIZE;
> - /* return : Success if return 0 */
> - return lz4_decompress_unknownoutputsize(src, src_len, dst, &dst_len);
> -}
> -
> -struct zcomp_backend zcomp_lz4 = {
> - .compress = zcomp_lz4_compress,
> - .decompress = zcomp_lz4_decompress,
> - .create = zcomp_lz4_create,
> - .destroy = zcomp_lz4_destroy,
> - .name = "lz4",
> -};
> diff --git a/drivers/block/zram/zcomp_lz4.h b/drivers/block/zram/zcomp_lz4.h
> deleted file mode 100644
> index 60613fb..0000000
> --- a/drivers/block/zram/zcomp_lz4.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/*
> - * Copyright (C) 2014 Sergey Senozhatsky.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * as published by the Free Software Foundation; either version
> - * 2 of the License, or (at your option) any later version.
> - */
> -
> -#ifndef _ZCOMP_LZ4_H_
> -#define _ZCOMP_LZ4_H_
> -
> -#include "zcomp.h"
> -
> -extern struct zcomp_backend zcomp_lz4;
> -
> -#endif /* _ZCOMP_LZ4_H_ */
> diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
> deleted file mode 100644
> index da1bc47..0000000
> --- a/drivers/block/zram/zcomp_lzo.c
> +++ /dev/null
> @@ -1,47 +0,0 @@
> -/*
> - * Copyright (C) 2014 Sergey Senozhatsky.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * as published by the Free Software Foundation; either version
> - * 2 of the License, or (at your option) any later version.
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/lzo.h>
> -
> -#include "zcomp_lzo.h"
> -
> -static void *lzo_create(void)
> -{
> - return kzalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
> -}
> -
> -static void lzo_destroy(void *private)
> -{
> - kfree(private);
> -}
> -
> -static int lzo_compress(const unsigned char *src, unsigned char *dst,
> - size_t *dst_len, void *private)
> -{
> - int ret = lzo1x_1_compress(src, PAGE_SIZE, dst, dst_len, private);
> - return ret == LZO_E_OK ? 0 : ret;
> -}
> -
> -static int lzo_decompress(const unsigned char *src, size_t src_len,
> - unsigned char *dst)
> -{
> - size_t dst_len = PAGE_SIZE;
> - int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len);
> - return ret == LZO_E_OK ? 0 : ret;
> -}
> -
> -struct zcomp_backend zcomp_lzo = {
> - .compress = lzo_compress,
> - .decompress = lzo_decompress,
> - .create = lzo_create,
> - .destroy = lzo_destroy,
> - .name = "lzo",
> -};
> diff --git a/drivers/block/zram/zcomp_lzo.h b/drivers/block/zram/zcomp_lzo.h
> deleted file mode 100644
> index 128c580..0000000
> --- a/drivers/block/zram/zcomp_lzo.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/*
> - * Copyright (C) 2014 Sergey Senozhatsky.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * as published by the Free Software Foundation; either version
> - * 2 of the License, or (at your option) any later version.
> - */
> -
> -#ifndef _ZCOMP_LZO_H_
> -#define _ZCOMP_LZO_H_
> -
> -#include "zcomp.h"
> -
> -extern struct zcomp_backend zcomp_lzo;
> -
> -#endif /* _ZCOMP_LZO_H_ */
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index b088ca9..84704d8 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -30,6 +30,7 @@
> #include <linux/err.h>
> #include <linux/idr.h>
> #include <linux/sysfs.h>
> +#include <linux/crypto.h>
>
> #include "zram_drv.h"
>
> @@ -349,7 +350,7 @@ out:
> static ssize_t comp_algorithm_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - size_t sz;
> + size_t sz = 0;
> struct zram *zram = dev_to_zram(dev);
>
> down_read(&zram->init_lock);
> @@ -378,7 +379,7 @@ static ssize_t comp_algorithm_store(struct device *dev,
> if (sz > 0 && zram->compressor[sz - 1] == '\n')
> zram->compressor[sz - 1] = 0x00;
>
> - if (!zcomp_available_algorithm(zram->compressor))
> + if (!crypto_has_comp(zram->compressor, 0, 0))
> len = -EINVAL;
>
> up_write(&zram->init_lock);
> @@ -562,13 +563,14 @@ static void zram_free_page(struct zram *zram, size_t index)
> zram_set_obj_size(meta, index, 0);
> }
>
> -static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> +static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
> + char *mem, u32 index)
> {
> int ret = 0;
> unsigned char *cmem;
> struct zram_meta *meta = zram->meta;
> unsigned long handle;
> - size_t size;
> + unsigned int size;
>
> bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> handle = meta->table[index].handle;
> @@ -584,7 +586,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> if (size == PAGE_SIZE)
> copy_page(mem, cmem);
> else
> - ret = zcomp_decompress(zram->comp, cmem, size, mem);
> + ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
> zs_unmap_object(meta->mem_pool, handle);
> bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
>
> @@ -604,6 +606,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> struct page *page;
> unsigned char *user_mem, *uncmem = NULL;
> struct zram_meta *meta = zram->meta;
> + struct zcomp_strm *zstrm = NULL;
> +
> page = bvec->bv_page;
>
> bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> @@ -619,6 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> /* Use a temporary buffer to decompress the page */
> uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
>
> + zstrm = zcomp_strm_find(zram->comp);
> user_mem = kmap_atomic(page);
> if (!is_partial_io(bvec))
> uncmem = user_mem;
> @@ -629,7 +634,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> goto out_cleanup;
> }
>
> - ret = zram_decompress_page(zram, uncmem, index);
> + ret = zram_decompress_page(zram, zstrm, uncmem, index);
> /* Should NEVER happen. Return bio error if it does. */
> if (unlikely(ret))
> goto out_cleanup;
> @@ -642,6 +647,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> ret = 0;
> out_cleanup:
> kunmap_atomic(user_mem);
> + zcomp_strm_release(zram->comp, zstrm);
> if (is_partial_io(bvec))
> kfree(uncmem);
> return ret;
> @@ -651,7 +657,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> int offset)
> {
> int ret = 0;
> - size_t clen;
> + unsigned int clen;
> unsigned long handle;
> struct page *page;
> unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
> @@ -670,12 +676,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> ret = -ENOMEM;
> goto out;
> }
> - ret = zram_decompress_page(zram, uncmem, index);
> + zstrm = zcomp_strm_find(zram->comp);
> + ret = zram_decompress_page(zram, zstrm, uncmem, index);
> if (ret)
> goto out;
> }
>
> - zstrm = zcomp_strm_find(zram->comp);
> + if (!zstrm)
> + zstrm = zcomp_strm_find(zram->comp);
> user_mem = kmap_atomic(page);
>
> if (is_partial_io(bvec)) {
> @@ -721,7 +729,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
>
> handle = zs_malloc(meta->mem_pool, clen);
> if (!handle) {
> - pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
> + pr_info("Error allocating memory for compressed page: %u, size=%u\n",
> index, clen);
> ret = -ENOMEM;
> goto out;
> --
> 1.9.1
>
Removed Herbert, David, Stephan not spam their mail boxes.
I'd rather investigate something like this.
WARNING!!! NOT TESTED AT ALL! like for real... only compile tested.
RFC
define ZLIB backend the same way as we already do. and introduce
compression frontend ->flags field. backend that requires zstream
for read op will set it to ZCOMP_NEED_READ_STRM. zram checks this
flag in read() path and does slow zcomp_strm_find()/zcomp_strm_release()
if backend requires it; otherwise we have our fast "just decompress it"
path.
this is very ugly and quick. just as an idea at the moment.
I will return to this a bit later.
not-yet-signed-off-by: Sergey Senozhatsky <[email protected]>
---
drivers/block/zram/Kconfig | 12 ++++-
drivers/block/zram/Makefile | 1 +
drivers/block/zram/zcomp.c | 21 +++++++-
drivers/block/zram/zcomp.h | 9 +++-
drivers/block/zram/zcomp_lz4.c | 2 +-
drivers/block/zram/zcomp_lzo.c | 2 +-
drivers/block/zram/zcomp_zlib.c | 115 ++++++++++++++++++++++++++++++++++++++++
drivers/block/zram/zcomp_zlib.h | 17 ++++++
drivers/block/zram/zram_drv.c | 15 ++++--
9 files changed, 184 insertions(+), 10 deletions(-)
create mode 100644 drivers/block/zram/zcomp_zlib.c
create mode 100644 drivers/block/zram/zcomp_zlib.h
diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 386ba3d..7afd2ac 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -23,4 +23,14 @@ config ZRAM_LZ4_COMPRESS
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.
+
+config ZRAM_ZLIB_COMPRESS
+ bool "Enable ZLIB algorithm support"
+ depends on ZRAM
+ select ZLIB_INFLATE
+ select ZLIB_DEFLATE
+ default n
+ help
+ This option enables ZLIB compression algorithm support. Compression
+ algorithm can be changed using `comp_algorithm' device attribute.
diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
index be0763f..0922f54 100644
--- a/drivers/block/zram/Makefile
+++ b/drivers/block/zram/Makefile
@@ -1,5 +1,6 @@
zram-y := zcomp_lzo.o zcomp.o zram_drv.o
zram-$(CONFIG_ZRAM_LZ4_COMPRESS) += zcomp_lz4.o
+zram-$(CONFIG_ZRAM_ZLIB_COMPRESS) += zcomp_zlib.o
obj-$(CONFIG_ZRAM) += zram.o
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 965d1af..4f04186 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -19,6 +19,9 @@
#ifdef CONFIG_ZRAM_LZ4_COMPRESS
#include "zcomp_lz4.h"
#endif
+#ifdef CONFIG_ZRAM_ZLIB_COMPRESS
+#include "zcomp_zlib.h"
+#endif
/*
* single zcomp_strm backend
@@ -48,6 +51,9 @@ static struct zcomp_backend *backends[] = {
#ifdef CONFIG_ZRAM_LZ4_COMPRESS
&zcomp_lz4,
#endif
+#ifdef CONFIG_ZRAM_ZLIB_COMPRESS
+ &zcomp_zlib,
+#endif
NULL
};
@@ -313,10 +319,16 @@ 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);
+ void *private = NULL;
+
+ if (unlikely(zstrm))
+ private = zstrm->private;
+
+ return comp->backend->decompress(src, src_len, dst, private);
}
void zcomp_destroy(struct zcomp *comp)
@@ -354,5 +366,10 @@ struct zcomp *zcomp_create(const char *compress, int max_strm)
kfree(comp);
return ERR_PTR(-ENOMEM);
}
+
+ /* FIXME quick dirty and ugly. ONLY for testing purposes */
+ if (sysfs_streq(compress, "zlib"))
+ comp->flags |= ZCOMP_NEED_READ_STRM;
+
return comp;
}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 46e2b9f..e3ac8d3 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -12,6 +12,8 @@
#include <linux/mutex.h>
+#define ZCOMP_NEED_READ_STRM (1 << 0)
+
struct zcomp_strm {
/* compression/decompression buffer */
void *buffer;
@@ -31,7 +33,7 @@ struct zcomp_backend {
size_t *dst_len, void *private);
int (*decompress)(const unsigned char *src, size_t src_len,
- unsigned char *dst);
+ unsigned char *dst, void *private);
void *(*create)(void);
void (*destroy)(void *private);
@@ -44,6 +46,8 @@ struct zcomp {
void *stream;
struct zcomp_backend *backend;
+ int flags;
+
struct zcomp_strm *(*strm_find)(struct zcomp *comp);
void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
bool (*set_max_streams)(struct zcomp *comp, int num_strm);
@@ -62,7 +66,8 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
const unsigned char *src, size_t *dst_len);
-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/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
index f2afb7e..52c85f9 100644
--- a/drivers/block/zram/zcomp_lz4.c
+++ b/drivers/block/zram/zcomp_lz4.c
@@ -31,7 +31,7 @@ static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst,
}
static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
+ unsigned char *dst, void *private)
{
size_t dst_len = PAGE_SIZE;
/* return : Success if return 0 */
diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
index da1bc47..11ba295 100644
--- a/drivers/block/zram/zcomp_lzo.c
+++ b/drivers/block/zram/zcomp_lzo.c
@@ -31,7 +31,7 @@ static int lzo_compress(const unsigned char *src, unsigned char *dst,
}
static int lzo_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
+ unsigned char *dst, void *private)
{
size_t dst_len = PAGE_SIZE;
int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len);
diff --git a/drivers/block/zram/zcomp_zlib.c b/drivers/block/zram/zcomp_zlib.c
new file mode 100644
index 0000000..f9ddcd7
--- /dev/null
+++ b/drivers/block/zram/zcomp_zlib.c
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2015 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/vmalloc.h>
+#include <linux/zlib.h>
+
+#include "zcomp_zlib.h"
+
+#define ZLIB_COMPRESSION_LEVEL 3
+
+static void *zlib_create(void)
+{
+ z_stream *stream;
+ size_t size;
+
+ stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+ if (!stream)
+ return NULL;
+
+ size = max(zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL),
+ zlib_inflate_workspacesize());
+ stream->workspace = vmalloc(size);
+ if (!stream->workspace) {
+ kfree(stream);
+ stream = NULL;
+ }
+
+ return stream;
+}
+
+static void zlib_destroy(void *private)
+{
+ z_stream *stream = private;
+
+ vfree(stream->workspace);
+ kfree(stream);
+}
+
+static int zlib_compress(const unsigned char *src, unsigned char *dst,
+ size_t *dst_len, void *private)
+{
+ z_stream *stream = private;
+ int err;
+
+ err = zlib_deflateInit(stream, ZLIB_COMPRESSION_LEVEL);
+ if (err != Z_OK)
+ goto out;
+
+ stream->next_in = src;
+ stream->avail_in = PAGE_SIZE;
+ stream->total_in = 0;
+ stream->next_out = dst;
+ stream->avail_out = PAGE_SIZE;
+ stream->total_out = 0;
+
+ err = zlib_deflate(stream, Z_FINISH);
+ if (err != Z_STREAM_END)
+ goto out;
+
+ err = zlib_deflateEnd(stream);
+ if (err != Z_OK)
+ goto out;
+
+ if (stream->total_out >= stream->total_in)
+ goto out;
+
+ *dst_len = stream->total_out;
+out:
+ return err == Z_OK ? 0 : err;
+}
+
+static int zlib_decompress(const unsigned char *src, size_t src_len,
+ unsigned char *dst, void *private)
+{
+ z_stream *stream = private;
+ int err;
+
+ err = zlib_inflateInit(stream);
+ if (err != Z_OK)
+ goto out;
+
+ stream->next_in = src;
+ stream->avail_in = src_len;
+ stream->total_in = 0;
+ stream->next_out = dst;
+ stream->avail_out = PAGE_SIZE;
+ stream->total_out = 0;
+
+ err = zlib_inflate(stream, Z_FINISH);
+ if (err != Z_STREAM_END)
+ goto out;
+
+ err = zlib_inflateEnd(stream);
+ if (err != Z_OK)
+ goto out;
+
+out:
+ return err == Z_OK ? 0 : err;
+}
+
+struct zcomp_backend zcomp_zlib = {
+ .compress = zlib_compress,
+ .decompress = zlib_decompress,
+ .create = zlib_create,
+ .destroy = zlib_destroy,
+ .name = "zlib",
+};
diff --git a/drivers/block/zram/zcomp_zlib.h b/drivers/block/zram/zcomp_zlib.h
new file mode 100644
index 0000000..d0e4fa0
--- /dev/null
+++ b/drivers/block/zram/zcomp_zlib.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2015 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_ZLIB_H_
+#define _ZCOMP_ZLIB_H_
+
+#include "zcomp.h"
+
+extern struct zcomp_backend zcomp_zlib;
+
+#endif /* _ZCOMP_ZLIB_H_ */
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index b6fdafc..72b0b0f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -587,10 +587,19 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
}
cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_RO);
- if (size == PAGE_SIZE)
+ if (size == PAGE_SIZE) {
copy_page(mem, cmem);
- else
- ret = zcomp_decompress(zram->comp, cmem, size, mem);
+ } else {
+ struct zcomp_strm *zstrm = NULL;
+
+ if (unlikely(zram->comp->flags & ZCOMP_NEED_READ_STRM))
+ zstrm = zcomp_strm_find(zram->comp);
+
+ ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
+
+ if (unlikely(zstrm))
+ zcomp_strm_release(zram->comp, zstrm);
+ }
zs_unmap_object(meta->mem_pool, handle);
bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
--
2.5.0.234.gefc8a62
On Thu, Aug 13, 2015 at 11:19:54AM +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2015 at 11:24:13AM +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 we don't do that, we cannot
> > utilize various compression algorithms in the system. To improve this
> > situation, zram will be changed to use crypto subsystem in following
> > patch. There is one problem with this change. Zram has a interface
> > that what compression algorithm it can support. Although crypto subsystem
> > has /proc interface to search all of crypto algorithm, but, there is
> > no way to get just compression algorithm in cryto subsystem. To implement
> > it on zram-side, crypto_alg_list and rwsem should be exported so
> > this patch do it.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
>
> Nack. We already have a netlink interface that can be used to
> query algorithms and the type information is present in the report.
> The interface is crypto_user and should be used instead of exporting
> the raw list.
Oh... I see.
Is there any way to access netlink interface and get the output from
kernel-side? I'd like to show information through
"/sys/block/zramX/comp_algorithm", because some user program can be
broken if we change output of userspace exposed interface.
Thanks.
On Thu, Aug 13, 2015 at 03:37:55PM +0900, Joonsoo Kim wrote:
>
> Is there any way to access netlink interface and get the output from
> kernel-side? I'd like to show information through
> "/sys/block/zramX/comp_algorithm", because some user program can be
> broken if we change output of userspace exposed interface.
You could simply deprecate that interface but keep it for existing
algorithms. Any new algorithms can then be queried through the
crypto_user interface.
The other option is to have a defined list of algorithms which is
independent of the crypto API. For example, have a look at what
IPsec does in net/xfrm/xfrm_algo.c. IPsec needs its own list
because they come with annotations.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Aug 13, 2015 at 12:51:13PM +0900, Sergey Senozhatsky wrote:
> On (08/13/15 11:24), 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.
> >
>
> I don't like this change, sorry.
>
> Here is why. One of the main reasons why I haven't implemented it using
> crypto API (and it was an option and I thought about it) was and still
> is the fact that crypto API requires tfm for both compress and decompress
> operations. And this is a show stopper. No matter how you implement it,
> it is slower than what we have by definition.
I don't think it is show stopper. Current implementation that uses zstrm
only for compression seems over-optimized thing to me. Some of other
compression algorithms need scratch-buffer to decompress the contents.
As you know, zlib is one of them. I tested some of other decompression
algorithm such as quicklz, wkdm and they also need scratch-buffer.
And, we cannot support crypto algorithm *module* in current
implementation.
If that optimization is really needed for the case that doesn't need
tfm except fetching function pointer, we can implement sharable tfm
in crypto subsystem.
>
> Now zram_bvec_read() operations depends on:
>
> a) other read operations
> because read() path now use limited in size idle stream list
>
> b) write operations
> because write() path uses same idle streams list
>
>
>
> Literally, you change zram_bvec_read() from a fast
>
> zram_decompress_page(zram, uncmem, index);
>
> to a slow
>
> zstrm = zcomp_strm_find(zram->comp);
> zram_decompress_page(zram, zstrm, uncmem, index);
> zcomp_strm_release(zram->comp, zstrm);
>
>
> you either slow down both write() and read() if you use a single idle
> stream list for both read and write, or double the amount of memory
> used by idle streams if you decide to use separate read and write
> idle stream lists. I see no real reason to do either of those. I like
> that the existing read() is as fast as probably possible.
Yes, there can be some performace regression. But, by allocating some
more streams, we can easily eliminate regression. Memory overhead
would be really small. I think that gaining flexibility and reducing
management overhead can compensate this small memory overhead.
Thanks.
>
> > 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 to compress the page 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.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
> > ---
> > drivers/block/zram/Kconfig | 13 +------
> > drivers/block/zram/Makefile | 4 +-
> > drivers/block/zram/zcomp.c | 87 ++++++++++++++++++------------------------
> > drivers/block/zram/zcomp.h | 34 +++++------------
> > 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 | 28 +++++++++-----
> > 9 files changed, 66 insertions(+), 228 deletions(-)
> > delete mode 100644 drivers/block/zram/zcomp_lz4.c
> > delete mode 100644 drivers/block/zram/zcomp_lz4.h
> > delete mode 100644 drivers/block/zram/zcomp_lzo.c
> > delete mode 100644 drivers/block/zram/zcomp_lzo.h
> >
> > diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> > index 386ba3d..36ec96f 100644
> > --- a/drivers/block/zram/Kconfig
> > +++ b/drivers/block/zram/Kconfig
> > @@ -1,8 +1,7 @@
> > config ZRAM
> > tristate "Compressed RAM block device support"
> > depends on BLOCK && SYSFS && ZSMALLOC
> > - select LZO_COMPRESS
> > - select LZO_DECOMPRESS
> > + select CRYPTO_LZO
> > default n
> > help
> > Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
> > @@ -14,13 +13,3 @@ config ZRAM
> > disks and maybe many more.
> >
> > See zram.txt for more information.
> > -
> > -config ZRAM_LZ4_COMPRESS
> > - bool "Enable LZ4 algorithm support"
> > - depends on ZRAM
> > - select LZ4_COMPRESS
> > - select LZ4_DECOMPRESS
> > - default n
> > - help
> > - This option enables LZ4 compression algorithm support. Compression
> > - algorithm can be changed using `comp_algorithm' device attribute.
> > \ No newline at end of file
> > diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
> > index be0763f..9e2b79e 100644
> > --- a/drivers/block/zram/Makefile
> > +++ b/drivers/block/zram/Makefile
> > @@ -1,5 +1,3 @@
> > -zram-y := zcomp_lzo.o zcomp.o zram_drv.o
> > -
> > -zram-$(CONFIG_ZRAM_LZ4_COMPRESS) += zcomp_lz4.o
> > +zram-y := zcomp.o zram_drv.o
> >
> > obj-$(CONFIG_ZRAM) += zram.o
> > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> > index 965d1af..3dc9bfa 100644
> > --- a/drivers/block/zram/zcomp.c
> > +++ b/drivers/block/zram/zcomp.c
> > @@ -13,12 +13,9 @@
> > #include <linux/slab.h>
> > #include <linux/wait.h>
> > #include <linux/sched.h>
> > +#include <linux/crypto.h>
> >
> > #include "zcomp.h"
> > -#include "zcomp_lzo.h"
> > -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> > -#include "zcomp_lz4.h"
> > -#endif
> >
> > /*
> > * single zcomp_strm backend
> > @@ -43,36 +40,17 @@ struct zcomp_strm_multi {
> > wait_queue_head_t strm_wait;
> > };
> >
> > -static struct zcomp_backend *backends[] = {
> > - &zcomp_lzo,
> > -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> > - &zcomp_lz4,
> > -#endif
> > - NULL
> > -};
> > -
> > -static struct zcomp_backend *find_backend(const char *compress)
> > -{
> > - int i = 0;
> > - while (backends[i]) {
> > - if (sysfs_streq(compress, backends[i]->name))
> > - break;
> > - i++;
> > - }
> > - return backends[i];
> > -}
> > -
> > static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> > {
> > - if (zstrm->private)
> > - comp->backend->destroy(zstrm->private);
> > + if (zstrm->tfm)
> > + crypto_free_comp(zstrm->tfm);
> > free_pages((unsigned long)zstrm->buffer, 1);
> > kfree(zstrm);
> > }
> >
> > /*
> > - * allocate new zcomp_strm structure with ->private initialized by
> > - * backend, return NULL on error
> > + * allocate new zcomp_strm structure with initializing crypto data structure,
> > + * return NULL on error
> > */
> > static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> > {
> > @@ -80,13 +58,18 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> > if (!zstrm)
> > return NULL;
> >
> > - zstrm->private = comp->backend->create();
> > + zstrm->tfm = crypto_alloc_comp(comp->name, 0, 0);
> > + if (IS_ERR(zstrm->tfm)) {
> > + kfree(zstrm);
> > + return NULL;
> > + }
> > +
> > /*
> > * allocate 2 pages. 1 for compressed data, plus 1 extra for the
> > * case when compressed size is larger than the original one
> > */
> > zstrm->buffer = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1);
> > - if (!zstrm->private || !zstrm->buffer) {
> > + if (!zstrm->buffer) {
> > zcomp_strm_free(comp, zstrm);
> > zstrm = NULL;
> > }
> > @@ -270,25 +253,26 @@ static int zcomp_strm_single_create(struct zcomp *comp)
> > /* show available compressors */
> > ssize_t zcomp_available_show(const char *comp, char *buf)
> > {
> > + struct crypto_alg *alg;
> > ssize_t sz = 0;
> > - int i = 0;
> >
> > - while (backends[i]) {
> > - if (!strcmp(comp, backends[i]->name))
> > + down_read(&crypto_alg_sem);
> > + list_for_each_entry(alg, &crypto_alg_list, cra_list) {
> > + if ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK)
> > + != CRYPTO_ALG_TYPE_COMPRESS)
> > + continue;
> > +
> > + if (sysfs_streq(comp, alg->cra_name))
> > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
> > - "[%s] ", backends[i]->name);
> > + "[%s] ", alg->cra_name);
> > else
> > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
> > - "%s ", backends[i]->name);
> > - i++;
> > + "%s ", alg->cra_name);
> > }
> > sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
> > - return sz;
> > -}
> > + up_read(&crypto_alg_sem);
> >
> > -bool zcomp_available_algorithm(const char *comp)
> > -{
> > - return find_backend(comp) != NULL;
> > + return sz;
> > }
> >
> > bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
> > @@ -307,16 +291,21 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
> > }
> >
> > int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > - const unsigned char *src, 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, const unsigned char *src,
> > - size_t src_len, unsigned char *dst)
> > +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > + const unsigned char *src, unsigned int src_len,
> > + unsigned char *dst)
> > {
> > - return comp->backend->decompress(src, src_len, dst);
> > + unsigned int size = PAGE_SIZE;
> > +
> > + return crypto_comp_decompress(zstrm->tfm, src, src_len,
> > + dst, &size);
> > }
> >
> > void zcomp_destroy(struct zcomp *comp)
> > @@ -335,17 +324,15 @@ void zcomp_destroy(struct zcomp *comp)
> > struct zcomp *zcomp_create(const char *compress, int max_strm)
> > {
> > struct zcomp *comp;
> > - struct zcomp_backend *backend;
> >
> > - backend = find_backend(compress);
> > - if (!backend)
> > + if (!crypto_has_comp(compress, 0, 0))
> > return ERR_PTR(-EINVAL);
> >
> > comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
> > if (!comp)
> > return ERR_PTR(-ENOMEM);
> >
> > - comp->backend = backend;
> > + comp->name = compress;
> > if (max_strm > 1)
> > zcomp_strm_multi_create(comp, max_strm);
> > else
> > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> > index 46e2b9f..35f7f10 100644
> > --- a/drivers/block/zram/zcomp.h
> > +++ b/drivers/block/zram/zcomp.h
> > @@ -10,39 +10,23 @@
> > #ifndef _ZCOMP_H_
> > #define _ZCOMP_H_
> >
> > +#include <linux/crypto.h>
> > #include <linux/mutex.h>
> >
> > struct zcomp_strm {
> > + struct crypto_comp *tfm;
> > +
> > /* compression/decompression buffer */
> > void *buffer;
> > - /*
> > - * The private data of the compression stream, only compression
> > - * stream backend can touch this (e.g. compression algorithm
> > - * working memory)
> > - */
> > - void *private;
> > +
> > /* used in multi stream backend, protected by backend strm_lock */
> > struct list_head list;
> > };
> >
> > -/* static compression backend */
> > -struct zcomp_backend {
> > - int (*compress)(const unsigned char *src, unsigned char *dst,
> > - size_t *dst_len, void *private);
> > -
> > - int (*decompress)(const unsigned char *src, size_t src_len,
> > - unsigned char *dst);
> > -
> > - void *(*create)(void);
> > - void (*destroy)(void *private);
> > -
> > - const char *name;
> > -};
> > -
> > /* dynamic per-device compression frontend */
> > struct zcomp {
> > void *stream;
> > - struct zcomp_backend *backend;
> > + const char *name;
> >
> > struct zcomp_strm *(*strm_find)(struct zcomp *comp);
> > void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
> > @@ -51,7 +35,6 @@ struct zcomp {
> > };
> >
> > ssize_t zcomp_available_show(const char *comp, char *buf);
> > -bool zcomp_available_algorithm(const char *comp);
> >
> > struct zcomp *zcomp_create(const char *comp, int max_strm);
> > void zcomp_destroy(struct zcomp *comp);
> > @@ -60,10 +43,11 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
> > void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
> >
> > int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > - const unsigned char *src, size_t *dst_len);
> > + const unsigned char *src, unsigned int *dst_len);
> >
> > -int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> > - size_t src_len, unsigned char *dst);
> > +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > + const unsigned char *src, unsigned int src_len,
> > + unsigned char *dst);
> >
> > bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
> > #endif /* _ZCOMP_H_ */
> > diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
> > deleted file mode 100644
> > index f2afb7e..0000000
> > --- a/drivers/block/zram/zcomp_lz4.c
> > +++ /dev/null
> > @@ -1,47 +0,0 @@
> > -/*
> > - * Copyright (C) 2014 Sergey Senozhatsky.
> > - *
> > - * This program is free software; you can redistribute it and/or
> > - * modify it under the terms of the GNU General Public License
> > - * as published by the Free Software Foundation; either version
> > - * 2 of the License, or (at your option) any later version.
> > - */
> > -
> > -#include <linux/kernel.h>
> > -#include <linux/slab.h>
> > -#include <linux/lz4.h>
> > -
> > -#include "zcomp_lz4.h"
> > -
> > -static void *zcomp_lz4_create(void)
> > -{
> > - return kzalloc(LZ4_MEM_COMPRESS, GFP_KERNEL);
> > -}
> > -
> > -static void zcomp_lz4_destroy(void *private)
> > -{
> > - kfree(private);
> > -}
> > -
> > -static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst,
> > - size_t *dst_len, void *private)
> > -{
> > - /* return : Success if return 0 */
> > - return lz4_compress(src, PAGE_SIZE, dst, dst_len, private);
> > -}
> > -
> > -static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len,
> > - unsigned char *dst)
> > -{
> > - size_t dst_len = PAGE_SIZE;
> > - /* return : Success if return 0 */
> > - return lz4_decompress_unknownoutputsize(src, src_len, dst, &dst_len);
> > -}
> > -
> > -struct zcomp_backend zcomp_lz4 = {
> > - .compress = zcomp_lz4_compress,
> > - .decompress = zcomp_lz4_decompress,
> > - .create = zcomp_lz4_create,
> > - .destroy = zcomp_lz4_destroy,
> > - .name = "lz4",
> > -};
> > diff --git a/drivers/block/zram/zcomp_lz4.h b/drivers/block/zram/zcomp_lz4.h
> > deleted file mode 100644
> > index 60613fb..0000000
> > --- a/drivers/block/zram/zcomp_lz4.h
> > +++ /dev/null
> > @@ -1,17 +0,0 @@
> > -/*
> > - * Copyright (C) 2014 Sergey Senozhatsky.
> > - *
> > - * This program is free software; you can redistribute it and/or
> > - * modify it under the terms of the GNU General Public License
> > - * as published by the Free Software Foundation; either version
> > - * 2 of the License, or (at your option) any later version.
> > - */
> > -
> > -#ifndef _ZCOMP_LZ4_H_
> > -#define _ZCOMP_LZ4_H_
> > -
> > -#include "zcomp.h"
> > -
> > -extern struct zcomp_backend zcomp_lz4;
> > -
> > -#endif /* _ZCOMP_LZ4_H_ */
> > diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
> > deleted file mode 100644
> > index da1bc47..0000000
> > --- a/drivers/block/zram/zcomp_lzo.c
> > +++ /dev/null
> > @@ -1,47 +0,0 @@
> > -/*
> > - * Copyright (C) 2014 Sergey Senozhatsky.
> > - *
> > - * This program is free software; you can redistribute it and/or
> > - * modify it under the terms of the GNU General Public License
> > - * as published by the Free Software Foundation; either version
> > - * 2 of the License, or (at your option) any later version.
> > - */
> > -
> > -#include <linux/kernel.h>
> > -#include <linux/slab.h>
> > -#include <linux/lzo.h>
> > -
> > -#include "zcomp_lzo.h"
> > -
> > -static void *lzo_create(void)
> > -{
> > - return kzalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
> > -}
> > -
> > -static void lzo_destroy(void *private)
> > -{
> > - kfree(private);
> > -}
> > -
> > -static int lzo_compress(const unsigned char *src, unsigned char *dst,
> > - size_t *dst_len, void *private)
> > -{
> > - int ret = lzo1x_1_compress(src, PAGE_SIZE, dst, dst_len, private);
> > - return ret == LZO_E_OK ? 0 : ret;
> > -}
> > -
> > -static int lzo_decompress(const unsigned char *src, size_t src_len,
> > - unsigned char *dst)
> > -{
> > - size_t dst_len = PAGE_SIZE;
> > - int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len);
> > - return ret == LZO_E_OK ? 0 : ret;
> > -}
> > -
> > -struct zcomp_backend zcomp_lzo = {
> > - .compress = lzo_compress,
> > - .decompress = lzo_decompress,
> > - .create = lzo_create,
> > - .destroy = lzo_destroy,
> > - .name = "lzo",
> > -};
> > diff --git a/drivers/block/zram/zcomp_lzo.h b/drivers/block/zram/zcomp_lzo.h
> > deleted file mode 100644
> > index 128c580..0000000
> > --- a/drivers/block/zram/zcomp_lzo.h
> > +++ /dev/null
> > @@ -1,17 +0,0 @@
> > -/*
> > - * Copyright (C) 2014 Sergey Senozhatsky.
> > - *
> > - * This program is free software; you can redistribute it and/or
> > - * modify it under the terms of the GNU General Public License
> > - * as published by the Free Software Foundation; either version
> > - * 2 of the License, or (at your option) any later version.
> > - */
> > -
> > -#ifndef _ZCOMP_LZO_H_
> > -#define _ZCOMP_LZO_H_
> > -
> > -#include "zcomp.h"
> > -
> > -extern struct zcomp_backend zcomp_lzo;
> > -
> > -#endif /* _ZCOMP_LZO_H_ */
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index b088ca9..84704d8 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -30,6 +30,7 @@
> > #include <linux/err.h>
> > #include <linux/idr.h>
> > #include <linux/sysfs.h>
> > +#include <linux/crypto.h>
> >
> > #include "zram_drv.h"
> >
> > @@ -349,7 +350,7 @@ out:
> > static ssize_t comp_algorithm_show(struct device *dev,
> > struct device_attribute *attr, char *buf)
> > {
> > - size_t sz;
> > + size_t sz = 0;
> > struct zram *zram = dev_to_zram(dev);
> >
> > down_read(&zram->init_lock);
> > @@ -378,7 +379,7 @@ static ssize_t comp_algorithm_store(struct device *dev,
> > if (sz > 0 && zram->compressor[sz - 1] == '\n')
> > zram->compressor[sz - 1] = 0x00;
> >
> > - if (!zcomp_available_algorithm(zram->compressor))
> > + if (!crypto_has_comp(zram->compressor, 0, 0))
> > len = -EINVAL;
> >
> > up_write(&zram->init_lock);
> > @@ -562,13 +563,14 @@ static void zram_free_page(struct zram *zram, size_t index)
> > zram_set_obj_size(meta, index, 0);
> > }
> >
> > -static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> > +static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm,
> > + char *mem, u32 index)
> > {
> > int ret = 0;
> > unsigned char *cmem;
> > struct zram_meta *meta = zram->meta;
> > unsigned long handle;
> > - size_t size;
> > + unsigned int size;
> >
> > bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > handle = meta->table[index].handle;
> > @@ -584,7 +586,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> > if (size == PAGE_SIZE)
> > copy_page(mem, cmem);
> > else
> > - ret = zcomp_decompress(zram->comp, cmem, size, mem);
> > + ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
> > zs_unmap_object(meta->mem_pool, handle);
> > bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
> >
> > @@ -604,6 +606,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> > struct page *page;
> > unsigned char *user_mem, *uncmem = NULL;
> > struct zram_meta *meta = zram->meta;
> > + struct zcomp_strm *zstrm = NULL;
> > +
> > page = bvec->bv_page;
> >
> > bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
> > @@ -619,6 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> > /* Use a temporary buffer to decompress the page */
> > uncmem = kmalloc(PAGE_SIZE, GFP_NOIO);
> >
> > + zstrm = zcomp_strm_find(zram->comp);
> > user_mem = kmap_atomic(page);
> > if (!is_partial_io(bvec))
> > uncmem = user_mem;
> > @@ -629,7 +634,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> > goto out_cleanup;
> > }
> >
> > - ret = zram_decompress_page(zram, uncmem, index);
> > + ret = zram_decompress_page(zram, zstrm, uncmem, index);
> > /* Should NEVER happen. Return bio error if it does. */
> > if (unlikely(ret))
> > goto out_cleanup;
> > @@ -642,6 +647,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
> > ret = 0;
> > out_cleanup:
> > kunmap_atomic(user_mem);
> > + zcomp_strm_release(zram->comp, zstrm);
> > if (is_partial_io(bvec))
> > kfree(uncmem);
> > return ret;
> > @@ -651,7 +657,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> > int offset)
> > {
> > int ret = 0;
> > - size_t clen;
> > + unsigned int clen;
> > unsigned long handle;
> > struct page *page;
> > unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
> > @@ -670,12 +676,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> > ret = -ENOMEM;
> > goto out;
> > }
> > - ret = zram_decompress_page(zram, uncmem, index);
> > + zstrm = zcomp_strm_find(zram->comp);
> > + ret = zram_decompress_page(zram, zstrm, uncmem, index);
> > if (ret)
> > goto out;
> > }
> >
> > - zstrm = zcomp_strm_find(zram->comp);
> > + if (!zstrm)
> > + zstrm = zcomp_strm_find(zram->comp);
> > user_mem = kmap_atomic(page);
> >
> > if (is_partial_io(bvec)) {
> > @@ -721,7 +729,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> >
> > handle = zs_malloc(meta->mem_pool, clen);
> > if (!handle) {
> > - pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
> > + pr_info("Error allocating memory for compressed page: %u, size=%u\n",
> > index, clen);
> > ret = -ENOMEM;
> > goto out;
> > --
> > 1.9.1
> >
> --
> 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 Thu, Aug 13, 2015 at 02:38:23PM +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2015 at 03:37:55PM +0900, Joonsoo Kim wrote:
> >
> > Is there any way to access netlink interface and get the output from
> > kernel-side? I'd like to show information through
> > "/sys/block/zramX/comp_algorithm", because some user program can be
> > broken if we change output of userspace exposed interface.
>
> You could simply deprecate that interface but keep it for existing
> algorithms. Any new algorithms can then be queried through the
> crypto_user interface.
>
> The other option is to have a defined list of algorithms which is
> independent of the crypto API. For example, have a look at what
> IPsec does in net/xfrm/xfrm_algo.c. IPsec needs its own list
> because they come with annotations.
Hmm... they looks suboptimal.
How about introducing new functions to search supported algorithm in
kernel-side? As crypto API is used in more places, this interface
would be requested more. Defined list weaken the advantage of strong
point of generic crypto API.
Thanks.
On Thu, Aug 13, 2015 at 04:30:31PM +0900, Joonsoo Kim wrote:
>
> How about introducing new functions to search supported algorithm in
> kernel-side? As crypto API is used in more places, this interface
> would be requested more. Defined list weaken the advantage of strong
> point of generic crypto API.
You're only getting the list so you can immediately reexport
it to user-space, with no added value whatsoever. As I said
we already have an interface for that so you should use it.
If more genuine uses come up then I will reconsider.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Aug 13, 2015 at 03:32:33PM +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2015 at 04:19:41PM +0900, Joonsoo Kim wrote:
> >
> > If that optimization is really needed for the case that doesn't need
> > tfm except fetching function pointer, we can implement sharable tfm
> > in crypto subsystem.
>
> I'm happy to consider changes to the crypto compression interface
> as long as it continues to support the existing algorithms and users.
Happy to hear that.
I will think how it can be improved.
Thanks.
On Thu, Aug 13, 2015 at 03:29:18PM +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2015 at 04:30:31PM +0900, Joonsoo Kim wrote:
> >
> > How about introducing new functions to search supported algorithm in
> > kernel-side? As crypto API is used in more places, this interface
> > would be requested more. Defined list weaken the advantage of strong
> > point of generic crypto API.
>
> You're only getting the list so you can immediately reexport
> it to user-space, with no added value whatsoever. As I said
> we already have an interface for that so you should use it.
>
> If more genuine uses come up then I will reconsider.
Okay. I will think this issue more and come back with better
solution.
Thanks.
On Thu, Aug 13, 2015 at 04:19:41PM +0900, Joonsoo Kim wrote:
>
> If that optimization is really needed for the case that doesn't need
> tfm except fetching function pointer, we can implement sharable tfm
> in crypto subsystem.
I'm happy to consider changes to the crypto compression interface
as long as it continues to support the existing algorithms and users.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt