* [PATCHv8 1/6] zram: introduce compressing backend abstraction
2014-02-26 12:27 [PATCHv8 0/6] add compressing abstraction and multi stream support Sergey Senozhatsky
@ 2014-02-26 12:27 ` Sergey Senozhatsky
2014-02-27 23:18 ` Andrew Morton
2014-02-26 12:27 ` [PATCHv8 2/6] zram: use zcomp compressing backends Sergey Senozhatsky
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-26 12:27 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel,
Sergey Senozhatsky
ZRAM performs direct LZO compression algorithm calls, making it the one and
only option. Introduce compressing backend abstraction zcomp in order to
support multiple compression algorithms with the following set of operations:
.create
.destroy
.compress
.decompress
Schematically zram write() usually contains the following steps:
0) preparation (decompression of partioal IO, etc.)
1) lock buffer_lock mutex (protects meta compress buffers)
2) compress (using meta compress buffers)
3) alloc and map zs_pool object
4) copy compressed data (from meta compress buffers) to object allocated by 3)
5) free previous pool page, assign a new one
6) unlock buffer_lock mutex
As we can see, compressing buffers must remain untouched from 1) to 4),
because, otherwise, concurrent write() can overwrite data. At the same time,
zram_meta must be aware of a) specific compression algorithm memory requirements
and b) necessary locking to protect compression buffers. To remove requirement
a) new struct zcomp_strm introduced, which contains a compress/decompress
`buffer' and compression algorithm `private' part. While struct zcomp implements
zcomp_strm stream handling and locking by means of get() and put() semantics and
removes requirement b) from zram meta. zcomp ->create() and ->destroy(),
respectively, allocate and deallocate algorithm specific zcomp_strm `private'
part.
Every zcomp has zcomp stream and mutex to protect its compression stream. Stream
usage semantics remains the same -- only one write can hold stream lock and use
its buffers. zcomp_strm_get() turns caller into exclusive user of a stream
(holding stream mutex until zram put stream), and zcomp_strm_put() makes zcomp
stream available (unlock the stream mutex). Hence no concurrent write
(compression) operations possible at the moment.
iozone -t 3 -R -r 16K -s 60M -I +Z
test base patched
--------------------------------------------------
Initial write 597992.91 591660.58
Rewrite 609674.34 616054.97
Read 2404771.75 2452909.12
Re-read 2459216.81 2470074.44
Reverse Read 1652769.66 1589128.66
Stride read 2202441.81 2202173.31
Random read 2236311.47 2276565.31
Mixed workload 1423760.41 1709760.06
Random write 579584.08 615933.86
Pwrite 597550.02 594933.70
Pread 1703672.53 1718126.72
Fwrite 1330497.06 1461054.00
Fread 3922851.00 3957242.62
Usage examples:
comp = zcomp_create(NAME) /* NAME e.g. "lzo" */
which initialises compressing backend if requested algorithm is supported.
Compress:
zstrm = zcomp_strm_get(comp)
zcomp_compress(comp, zstrm, src, &dst_len)
[..] /* copy compressed data */
zcomp_strm_put(comp, zstrm)
Decompress:
zcomp_decompress(comp, src, src_len, dst);
Free compessing backend and its zcomp stream:
zcomp_destroy(comp)
Acked-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/block/zram/zcomp.c | 116 +++++++++++++++++++++++++++++++++++++++++
drivers/block/zram/zcomp.h | 53 +++++++++++++++++++
drivers/block/zram/zcomp_lzo.c | 48 +++++++++++++++++
3 files changed, 217 insertions(+)
create mode 100644 drivers/block/zram/zcomp.c
create mode 100644 drivers/block/zram/zcomp.h
create mode 100644 drivers/block/zram/zcomp_lzo.c
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
new file mode 100644
index 0000000..f67c38a
--- /dev/null
+++ b/drivers/block/zram/zcomp.c
@@ -0,0 +1,116 @@
+/*
+ * 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/string.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+#include <linux/sched.h>
+
+#include "zcomp.h"
+
+extern struct zcomp_backend zcomp_lzo;
+
+static struct zcomp_backend *find_backend(const char *compress)
+{
+ if (strncmp(compress, "lzo", 3) == 0)
+ return &zcomp_lzo;
+ return NULL;
+}
+
+static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ if (zstrm->private)
+ comp->backend->destroy(zstrm->private);
+ free_pages((unsigned long)zstrm->buffer, 1);
+ kfree(zstrm);
+}
+
+/*
+ * allocate new zcomp_strm structure with ->private initialized by
+ * backend, return NULL on error
+ */
+static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
+{
+ struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), GFP_KERNEL);
+ if (!zstrm)
+ return NULL;
+
+ zstrm->private = comp->backend->create();
+ /*
+ * 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) {
+ zcomp_strm_free(comp, zstrm);
+ zstrm = NULL;
+ }
+ return zstrm;
+}
+
+struct zcomp_strm *zcomp_strm_get(struct zcomp *comp)
+{
+ mutex_lock(&comp->strm_lock);
+ return comp->zstrm;
+}
+
+void zcomp_strm_put(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ mutex_unlock(&comp->strm_lock);
+}
+
+int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src, size_t *dst_len)
+{
+ return comp->backend->compress(src, zstrm->buffer, dst_len,
+ zstrm->private);
+}
+
+int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
+ size_t src_len, unsigned char *dst)
+{
+ return comp->backend->decompress(src, src_len, dst);
+}
+
+void zcomp_destroy(struct zcomp *comp)
+{
+ zcomp_strm_free(comp, comp->zstrm);
+ kfree(comp);
+}
+
+/*
+ * search available compressors for requested algorithm.
+ * allocate new zcomp and initialize it. return NULL
+ * if requested algorithm is not supported or in case
+ * of init error
+ */
+struct zcomp *zcomp_create(const char *compress)
+{
+ struct zcomp *comp;
+ struct zcomp_backend *backend;
+
+ backend = find_backend(compress);
+ if (!backend)
+ return NULL;
+
+ comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
+ if (!comp)
+ return NULL;
+
+ comp->backend = backend;
+ mutex_init(&comp->strm_lock);
+
+ comp->zstrm = zcomp_strm_alloc(comp);
+ if (!comp->zstrm) {
+ kfree(comp);
+ return NULL;
+ }
+ return comp;
+}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
new file mode 100644
index 0000000..5106f8e
--- /dev/null
+++ b/drivers/block/zram/zcomp.h
@@ -0,0 +1,53 @@
+/*
+ * 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_H_
+#define _ZCOMP_H_
+
+#include <linux/mutex.h>
+
+struct zcomp_strm {
+ void *buffer; /* compression/decompression buffer */
+ void *private;
+ 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 {
+ struct mutex strm_lock;
+ struct zcomp_strm *zstrm;
+ struct zcomp_backend *backend;
+};
+
+struct zcomp *zcomp_create(const char *comp);
+void zcomp_destroy(struct zcomp *comp);
+
+struct zcomp_strm *zcomp_strm_get(struct zcomp *comp);
+void zcomp_strm_put(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,
+ size_t src_len, unsigned char *dst);
+#endif /* _ZCOMP_H_ */
diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
new file mode 100644
index 0000000..b7722a2
--- /dev/null
+++ b/drivers/block/zram/zcomp_lzo.c
@@ -0,0 +1,48 @@
+/*
+ * 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.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;
+}
+
+extern struct zcomp_backend zcomp_lzo;
+struct zcomp_backend zcomp_lzo = {
+ .compress = lzo_compress,
+ .decompress = lzo_decompress,
+ .create = lzo_create,
+ .destroy = lzo_destroy,
+ .name = "lzo",
+};
--
1.9.0.291.g027825b
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv8 1/6] zram: introduce compressing backend abstraction
2014-02-26 12:27 ` [PATCHv8 1/6] zram: introduce compressing backend abstraction Sergey Senozhatsky
@ 2014-02-27 23:18 ` Andrew Morton
2014-02-28 17:40 ` Sergey Senozhatsky
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2014-02-27 23:18 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel
On Wed, 26 Feb 2014 15:27:54 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> ZRAM performs direct LZO compression algorithm calls, making it the one and
> only option. Introduce compressing backend abstraction zcomp in order to
> support multiple compression algorithms with the following set of operations:
Well... why? Presumably because other compression schemes have
compelling advantages over LZO, otherwise there's no point in the
patch!
Please describe this in some detail, so that others can judge the
desirability of the patch.
> .create
> .destroy
> .compress
> .decompress
>
> Schematically zram write() usually contains the following steps:
> 0) preparation (decompression of partioal IO, etc.)
> 1) lock buffer_lock mutex (protects meta compress buffers)
> 2) compress (using meta compress buffers)
> 3) alloc and map zs_pool object
> 4) copy compressed data (from meta compress buffers) to object allocated by 3)
> 5) free previous pool page, assign a new one
> 6) unlock buffer_lock mutex
>
> As we can see, compressing buffers must remain untouched from 1) to 4),
> because, otherwise, concurrent write() can overwrite data. At the same time,
> zram_meta must be aware of a) specific compression algorithm memory requirements
> and b) necessary locking to protect compression buffers. To remove requirement
> a) new struct zcomp_strm introduced, which contains a compress/decompress
> `buffer' and compression algorithm `private' part. While struct zcomp implements
> zcomp_strm stream handling and locking by means of get() and put() semantics and
> removes requirement b) from zram meta. zcomp ->create() and ->destroy(),
> respectively, allocate and deallocate algorithm specific zcomp_strm `private'
> part.
>
> Every zcomp has zcomp stream and mutex to protect its compression stream. Stream
> usage semantics remains the same -- only one write can hold stream lock and use
> its buffers. zcomp_strm_get() turns caller into exclusive user of a stream
> (holding stream mutex until zram put stream), and zcomp_strm_put() makes zcomp
> stream available (unlock the stream mutex). Hence no concurrent write
> (compression) operations possible at the moment.
>
> iozone -t 3 -R -r 16K -s 60M -I +Z
>
> test base patched
> --------------------------------------------------
> Initial write 597992.91 591660.58
> Rewrite 609674.34 616054.97
> Read 2404771.75 2452909.12
> Re-read 2459216.81 2470074.44
> Reverse Read 1652769.66 1589128.66
> Stride read 2202441.81 2202173.31
> Random read 2236311.47 2276565.31
> Mixed workload 1423760.41 1709760.06
> Random write 579584.08 615933.86
> Pwrite 597550.02 594933.70
> Pread 1703672.53 1718126.72
> Fwrite 1330497.06 1461054.00
> Fread 3922851.00 3957242.62
>
> Usage examples:
>
> comp = zcomp_create(NAME) /* NAME e.g. "lzo" */
>
> which initialises compressing backend if requested algorithm is supported.
>
> Compress:
> zstrm = zcomp_strm_get(comp)
> zcomp_compress(comp, zstrm, src, &dst_len)
> [..] /* copy compressed data */
> zcomp_strm_put(comp, zstrm)
>
> Decompress:
> zcomp_decompress(comp, src, src_len, dst);
>
> Free compessing backend and its zcomp stream:
> zcomp_destroy(comp)
>
> ...
>
> --- /dev/null
> +++ b/drivers/block/zram/zcomp.c
> @@ -0,0 +1,116 @@
> +/*
> + * 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/string.h>
> +#include <linux/slab.h>
> +#include <linux/wait.h>
> +#include <linux/sched.h>
> +
> +#include "zcomp.h"
> +
> +extern struct zcomp_backend zcomp_lzo;
This should be in a header file please. So we know that the definition
site and all users agree on the type.
> +static struct zcomp_backend *find_backend(const char *compress)
> +{
> + if (strncmp(compress, "lzo", 3) == 0)
> + return &zcomp_lzo;
> + return NULL;
> +}
> +
> +static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> +{
> + if (zstrm->private)
> + comp->backend->destroy(zstrm->private);
> + free_pages((unsigned long)zstrm->buffer, 1);
> + kfree(zstrm);
> +}
> +
> +/*
> + * allocate new zcomp_strm structure with ->private initialized by
> + * backend, return NULL on error
> + */
> +static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> +{
> + struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), GFP_KERNEL);
> + if (!zstrm)
> + return NULL;
> +
> + zstrm->private = comp->backend->create();
> + /*
> + * 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) {
> + zcomp_strm_free(comp, zstrm);
> + zstrm = NULL;
> + }
> + return zstrm;
> +}
> +
> +struct zcomp_strm *zcomp_strm_get(struct zcomp *comp)
> +{
> + mutex_lock(&comp->strm_lock);
> + return comp->zstrm;
> +}
> +
> +void zcomp_strm_put(struct zcomp *comp, struct zcomp_strm *zstrm)
> +{
> + mutex_unlock(&comp->strm_lock);
> +}
These are poorly named. When we see "get" and "put" we expect that
they will increment-refcount and decrement-refcount-free-on-zero. But
these function do not implement such a pattern.
> +int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> + const unsigned char *src, size_t *dst_len)
> +{
> + return comp->backend->compress(src, zstrm->buffer, dst_len,
> + zstrm->private);
> +}
> +
> +int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> + size_t src_len, unsigned char *dst)
> +{
> + return comp->backend->decompress(src, src_len, dst);
> +}
> +
> +void zcomp_destroy(struct zcomp *comp)
> +{
> + zcomp_strm_free(comp, comp->zstrm);
> + kfree(comp);
> +}
> +
> +/*
> + * search available compressors for requested algorithm.
> + * allocate new zcomp and initialize it. return NULL
> + * if requested algorithm is not supported or in case
> + * of init error
> + */
> +struct zcomp *zcomp_create(const char *compress)
> +{
> + struct zcomp *comp;
> + struct zcomp_backend *backend;
> +
> + backend = find_backend(compress);
> + if (!backend)
> + return NULL;
> +
> + comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
> + if (!comp)
> + return NULL;
> +
> + comp->backend = backend;
> + mutex_init(&comp->strm_lock);
> +
> + comp->zstrm = zcomp_strm_alloc(comp);
> + if (!comp->zstrm) {
> + kfree(comp);
> + return NULL;
> + }
> + return comp;
> +}
> diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> new file mode 100644
> index 0000000..5106f8e
> --- /dev/null
> +++ b/drivers/block/zram/zcomp.h
> @@ -0,0 +1,53 @@
> +/*
> + * 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_H_
> +#define _ZCOMP_H_
> +
> +#include <linux/mutex.h>
> +
> +struct zcomp_strm {
> + void *buffer; /* compression/decompression buffer */
> + void *private;
Document this?
> + struct list_head list;
What does this do and what lock protects it?
> +};
> +
> +/* 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 {
> + struct mutex strm_lock;
> + struct zcomp_strm *zstrm;
> + struct zcomp_backend *backend;
> +};
> +
> +struct zcomp *zcomp_create(const char *comp);
> +void zcomp_destroy(struct zcomp *comp);
> +
> +struct zcomp_strm *zcomp_strm_get(struct zcomp *comp);
> +void zcomp_strm_put(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,
> + size_t src_len, unsigned char *dst);
> +#endif /* _ZCOMP_H_ */
> diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
> new file mode 100644
> index 0000000..b7722a2
> --- /dev/null
> +++ b/drivers/block/zram/zcomp_lzo.c
> @@ -0,0 +1,48 @@
> +/*
> + * 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.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;
> +}
> +
> +extern struct zcomp_backend zcomp_lzo;
This is unneeded, should be in the header file.
> +struct zcomp_backend zcomp_lzo = {
> + .compress = lzo_compress,
> + .decompress = lzo_decompress,
> + .create = lzo_create,
> + .destroy = lzo_destroy,
> + .name = "lzo",
> +};
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCHv8 1/6] zram: introduce compressing backend abstraction
2014-02-27 23:18 ` Andrew Morton
@ 2014-02-28 17:40 ` Sergey Senozhatsky
0 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-28 17:40 UTC (permalink / raw)
To: Andrew Morton
Cc: Sergey Senozhatsky, Minchan Kim, Jerome Marchand, Nitin Gupta,
linux-kernel
Hello Andrew,
On (02/27/14 15:18), Andrew Morton wrote:
> On Wed, 26 Feb 2014 15:27:54 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
>
> > ZRAM performs direct LZO compression algorithm calls, making it the one and
> > only option. Introduce compressing backend abstraction zcomp in order to
> > support multiple compression algorithms with the following set of operations:
>
> Well... why? Presumably because other compression schemes have
> compelling advantages over LZO, otherwise there's no point in the
> patch!
>
> Please describe this in some detail, so that others can judge the
> desirability of the patch.
>
thanks for review.
all of your notes and concerns were addressed in v9 (will send shortly).
-ss
> > .create
> > .destroy
> > .compress
> > .decompress
> >
> > Schematically zram write() usually contains the following steps:
> > 0) preparation (decompression of partioal IO, etc.)
> > 1) lock buffer_lock mutex (protects meta compress buffers)
> > 2) compress (using meta compress buffers)
> > 3) alloc and map zs_pool object
> > 4) copy compressed data (from meta compress buffers) to object allocated by 3)
> > 5) free previous pool page, assign a new one
> > 6) unlock buffer_lock mutex
> >
> > As we can see, compressing buffers must remain untouched from 1) to 4),
> > because, otherwise, concurrent write() can overwrite data. At the same time,
> > zram_meta must be aware of a) specific compression algorithm memory requirements
> > and b) necessary locking to protect compression buffers. To remove requirement
> > a) new struct zcomp_strm introduced, which contains a compress/decompress
> > `buffer' and compression algorithm `private' part. While struct zcomp implements
> > zcomp_strm stream handling and locking by means of get() and put() semantics and
> > removes requirement b) from zram meta. zcomp ->create() and ->destroy(),
> > respectively, allocate and deallocate algorithm specific zcomp_strm `private'
> > part.
> >
> > Every zcomp has zcomp stream and mutex to protect its compression stream. Stream
> > usage semantics remains the same -- only one write can hold stream lock and use
> > its buffers. zcomp_strm_get() turns caller into exclusive user of a stream
> > (holding stream mutex until zram put stream), and zcomp_strm_put() makes zcomp
> > stream available (unlock the stream mutex). Hence no concurrent write
> > (compression) operations possible at the moment.
> >
> > iozone -t 3 -R -r 16K -s 60M -I +Z
> >
> > test base patched
> > --------------------------------------------------
> > Initial write 597992.91 591660.58
> > Rewrite 609674.34 616054.97
> > Read 2404771.75 2452909.12
> > Re-read 2459216.81 2470074.44
> > Reverse Read 1652769.66 1589128.66
> > Stride read 2202441.81 2202173.31
> > Random read 2236311.47 2276565.31
> > Mixed workload 1423760.41 1709760.06
> > Random write 579584.08 615933.86
> > Pwrite 597550.02 594933.70
> > Pread 1703672.53 1718126.72
> > Fwrite 1330497.06 1461054.00
> > Fread 3922851.00 3957242.62
> >
> > Usage examples:
> >
> > comp = zcomp_create(NAME) /* NAME e.g. "lzo" */
> >
> > which initialises compressing backend if requested algorithm is supported.
> >
> > Compress:
> > zstrm = zcomp_strm_get(comp)
> > zcomp_compress(comp, zstrm, src, &dst_len)
> > [..] /* copy compressed data */
> > zcomp_strm_put(comp, zstrm)
> >
> > Decompress:
> > zcomp_decompress(comp, src, src_len, dst);
> >
> > Free compessing backend and its zcomp stream:
> > zcomp_destroy(comp)
> >
> > ...
> >
> > --- /dev/null
> > +++ b/drivers/block/zram/zcomp.c
> > @@ -0,0 +1,116 @@
> > +/*
> > + * 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/string.h>
> > +#include <linux/slab.h>
> > +#include <linux/wait.h>
> > +#include <linux/sched.h>
> > +
> > +#include "zcomp.h"
> > +
> > +extern struct zcomp_backend zcomp_lzo;
>
> This should be in a header file please. So we know that the definition
> site and all users agree on the type.
>
> > +static struct zcomp_backend *find_backend(const char *compress)
> > +{
> > + if (strncmp(compress, "lzo", 3) == 0)
> > + return &zcomp_lzo;
> > + return NULL;
> > +}
> > +
> > +static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> > +{
> > + if (zstrm->private)
> > + comp->backend->destroy(zstrm->private);
> > + free_pages((unsigned long)zstrm->buffer, 1);
> > + kfree(zstrm);
> > +}
> > +
> > +/*
> > + * allocate new zcomp_strm structure with ->private initialized by
> > + * backend, return NULL on error
> > + */
> > +static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
> > +{
> > + struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), GFP_KERNEL);
> > + if (!zstrm)
> > + return NULL;
> > +
> > + zstrm->private = comp->backend->create();
> > + /*
> > + * 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) {
> > + zcomp_strm_free(comp, zstrm);
> > + zstrm = NULL;
> > + }
> > + return zstrm;
> > +}
> > +
> > +struct zcomp_strm *zcomp_strm_get(struct zcomp *comp)
> > +{
> > + mutex_lock(&comp->strm_lock);
> > + return comp->zstrm;
> > +}
> > +
> > +void zcomp_strm_put(struct zcomp *comp, struct zcomp_strm *zstrm)
> > +{
> > + mutex_unlock(&comp->strm_lock);
> > +}
>
> These are poorly named. When we see "get" and "put" we expect that
> they will increment-refcount and decrement-refcount-free-on-zero. But
> these function do not implement such a pattern.
>
> > +int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > + const unsigned char *src, size_t *dst_len)
> > +{
> > + return comp->backend->compress(src, zstrm->buffer, dst_len,
> > + zstrm->private);
> > +}
> > +
> > +int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
> > + size_t src_len, unsigned char *dst)
> > +{
> > + return comp->backend->decompress(src, src_len, dst);
> > +}
> > +
> > +void zcomp_destroy(struct zcomp *comp)
> > +{
> > + zcomp_strm_free(comp, comp->zstrm);
> > + kfree(comp);
> > +}
> > +
> > +/*
> > + * search available compressors for requested algorithm.
> > + * allocate new zcomp and initialize it. return NULL
> > + * if requested algorithm is not supported or in case
> > + * of init error
> > + */
> > +struct zcomp *zcomp_create(const char *compress)
> > +{
> > + struct zcomp *comp;
> > + struct zcomp_backend *backend;
> > +
> > + backend = find_backend(compress);
> > + if (!backend)
> > + return NULL;
> > +
> > + comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL);
> > + if (!comp)
> > + return NULL;
> > +
> > + comp->backend = backend;
> > + mutex_init(&comp->strm_lock);
> > +
> > + comp->zstrm = zcomp_strm_alloc(comp);
> > + if (!comp->zstrm) {
> > + kfree(comp);
> > + return NULL;
> > + }
> > + return comp;
> > +}
> > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> > new file mode 100644
> > index 0000000..5106f8e
> > --- /dev/null
> > +++ b/drivers/block/zram/zcomp.h
> > @@ -0,0 +1,53 @@
> > +/*
> > + * 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_H_
> > +#define _ZCOMP_H_
> > +
> > +#include <linux/mutex.h>
> > +
> > +struct zcomp_strm {
> > + void *buffer; /* compression/decompression buffer */
> > + void *private;
>
> Document this?
>
> > + struct list_head list;
>
> What does this do and what lock protects it?
>
> > +};
> > +
> > +/* 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 {
> > + struct mutex strm_lock;
> > + struct zcomp_strm *zstrm;
> > + struct zcomp_backend *backend;
> > +};
> > +
> > +struct zcomp *zcomp_create(const char *comp);
> > +void zcomp_destroy(struct zcomp *comp);
> > +
> > +struct zcomp_strm *zcomp_strm_get(struct zcomp *comp);
> > +void zcomp_strm_put(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,
> > + size_t src_len, unsigned char *dst);
> > +#endif /* _ZCOMP_H_ */
> > diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
> > new file mode 100644
> > index 0000000..b7722a2
> > --- /dev/null
> > +++ b/drivers/block/zram/zcomp_lzo.c
> > @@ -0,0 +1,48 @@
> > +/*
> > + * 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.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;
> > +}
> > +
> > +extern struct zcomp_backend zcomp_lzo;
>
> This is unneeded, should be in the header file.
>
> > +struct zcomp_backend zcomp_lzo = {
> > + .compress = lzo_compress,
> > + .decompress = lzo_decompress,
> > + .create = lzo_create,
> > + .destroy = lzo_destroy,
> > + .name = "lzo",
> > +};
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv8 2/6] zram: use zcomp compressing backends
2014-02-26 12:27 [PATCHv8 0/6] add compressing abstraction and multi stream support Sergey Senozhatsky
2014-02-26 12:27 ` [PATCHv8 1/6] zram: introduce compressing backend abstraction Sergey Senozhatsky
@ 2014-02-26 12:27 ` Sergey Senozhatsky
2014-02-26 12:27 ` [PATCHv8 3/6] zram: factor out single stream compression Sergey Senozhatsky
` (4 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-26 12:27 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel,
Sergey Senozhatsky
Do not perform direct LZO compress/decompress calls, initialise
and use zcomp LZO backend (single compression stream) instead.
Acked-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/block/zram/Makefile | 2 +-
drivers/block/zram/zram_drv.c | 59 ++++++++++++++++++-------------------------
drivers/block/zram/zram_drv.h | 8 +++---
3 files changed, 29 insertions(+), 40 deletions(-)
diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
index cb0f9ce..757c6a5 100644
--- a/drivers/block/zram/Makefile
+++ b/drivers/block/zram/Makefile
@@ -1,3 +1,3 @@
-zram-y := zram_drv.o
+zram-y := zcomp_lzo.o zcomp.o zram_drv.o
obj-$(CONFIG_ZRAM) += zram.o
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 9baac5b..569ff58 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -29,7 +29,6 @@
#include <linux/genhd.h>
#include <linux/highmem.h>
#include <linux/slab.h>
-#include <linux/lzo.h>
#include <linux/string.h>
#include <linux/vmalloc.h>
@@ -38,6 +37,7 @@
/* Globals */
static int zram_major;
static struct zram *zram_devices;
+static const char *default_compressor = "lzo";
/* Module params (documentation at end) */
static unsigned int num_devices = 1;
@@ -160,8 +160,6 @@ static inline int valid_io_request(struct zram *zram, struct bio *bio)
static void zram_meta_free(struct zram_meta *meta)
{
zs_destroy_pool(meta->mem_pool);
- kfree(meta->compress_workmem);
- free_pages((unsigned long)meta->compress_buffer, 1);
vfree(meta->table);
kfree(meta);
}
@@ -173,22 +171,11 @@ static struct zram_meta *zram_meta_alloc(u64 disksize)
if (!meta)
goto out;
- meta->compress_workmem = kzalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
- if (!meta->compress_workmem)
- goto free_meta;
-
- meta->compress_buffer =
- (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1);
- if (!meta->compress_buffer) {
- pr_err("Error allocating compressor buffer space\n");
- goto free_workmem;
- }
-
num_pages = disksize >> PAGE_SHIFT;
meta->table = vzalloc(num_pages * sizeof(*meta->table));
if (!meta->table) {
pr_err("Error allocating zram address table\n");
- goto free_buffer;
+ goto free_meta;
}
meta->mem_pool = zs_create_pool(GFP_NOIO | __GFP_HIGHMEM);
@@ -198,15 +185,10 @@ static struct zram_meta *zram_meta_alloc(u64 disksize)
}
rwlock_init(&meta->tb_lock);
- mutex_init(&meta->buffer_lock);
return meta;
free_table:
vfree(meta->table);
-free_buffer:
- free_pages((unsigned long)meta->compress_buffer, 1);
-free_workmem:
- kfree(meta->compress_workmem);
free_meta:
kfree(meta);
meta = NULL;
@@ -280,8 +262,7 @@ static void zram_free_page(struct zram *zram, size_t index)
static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
{
- int ret = LZO_E_OK;
- size_t clen = PAGE_SIZE;
+ int ret = 0;
unsigned char *cmem;
struct zram_meta *meta = zram->meta;
unsigned long handle;
@@ -301,12 +282,12 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
if (size == PAGE_SIZE)
copy_page(mem, cmem);
else
- ret = lzo1x_decompress_safe(cmem, size, mem, &clen);
+ ret = zcomp_decompress(zram->comp, cmem, size, mem);
zs_unmap_object(meta->mem_pool, handle);
read_unlock(&meta->tb_lock);
/* Should NEVER happen. Return bio error if it does. */
- if (unlikely(ret != LZO_E_OK)) {
+ if (unlikely(ret)) {
pr_err("Decompression failed! err=%d, page=%u\n", ret, index);
atomic64_inc(&zram->stats.failed_reads);
return ret;
@@ -349,7 +330,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
ret = zram_decompress_page(zram, uncmem, index);
/* Should NEVER happen. Return bio error if it does. */
- if (unlikely(ret != LZO_E_OK))
+ if (unlikely(ret))
goto out_cleanup;
if (is_partial_io(bvec))
@@ -374,11 +355,10 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
struct page *page;
unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
struct zram_meta *meta = zram->meta;
+ struct zcomp_strm *zstrm;
bool locked = false;
page = bvec->bv_page;
- src = meta->compress_buffer;
-
if (is_partial_io(bvec)) {
/*
* This is a partial IO. We need to read the full page
@@ -394,7 +374,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
goto out;
}
- mutex_lock(&meta->buffer_lock);
+ zstrm = zcomp_strm_get(zram->comp);
locked = true;
user_mem = kmap_atomic(page);
@@ -420,22 +400,20 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
goto out;
}
- ret = lzo1x_1_compress(uncmem, PAGE_SIZE, src, &clen,
- meta->compress_workmem);
+ ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen);
if (!is_partial_io(bvec)) {
kunmap_atomic(user_mem);
user_mem = NULL;
uncmem = NULL;
}
- if (unlikely(ret != LZO_E_OK)) {
+ if (unlikely(ret)) {
pr_err("Compression failed! err=%d\n", ret);
goto out;
}
-
+ src = zstrm->buffer;
if (unlikely(clen > max_zpage_size)) {
clen = PAGE_SIZE;
- src = NULL;
if (is_partial_io(bvec))
src = uncmem;
}
@@ -457,6 +435,8 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
memcpy(cmem, src, clen);
}
+ zcomp_strm_put(zram->comp, zstrm);
+ locked = false;
zs_unmap_object(meta->mem_pool, handle);
/*
@@ -475,10 +455,9 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
atomic64_inc(&zram->stats.pages_stored);
out:
if (locked)
- mutex_unlock(&meta->buffer_lock);
+ zcomp_strm_put(zram->comp, zstrm);
if (is_partial_io(bvec))
kfree(uncmem);
-
if (ret)
atomic64_inc(&zram->stats.failed_writes);
return ret;
@@ -522,6 +501,7 @@ static void zram_reset_device(struct zram *zram, bool reset_capacity)
zs_free(meta->mem_pool, handle);
}
+ zcomp_destroy(zram->comp);
zram_meta_free(zram->meta);
zram->meta = NULL;
/* Reset stats */
@@ -550,9 +530,18 @@ static ssize_t disksize_store(struct device *dev,
return -EBUSY;
}
+ zram->comp = zcomp_create(default_compressor);
+ if (!zram->comp) {
+ up_write(&zram->init_lock);
+ pr_info("Cannot initialise %s compressing backend\n",
+ default_compressor);
+ return -EINVAL;
+ }
+
disksize = PAGE_ALIGN(disksize);
zram->meta = zram_meta_alloc(disksize);
if (!zram->meta) {
+ zcomp_destroy(zram->comp);
up_write(&zram->init_lock);
return -ENOMEM;
}
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 1d5b1f5..45e04f7 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -16,9 +16,10 @@
#define _ZRAM_DRV_H_
#include <linux/spinlock.h>
-#include <linux/mutex.h>
#include <linux/zsmalloc.h>
+#include "zcomp.h"
+
/*
* Some arbitrary value. This is just to catch
* invalid value for num_devices module parameter.
@@ -81,17 +82,16 @@ struct zram_stats {
struct zram_meta {
rwlock_t tb_lock; /* protect table */
- void *compress_workmem;
- void *compress_buffer;
struct table *table;
struct zs_pool *mem_pool;
- struct mutex buffer_lock; /* protect compress buffers */
};
struct zram {
struct zram_meta *meta;
struct request_queue *queue;
struct gendisk *disk;
+ struct zcomp *comp;
+
/* Prevent concurrent execution of device init, reset and R/W request */
struct rw_semaphore init_lock;
/*
--
1.9.0.291.g027825b
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCHv8 3/6] zram: factor out single stream compression
2014-02-26 12:27 [PATCHv8 0/6] add compressing abstraction and multi stream support Sergey Senozhatsky
2014-02-26 12:27 ` [PATCHv8 1/6] zram: introduce compressing backend abstraction Sergey Senozhatsky
2014-02-26 12:27 ` [PATCHv8 2/6] zram: use zcomp compressing backends Sergey Senozhatsky
@ 2014-02-26 12:27 ` Sergey Senozhatsky
2014-02-27 23:20 ` Andrew Morton
2014-02-26 12:27 ` [PATCHv8 4/6] zram: add multi stream functionality Sergey Senozhatsky
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-26 12:27 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel,
Sergey Senozhatsky
This is preparation patch to add multi stream support to zcomp.
Introduce struct zcomp_strm_single and a set of functions to manage zcomp_strm
stream access. zcomp_strm_single implements single compession stream, same way
as current zcomp implementation. This moves zcomp_strm stream control and
locking from zcomp, so compressing backend zcomp is not aware of required
locking.
Single and multi streams require different locking schemes. Minchan Kim
reported that spinlock-based locking scheme (which is used in multi stream
implementation) has demonstrated a severe perfomance regression for single
compression stream case, comparing to mutex-based.
see https://lkml.org/lkml/2014/2/18/16
The following set of functions added:
- zcomp_strm_single_get()/zcomp_strm_single_put()
get and put compression stream, implement required locking
- zcomp_strm_single_create()/zcomp_strm_single_destroy()
create and destroy zcomp_strm_single
New ->strm_get() and ->strm_put() callbacks added to zcomp, which are set to
zcomp_strm_single_get() and zcomp_strm_single_put() during initialisation.
Instead of direct locking and zcomp_strm access from zcomp_strm_get() and
zcomp_strm_put(), zcomp now calls ->strm_get() and ->strm_put()
correspondingly.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/block/zram/zcomp.c | 61 ++++++++++++++++++++++++++++++++++++++++------
drivers/block/zram/zcomp.h | 7 ++++--
2 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index f67c38a..210cc28 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,6 +15,14 @@
#include "zcomp.h"
+/*
+ * single zcomp_strm backend
+ */
+struct zcomp_strm_single {
+ struct mutex strm_lock;
+ struct zcomp_strm *zstrm;
+};
+
extern struct zcomp_backend zcomp_lzo;
static struct zcomp_backend *find_backend(const char *compress)
@@ -55,15 +63,55 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
return zstrm;
}
+static struct zcomp_strm *zcomp_strm_single_get(struct zcomp *comp)
+{
+ struct zcomp_strm_single *zs = comp->stream;
+ mutex_lock(&zs->strm_lock);
+ return zs->zstrm;
+}
+
+static void zcomp_strm_single_put(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ struct zcomp_strm_single *zs = comp->stream;
+ mutex_unlock(&zs->strm_lock);
+}
+
+static void zcomp_strm_single_destroy(struct zcomp *comp)
+{
+ struct zcomp_strm_single *zs = comp->stream;
+ zcomp_strm_free(comp, zs->zstrm);
+ kfree(zs);
+}
+
+static int zcomp_strm_single_create(struct zcomp *comp)
+{
+ struct zcomp_strm_single *zs;
+
+ comp->destroy = zcomp_strm_single_destroy;
+ comp->strm_get = zcomp_strm_single_get;
+ comp->strm_put = zcomp_strm_single_put;
+ zs = kmalloc(sizeof(struct zcomp_strm_single), GFP_KERNEL);
+ if (!zs)
+ return -ENOMEM;
+
+ comp->stream = zs;
+ mutex_init(&zs->strm_lock);
+ zs->zstrm = zcomp_strm_alloc(comp);
+ if (!zs->zstrm) {
+ kfree(zs);
+ return -ENOMEM;
+ }
+ return 0;
+}
+
struct zcomp_strm *zcomp_strm_get(struct zcomp *comp)
{
- mutex_lock(&comp->strm_lock);
- return comp->zstrm;
+ return comp->strm_get(comp);
}
void zcomp_strm_put(struct zcomp *comp, struct zcomp_strm *zstrm)
{
- mutex_unlock(&comp->strm_lock);
+ comp->strm_put(comp, zstrm);
}
int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
@@ -81,7 +129,7 @@ int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
void zcomp_destroy(struct zcomp *comp)
{
- zcomp_strm_free(comp, comp->zstrm);
+ comp->destroy(comp);
kfree(comp);
}
@@ -105,10 +153,7 @@ struct zcomp *zcomp_create(const char *compress)
return NULL;
comp->backend = backend;
- mutex_init(&comp->strm_lock);
-
- comp->zstrm = zcomp_strm_alloc(comp);
- if (!comp->zstrm) {
+ if (zcomp_strm_single_create(comp) != 0) {
kfree(comp);
return NULL;
}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 5106f8e..861e04d 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -34,9 +34,12 @@ struct zcomp_backend {
/* dynamic per-device compression frontend */
struct zcomp {
- struct mutex strm_lock;
- struct zcomp_strm *zstrm;
+ void *stream;
struct zcomp_backend *backend;
+
+ struct zcomp_strm *(*strm_get)(struct zcomp *comp);
+ void (*strm_put)(struct zcomp *comp, struct zcomp_strm *zstrm);
+ void (*destroy)(struct zcomp *comp);
};
struct zcomp *zcomp_create(const char *comp);
--
1.9.0.291.g027825b
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv8 3/6] zram: factor out single stream compression
2014-02-26 12:27 ` [PATCHv8 3/6] zram: factor out single stream compression Sergey Senozhatsky
@ 2014-02-27 23:20 ` Andrew Morton
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2014-02-27 23:20 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel
On Wed, 26 Feb 2014 15:27:56 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> This is preparation patch to add multi stream support to zcomp.
>
> Introduce struct zcomp_strm_single and a set of functions to manage zcomp_strm
> stream access. zcomp_strm_single implements single compession stream, same way
> as current zcomp implementation. This moves zcomp_strm stream control and
> locking from zcomp, so compressing backend zcomp is not aware of required
> locking.
>
> Single and multi streams require different locking schemes. Minchan Kim
> reported that spinlock-based locking scheme (which is used in multi stream
> implementation) has demonstrated a severe perfomance regression for single
> compression stream case, comparing to mutex-based.
> see https://lkml.org/lkml/2014/2/18/16
>
> The following set of functions added:
> - zcomp_strm_single_get()/zcomp_strm_single_put()
> get and put compression stream, implement required locking
> - zcomp_strm_single_create()/zcomp_strm_single_destroy()
> create and destroy zcomp_strm_single
>
> New ->strm_get() and ->strm_put() callbacks added to zcomp, which are set to
> zcomp_strm_single_get() and zcomp_strm_single_put() during initialisation.
> Instead of direct locking and zcomp_strm access from zcomp_strm_get() and
> zcomp_strm_put(), zcomp now calls ->strm_get() and ->strm_put()
> correspondingly.
>
> ...
>
> +static struct zcomp_strm *zcomp_strm_single_get(struct zcomp *comp)
> +{
> + struct zcomp_strm_single *zs = comp->stream;
> + mutex_lock(&zs->strm_lock);
> + return zs->zstrm;
> +}
> +
> +static void zcomp_strm_single_put(struct zcomp *comp, struct zcomp_strm *zstrm)
> +{
> + struct zcomp_strm_single *zs = comp->stream;
> + mutex_unlock(&zs->strm_lock);
> +}
Again, these are not "get" and "put" operations.
>
> ...
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv8 4/6] zram: add multi stream functionality
2014-02-26 12:27 [PATCHv8 0/6] add compressing abstraction and multi stream support Sergey Senozhatsky
` (2 preceding siblings ...)
2014-02-26 12:27 ` [PATCHv8 3/6] zram: factor out single stream compression Sergey Senozhatsky
@ 2014-02-26 12:27 ` Sergey Senozhatsky
2014-02-27 23:22 ` Andrew Morton
2014-02-27 23:25 ` Andrew Morton
2014-02-26 12:27 ` [PATCHv8 5/6] zram: add set_max_streams knob Sergey Senozhatsky
` (2 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-26 12:27 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel,
Sergey Senozhatsky
This patch implements multi stream compression support.
Introduce struct zcomp_strm_multi and a set of functions to manage
zcomp_strm stream access. zcomp_strm_multi has a list of idle zcomp_strm
structs, spinlock to protect idle list and wait queue, making it possible
to perform parallel compressions.
The following set of functions added:
- zcomp_strm_multi_get()/zcomp_strm_multi_put()
get and put compression stream, implement required locking
- zcomp_strm_multi_create()/zcomp_strm_multi_destroy()
create and destroy zcomp_strm_multi
zcomp ->strm_get() and ->strm_put() callbacks are set during initialisation
to zcomp_strm_multi_get()/zcomp_strm_multi_put() correspondingly.
Each time zcomp issues a zcomp_strm_multi_get() call, the following set of
operations performed:
- spin lock strm_lock
- if idle list is not empty, remove zcomp_strm from idle list, spin
unlock and return zcomp stream pointer to caller
- if idle list is empty, current adds itself to wait queue. it will be
awaken by zcomp_strm_multi_put() caller.
zcomp_strm_multi_put():
- spin lock strm_lock
- add zcomp stream to idle list
- spin unlock, wake up sleeper
Minchan Kim reported that spinlock-based locking scheme has demonstrated a
severe perfomance regression for single compression stream case, comparing
to mutex-based (see https://lkml.org/lkml/2014/2/18/16)
base spinlock mutex
==Initial write ==Initial write ==Initial write
records: 5 records: 5 records: 5
avg: 1642424.35 avg: 699610.40 avg: 1655583.71
std: 39890.95(2.43%) std: 232014.19(33.16%) std: 52293.96
max: 1690170.94 max: 1163473.45 max: 1697164.75
min: 1568669.52 min: 573429.88 min: 1553410.23
==Rewrite ==Rewrite ==Rewrite
records: 5 records: 5 records: 5
avg: 1611775.39 avg: 501406.64 avg: 1684419.11
std: 17144.58(1.06%) std: 15354.41(3.06%) std: 18367.42
max: 1641800.95 max: 531356.78 max: 1706445.84
min: 1593515.27 min: 488817.78 min: 1655335.73
When only one compression stream available, mutex with spin on owner tends
to perform much better than frequent wait_event()/wake_up(). This is why
single stream implemented as a special case with mutex locking.
Introduce zram device attribute max_comp_streams to show and store current
zcomp's max number of zcomp streams (max_strm) and extend zcomp's
zcomp_create() with `max_strm' parameter. `max_strm' limits the number of
zcomp_strm structs in compression backend's idle list (max_comp_streams).
max_comp_streams used during initialisation as follows:
-- passing to zcomp_create() max_strm equals to 1 will initialise zcomp
using single compression stream zcomp_strm_single (mutex-based locking).
-- passing to zcomp_create() max_strm greater than 1 will initialise zcomp
using multi compression stream zcomp_strm_multi (spinlock-based locking).
default max_comp_streams value is 1, meaning that zram with single stream
will be initialised.
Later patch will introduce configuration knob to change max_comp_streams
on already initialised and used zcomp.
iozone -t 3 -R -r 16K -s 60M -I +Z
test base 1 strm (mutex) 3 strm (spinlock)
-----------------------------------------------------------------------
Initial write 589286.78 583518.39 718011.05
Rewrite 604837.97 596776.38 1515125.72
Random write 584120.11 595714.58 1388850.25
Pwrite 535731.17 541117.38 739295.27
Fwrite 1418083.88 1478612.72 1484927.06
Usage example:
set max_comp_streams to 4
echo 4 > /sys/block/zram0/max_comp_streams
show current max_comp_streams (default value is 1).
cat /sys/block/zram0/max_comp_streams
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/block/zram/zcomp.c | 126 +++++++++++++++++++++++++++++++++++++++++-
drivers/block/zram/zcomp.h | 2 +-
drivers/block/zram/zram_drv.c | 42 +++++++++++++-
drivers/block/zram/zram_drv.h | 2 +-
4 files changed, 166 insertions(+), 6 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 210cc28..0b790ac 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,6 +15,8 @@
#include "zcomp.h"
+extern struct zcomp_backend zcomp_lzo;
+
/*
* single zcomp_strm backend
*/
@@ -23,7 +25,20 @@ struct zcomp_strm_single {
struct zcomp_strm *zstrm;
};
-extern struct zcomp_backend zcomp_lzo;
+/*
+ * multi zcomp_strm backend
+ */
+struct zcomp_strm_multi {
+ /* protect strm list */
+ spinlock_t strm_lock;
+ /* max possible number of zstrm streams */
+ int max_strm;
+ /* number of available zstrm streams */
+ int avail_strm;
+ /* list of available strms */
+ struct list_head idle_strm;
+ wait_queue_head_t strm_wait;
+};
static struct zcomp_backend *find_backend(const char *compress)
{
@@ -63,6 +78,107 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
return zstrm;
}
+/*
+ * get idle zcomp_strm or wait until other process release
+ * (zcomp_strm_put()) one for us
+ */
+static struct zcomp_strm *zcomp_strm_multi_get(struct zcomp *comp)
+{
+ struct zcomp_strm_multi *zs = comp->stream;
+ struct zcomp_strm *zstrm;
+
+ while (1) {
+ spin_lock(&zs->strm_lock);
+ if (!list_empty(&zs->idle_strm)) {
+ zstrm = list_entry(zs->idle_strm.next,
+ struct zcomp_strm, list);
+ list_del(&zstrm->list);
+ spin_unlock(&zs->strm_lock);
+ return zstrm;
+ }
+ /* zstrm streams limit reached, wait for idle stream */
+ if (zs->avail_strm >= zs->max_strm) {
+ spin_unlock(&zs->strm_lock);
+ wait_event(zs->strm_wait, !list_empty(&zs->idle_strm));
+ continue;
+ }
+ /* allocate new zstrm stream */
+ zs->avail_strm++;
+ spin_unlock(&zs->strm_lock);
+
+ zstrm = zcomp_strm_alloc(comp);
+ if (!zstrm) {
+ spin_lock(&zs->strm_lock);
+ zs->avail_strm--;
+ spin_unlock(&zs->strm_lock);
+ wait_event(zs->strm_wait, !list_empty(&zs->idle_strm));
+ continue;
+ }
+ break;
+ }
+ return zstrm;
+}
+
+/* add stream back to idle list and wake up waiter or free the stream */
+static void zcomp_strm_multi_put(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+ struct zcomp_strm_multi *zs = comp->stream;
+
+ spin_lock(&zs->strm_lock);
+ if (zs->avail_strm <= zs->max_strm) {
+ list_add(&zstrm->list, &zs->idle_strm);
+ spin_unlock(&zs->strm_lock);
+ wake_up(&zs->strm_wait);
+ return;
+ }
+
+ zs->avail_strm--;
+ spin_unlock(&zs->strm_lock);
+ zcomp_strm_free(comp, zstrm);
+}
+
+static void zcomp_strm_multi_destroy(struct zcomp *comp)
+{
+ struct zcomp_strm_multi *zs = comp->stream;
+ struct zcomp_strm *zstrm;
+
+ while (!list_empty(&zs->idle_strm)) {
+ zstrm = list_entry(zs->idle_strm.next,
+ struct zcomp_strm, list);
+ list_del(&zstrm->list);
+ zcomp_strm_free(comp, zstrm);
+ }
+ kfree(zs);
+}
+
+static int zcomp_strm_multi_create(struct zcomp *comp, int max_strm)
+{
+ struct zcomp_strm *zstrm;
+ struct zcomp_strm_multi *zs;
+
+ comp->destroy = zcomp_strm_multi_destroy;
+ comp->strm_get = zcomp_strm_multi_get;
+ comp->strm_put = zcomp_strm_multi_put;
+ zs = kmalloc(sizeof(struct zcomp_strm_multi), GFP_KERNEL);
+ if (!zs)
+ return -ENOMEM;
+
+ comp->stream = zs;
+ spin_lock_init(&zs->strm_lock);
+ INIT_LIST_HEAD(&zs->idle_strm);
+ init_waitqueue_head(&zs->strm_wait);
+ zs->max_strm = max_strm;
+ zs->avail_strm = 1;
+
+ zstrm = zcomp_strm_alloc(comp);
+ if (!zstrm) {
+ kfree(zs);
+ return -ENOMEM;
+ }
+ list_add(&zstrm->list, &zs->idle_strm);
+ return 0;
+}
+
static struct zcomp_strm *zcomp_strm_single_get(struct zcomp *comp)
{
struct zcomp_strm_single *zs = comp->stream;
@@ -139,7 +255,7 @@ void zcomp_destroy(struct zcomp *comp)
* if requested algorithm is not supported or in case
* of init error
*/
-struct zcomp *zcomp_create(const char *compress)
+struct zcomp *zcomp_create(const char *compress, int max_strm)
{
struct zcomp *comp;
struct zcomp_backend *backend;
@@ -153,7 +269,11 @@ struct zcomp *zcomp_create(const char *compress)
return NULL;
comp->backend = backend;
- if (zcomp_strm_single_create(comp) != 0) {
+ if (max_strm > 1)
+ zcomp_strm_multi_create(comp, max_strm);
+ else
+ zcomp_strm_single_create(comp);
+ if (!comp->stream) {
kfree(comp);
return NULL;
}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 861e04d..9d77cbf 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -42,7 +42,7 @@ struct zcomp {
void (*destroy)(struct zcomp *comp);
};
-struct zcomp *zcomp_create(const char *comp);
+struct zcomp *zcomp_create(const char *comp, int max_strm);
void zcomp_destroy(struct zcomp *comp);
struct zcomp_strm *zcomp_strm_get(struct zcomp *comp);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 569ff58..42b9c7f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -108,6 +108,40 @@ static ssize_t mem_used_total_show(struct device *dev,
return sprintf(buf, "%llu\n", val);
}
+static ssize_t max_comp_streams_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int val;
+ struct zram *zram = dev_to_zram(dev);
+
+ down_read(&zram->init_lock);
+ val = zram->max_comp_streams;
+ up_read(&zram->init_lock);
+
+ return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t max_comp_streams_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ int num;
+ struct zram *zram = dev_to_zram(dev);
+
+ if (kstrtoint(buf, 0, &num))
+ return -EINVAL;
+ if (num < 1)
+ return -EINVAL;
+ down_write(&zram->init_lock);
+ if (init_done(zram)) {
+ up_write(&zram->init_lock);
+ pr_info("Can't set max_comp_streams for initialized device\n");
+ return -EBUSY;
+ }
+ zram->max_comp_streams = num;
+ up_write(&zram->init_lock);
+ return len;
+}
+
/* flag operations needs meta->tb_lock */
static int zram_test_flag(struct zram_meta *meta, u32 index,
enum zram_pageflags flag)
@@ -502,6 +536,8 @@ static void zram_reset_device(struct zram *zram, bool reset_capacity)
}
zcomp_destroy(zram->comp);
+ zram->max_comp_streams = 1;
+
zram_meta_free(zram->meta);
zram->meta = NULL;
/* Reset stats */
@@ -530,7 +566,7 @@ static ssize_t disksize_store(struct device *dev,
return -EBUSY;
}
- zram->comp = zcomp_create(default_compressor);
+ zram->comp = zcomp_create(default_compressor, zram->max_comp_streams);
if (!zram->comp) {
up_write(&zram->init_lock);
pr_info("Cannot initialise %s compressing backend\n",
@@ -693,6 +729,8 @@ static DEVICE_ATTR(initstate, S_IRUGO, initstate_show, NULL);
static DEVICE_ATTR(reset, S_IWUSR, NULL, reset_store);
static DEVICE_ATTR(orig_data_size, S_IRUGO, orig_data_size_show, NULL);
static DEVICE_ATTR(mem_used_total, S_IRUGO, mem_used_total_show, NULL);
+static DEVICE_ATTR(max_comp_streams, S_IRUGO | S_IWUSR,
+ max_comp_streams_show, max_comp_streams_store);
ZRAM_ATTR_RO(num_reads);
ZRAM_ATTR_RO(num_writes);
@@ -717,6 +755,7 @@ static struct attribute *zram_disk_attrs[] = {
&dev_attr_orig_data_size.attr,
&dev_attr_compr_data_size.attr,
&dev_attr_mem_used_total.attr,
+ &dev_attr_max_comp_streams.attr,
NULL,
};
@@ -779,6 +818,7 @@ static int create_device(struct zram *zram, int device_id)
}
zram->meta = NULL;
+ zram->max_comp_streams = 1;
return 0;
out_free_disk:
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 45e04f7..ccf36d1 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -99,7 +99,7 @@ struct zram {
* we can store in a disk.
*/
u64 disksize; /* bytes */
-
+ int max_comp_streams;
struct zram_stats stats;
};
#endif
--
1.9.0.291.g027825b
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv8 4/6] zram: add multi stream functionality
2014-02-26 12:27 ` [PATCHv8 4/6] zram: add multi stream functionality Sergey Senozhatsky
@ 2014-02-27 23:22 ` Andrew Morton
2014-02-27 23:25 ` Andrew Morton
1 sibling, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2014-02-27 23:22 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel
On Wed, 26 Feb 2014 15:27:57 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> This patch implements multi stream compression support.
um, what is "multi stream compression". Please define this term fully.
It's not immediately clear what the advantage is. How does this
feature benefit our users?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv8 4/6] zram: add multi stream functionality
2014-02-26 12:27 ` [PATCHv8 4/6] zram: add multi stream functionality Sergey Senozhatsky
2014-02-27 23:22 ` Andrew Morton
@ 2014-02-27 23:25 ` Andrew Morton
1 sibling, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2014-02-27 23:25 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel
On Wed, 26 Feb 2014 15:27:57 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> This patch implements multi stream compression support.
>
> ...
>
> @@ -693,6 +729,8 @@ static DEVICE_ATTR(initstate, S_IRUGO, initstate_show, NULL);
> static DEVICE_ATTR(reset, S_IWUSR, NULL, reset_store);
> static DEVICE_ATTR(orig_data_size, S_IRUGO, orig_data_size_show, NULL);
> static DEVICE_ATTR(mem_used_total, S_IRUGO, mem_used_total_show, NULL);
> +static DEVICE_ATTR(max_comp_streams, S_IRUGO | S_IWUSR,
> + max_comp_streams_show, max_comp_streams_store);
>
Update Documentation/blockdev/zram.txt and
Documentation/ABI/testing/sysfs-block-zram, please.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv8 5/6] zram: add set_max_streams knob
2014-02-26 12:27 [PATCHv8 0/6] add compressing abstraction and multi stream support Sergey Senozhatsky
` (3 preceding siblings ...)
2014-02-26 12:27 ` [PATCHv8 4/6] zram: add multi stream functionality Sergey Senozhatsky
@ 2014-02-26 12:27 ` Sergey Senozhatsky
2014-02-27 23:27 ` Andrew Morton
2014-02-26 12:27 ` [PATCHv8 6/6] zram: document max_comp_streams Sergey Senozhatsky
2014-02-27 2:16 ` [PATCHv8 0/6] add compressing abstraction and multi stream support Minchan Kim
6 siblings, 1 reply; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-26 12:27 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel,
Sergey Senozhatsky
This patch allows to change max_comp_streams on initialised zcomp.
Introduce zcomp set_max_streams() knob, zcomp_strm_multi_set_max_streams()
and zcomp_strm_single_set_max_streams() callbacks to change streams limit
for zcomp_strm_multi and zcomp_strm_single, accordingly. set_max_streams
for single steam zcomp does nothing.
If user has lowered the limit, then zcomp_strm_multi_set_max_streams()
attempts to immediately free extra streams (as much as it can, depending
on idle streams availability).
Note, this patch does not allow to change stream 'policy' from single to
multi stream (or vice versa) on already initialised compression backend.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/block/zram/zcomp.c | 35 +++++++++++++++++++++++++++++++++++
drivers/block/zram/zcomp.h | 3 +++
drivers/block/zram/zram_drv.c | 7 +++----
3 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 0b790ac..37d72d9 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -137,6 +137,28 @@ static void zcomp_strm_multi_put(struct zcomp *comp, struct zcomp_strm *zstrm)
zcomp_strm_free(comp, zstrm);
}
+/* change max_strm limit */
+static int zcomp_strm_multi_set_max_streams(struct zcomp *comp, int num_strm)
+{
+ struct zcomp_strm_multi *zs = comp->stream;
+
+ spin_lock(&zs->strm_lock);
+ zs->max_strm = num_strm;
+ /*
+ * if user has lowered the limit and there are idle streams,
+ * immediately free as much streams (and memory) as we can.
+ */
+ while (zs->avail_strm > num_strm && !list_empty(&zs->idle_strm)) {
+ struct zcomp_strm *zstrm = list_entry(zs->idle_strm.next,
+ struct zcomp_strm, list);
+ list_del(&zstrm->list);
+ zcomp_strm_free(comp, zstrm);
+ zs->avail_strm--;
+ }
+ spin_unlock(&zs->strm_lock);
+ return 0;
+}
+
static void zcomp_strm_multi_destroy(struct zcomp *comp)
{
struct zcomp_strm_multi *zs = comp->stream;
@@ -159,6 +181,7 @@ static int zcomp_strm_multi_create(struct zcomp *comp, int max_strm)
comp->destroy = zcomp_strm_multi_destroy;
comp->strm_get = zcomp_strm_multi_get;
comp->strm_put = zcomp_strm_multi_put;
+ comp->set_max_streams = zcomp_strm_multi_set_max_streams;
zs = kmalloc(sizeof(struct zcomp_strm_multi), GFP_KERNEL);
if (!zs)
return -ENOMEM;
@@ -192,6 +215,12 @@ static void zcomp_strm_single_put(struct zcomp *comp, struct zcomp_strm *zstrm)
mutex_unlock(&zs->strm_lock);
}
+static int zcomp_strm_single_set_max_streams(struct zcomp *comp, int num_strm)
+{
+ /* zcomp_strm_single support only max_comp_streams == 1 */
+ return -ENOTSUPP;
+}
+
static void zcomp_strm_single_destroy(struct zcomp *comp)
{
struct zcomp_strm_single *zs = comp->stream;
@@ -206,6 +235,7 @@ static int zcomp_strm_single_create(struct zcomp *comp)
comp->destroy = zcomp_strm_single_destroy;
comp->strm_get = zcomp_strm_single_get;
comp->strm_put = zcomp_strm_single_put;
+ comp->set_max_streams = zcomp_strm_single_set_max_streams;
zs = kmalloc(sizeof(struct zcomp_strm_single), GFP_KERNEL);
if (!zs)
return -ENOMEM;
@@ -220,6 +250,11 @@ static int zcomp_strm_single_create(struct zcomp *comp)
return 0;
}
+int zcomp_set_max_streams(struct zcomp *comp, int num_strm)
+{
+ return comp->set_max_streams(comp, num_strm);
+}
+
struct zcomp_strm *zcomp_strm_get(struct zcomp *comp)
{
return comp->strm_get(comp);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 9d77cbf..a2a9dc1 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -39,6 +39,7 @@ struct zcomp {
struct zcomp_strm *(*strm_get)(struct zcomp *comp);
void (*strm_put)(struct zcomp *comp, struct zcomp_strm *zstrm);
+ int (*set_max_streams)(struct zcomp *comp, int num_strm);
void (*destroy)(struct zcomp *comp);
};
@@ -53,4 +54,6 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
size_t src_len, unsigned char *dst);
+
+int zcomp_set_max_streams(struct zcomp *comp, int num_strm);
#endif /* _ZCOMP_H_ */
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 42b9c7f..88b12d8 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -132,12 +132,11 @@ static ssize_t max_comp_streams_store(struct device *dev,
if (num < 1)
return -EINVAL;
down_write(&zram->init_lock);
+ zram->max_comp_streams = num;
if (init_done(zram)) {
- up_write(&zram->init_lock);
- pr_info("Can't set max_comp_streams for initialized device\n");
- return -EBUSY;
+ if (zcomp_set_max_streams(zram->comp, num))
+ pr_info("Cannot change max compression streams\n");
}
- zram->max_comp_streams = num;
up_write(&zram->init_lock);
return len;
}
--
1.9.0.291.g027825b
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv8 5/6] zram: add set_max_streams knob
2014-02-26 12:27 ` [PATCHv8 5/6] zram: add set_max_streams knob Sergey Senozhatsky
@ 2014-02-27 23:27 ` Andrew Morton
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2014-02-27 23:27 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel
On Wed, 26 Feb 2014 15:27:58 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> This patch allows to change max_comp_streams on initialised zcomp.
>
> Introduce zcomp set_max_streams() knob, zcomp_strm_multi_set_max_streams()
> and zcomp_strm_single_set_max_streams() callbacks to change streams limit
> for zcomp_strm_multi and zcomp_strm_single, accordingly. set_max_streams
> for single steam zcomp does nothing.
>
> If user has lowered the limit, then zcomp_strm_multi_set_max_streams()
> attempts to immediately free extra streams (as much as it can, depending
> on idle streams availability).
>
> Note, this patch does not allow to change stream 'policy' from single to
> multi stream (or vice versa) on already initialised compression backend.
>
> ...
>
> @@ -137,6 +137,28 @@ static void zcomp_strm_multi_put(struct zcomp *comp, struct zcomp_strm *zstrm)
> zcomp_strm_free(comp, zstrm);
> }
>
> +/* change max_strm limit */
> +static int zcomp_strm_multi_set_max_streams(struct zcomp *comp, int num_strm)
> +{
> + struct zcomp_strm_multi *zs = comp->stream;
> +
> + spin_lock(&zs->strm_lock);
> + zs->max_strm = num_strm;
> + /*
> + * if user has lowered the limit and there are idle streams,
> + * immediately free as much streams (and memory) as we can.
> + */
> + while (zs->avail_strm > num_strm && !list_empty(&zs->idle_strm)) {
> + struct zcomp_strm *zstrm = list_entry(zs->idle_strm.next,
> + struct zcomp_strm, list);
struct zcomp_strm *zstrm;
zstrm = list_entry(zs->idle_strm.next, struct zcomp_strm, list);
will avoid the coding-style mess.
> + list_del(&zstrm->list);
> + zcomp_strm_free(comp, zstrm);
> + zs->avail_strm--;
> + }
> + spin_unlock(&zs->strm_lock);
> + return 0;
> +}
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv8 6/6] zram: document max_comp_streams
2014-02-26 12:27 [PATCHv8 0/6] add compressing abstraction and multi stream support Sergey Senozhatsky
` (4 preceding siblings ...)
2014-02-26 12:27 ` [PATCHv8 5/6] zram: add set_max_streams knob Sergey Senozhatsky
@ 2014-02-26 12:27 ` Sergey Senozhatsky
2014-02-27 23:28 ` Andrew Morton
2014-02-27 2:16 ` [PATCHv8 0/6] add compressing abstraction and multi stream support Minchan Kim
6 siblings, 1 reply; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-26 12:27 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel,
Sergey Senozhatsky
Add max_comp_streams device attribute documentation.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
Documentation/ABI/testing/sysfs-block-zram | 9 ++++++++-
Documentation/blockdev/zram.txt | 31 +++++++++++++++++++++++++-----
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index 8aa0468..0da9ed6 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -50,7 +50,6 @@ Description:
The failed_reads file is read-only and specifies the number of
failed reads happened on this device.
-
What: /sys/block/zram<id>/failed_writes
Date: February 2014
Contact: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
@@ -58,6 +57,14 @@ Description:
The failed_writes file is read-only and specifies the number of
failed writes happened on this device.
+What: /sys/block/zram<id>/max_comp_streams
+Date: February 2014
+Contact: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
+Description:
+ The max_comp_streams file is read-write and specifies the
+ number of backend's zcomp_strm compression streams (number of
+ concurrent compress operations).
+
What: /sys/block/zram<id>/notify_free
Date: August 2010
Contact: Nitin Gupta <ngupta@vflare.org>
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index b31ac5e..1f96abb 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -21,7 +21,28 @@ Following shows a typical sequence of steps for using zram.
This creates 4 devices: /dev/zram{0,1,2,3}
(num_devices parameter is optional. Default: 1)
-2) Set Disksize
+2) Set max number of compression streams
+ Compression backend may use up to max_comp_streams compression streams,
+ thus allowing up to max_comp_streams concurrent compression operations.
+ By default, compression backend uses single compression stream.
+
+ Examples:
+ #show max compression streams number
+ cat /sys/block/zram0/max_comp_streams
+
+ #set max compression streams number to 3
+ echo 3 > /sys/block/zram0/max_comp_streams
+
+Note:
+In order to enable compression backend's multi stream support max_comp_streams must
+be initially set to desired concurrency level before ZRAM device initialisation.
+Once the device initialised as a single stream compression backend (max_comp_streams
+equals to 0) changing the value of max_comp_streams will not take any effect, because
+single stream compression backend implemented as a special case and does not support
+dynamic max_comp_streams. Only multi stream backend supports dynamic max_comp_streams
+adjustment.
+
+3) Set Disksize
Set disk size by writing the value to sysfs node 'disksize'.
The value can be either in bytes or you can use mem suffixes.
Examples:
@@ -38,14 +59,14 @@ There is little point creating a zram of greater than twice the size of memory
since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of the
size of the disk when not in use so a huge zram is wasteful.
-3) Activate:
+4) Activate:
mkswap /dev/zram0
swapon /dev/zram0
mkfs.ext4 /dev/zram1
mount /dev/zram1 /tmp
-4) Stats:
+5) Stats:
Per-device statistics are exported as various nodes under
/sys/block/zram<id>/
disksize
@@ -60,11 +81,11 @@ size of the disk when not in use so a huge zram is wasteful.
compr_data_size
mem_used_total
-5) Deactivate:
+6) Deactivate:
swapoff /dev/zram0
umount /dev/zram1
-6) Reset:
+7) Reset:
Write any positive value to 'reset' sysfs node
echo 1 > /sys/block/zram0/reset
echo 1 > /sys/block/zram1/reset
--
1.9.0.291.g027825b
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv8 0/6] add compressing abstraction and multi stream support
2014-02-26 12:27 [PATCHv8 0/6] add compressing abstraction and multi stream support Sergey Senozhatsky
` (5 preceding siblings ...)
2014-02-26 12:27 ` [PATCHv8 6/6] zram: document max_comp_streams Sergey Senozhatsky
@ 2014-02-27 2:16 ` Minchan Kim
2014-02-27 8:08 ` Sergey Senozhatsky
6 siblings, 1 reply; 16+ messages in thread
From: Minchan Kim @ 2014-02-27 2:16 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel
Hello Sergey,
Code looks good to me and I will queue it into ARM and x86 test tomorrow
and give the result to you with Acked-by.
Thanks for your all works!
On Wed, Feb 26, 2014 at 03:27:53PM +0300, Sergey Senozhatsky wrote:
> This patchset introduces zcomp compression backend abstraction
> adding ability to support compression algorithms other than LZO;
> support for multi compression streams, making parallel compressions
> possible.
>
> v7->v8 (reviewed by Minchan Kim):
> -- merge patches 'add multi stream functionality' and 'enable multi
> stream compression support in zram'
> -- return status code from set_max_streams knob and print message on
> error
> -- do not use atomic type for ->avail_strm
> -- return back: allocate by default only one stream for multi stream backend
> -- wake sleeping write in zcomp_strm_multi_put() only if we put stream
> to idle list
> -- minor code `nitpicks'
>
> v6->v7 (reviewed by Minchan Kim):
> -- enable multi and single stream support out of the box (drop
> ZRAM_MULTI_STREAM config option)
> -- add set_max_stream knob, so we can adjust max number of compression
> streams in runtime (for multi stream backend at the moment)
> -- minor code `nitpicks'
>
> v5->v6 (reviewed by Minchan Kim):
> -- handle single compression stream case separately, using mutex locking,
> to address perfomance regression
> -- handle multi compression stream using spin lock and wait_event()/wake_up()
> -- make multi compression stream support configurable (ZRAM_MULTI_STREAM
> config option)
>
> v4->v5 (reviewed by Minchan Kim):
> -- renamed zcomp buffer_lock; removed src len and dst len from
> compress() and decompress(); not using term `buffer' and
> `workmem' in code and documentation; define compress() and
> decompress() functions for LZO backend; not using goto's;
> do not put idle zcomp_strm to idle list tail.
>
> v3->v4 (reviewed by Minchan Kim):
> -- renamed compression backend and working memory structs as requested
> by Minchan Kim; fixed several issues noted by Minchan Kim.
>
> Sergey Senozhatsky (6):
> zram: introduce compressing backend abstraction
> zram: use zcomp compressing backends
> zram: factor out single stream compression
> zram: add multi stream functionality
> zram: add set_max_streams knob
> zram: document max_comp_streams
>
> Documentation/ABI/testing/sysfs-block-zram | 9 +-
> Documentation/blockdev/zram.txt | 31 ++-
> drivers/block/zram/Makefile | 2 +-
> drivers/block/zram/zcomp.c | 318 +++++++++++++++++++++++++++++
> drivers/block/zram/zcomp.h | 59 ++++++
> drivers/block/zram/zcomp_lzo.c | 48 +++++
> drivers/block/zram/zram_drv.c | 98 +++++----
> drivers/block/zram/zram_drv.h | 10 +-
> 8 files changed, 528 insertions(+), 47 deletions(-)
> create mode 100644 drivers/block/zram/zcomp.c
> create mode 100644 drivers/block/zram/zcomp.h
> create mode 100644 drivers/block/zram/zcomp_lzo.c
>
> --
> 1.9.0.291.g027825b
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Kind regards,
Minchan Kim
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCHv8 0/6] add compressing abstraction and multi stream support
2014-02-27 2:16 ` [PATCHv8 0/6] add compressing abstraction and multi stream support Minchan Kim
@ 2014-02-27 8:08 ` Sergey Senozhatsky
0 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2014-02-27 8:08 UTC (permalink / raw)
To: Minchan Kim
Cc: Sergey Senozhatsky, Andrew Morton, Jerome Marchand, Nitin Gupta,
linux-kernel
Hello Minchan,
On (02/27/14 11:16), Minchan Kim wrote:
> Hello Sergey,
>
> Code looks good to me and I will queue it into ARM and x86 test tomorrow
> and give the result to you with Acked-by.
>
> Thanks for your all works!
>
thanks a lot!
-ss
> On Wed, Feb 26, 2014 at 03:27:53PM +0300, Sergey Senozhatsky wrote:
> > This patchset introduces zcomp compression backend abstraction
> > adding ability to support compression algorithms other than LZO;
> > support for multi compression streams, making parallel compressions
> > possible.
> >
> > v7->v8 (reviewed by Minchan Kim):
> > -- merge patches 'add multi stream functionality' and 'enable multi
> > stream compression support in zram'
> > -- return status code from set_max_streams knob and print message on
> > error
> > -- do not use atomic type for ->avail_strm
> > -- return back: allocate by default only one stream for multi stream backend
> > -- wake sleeping write in zcomp_strm_multi_put() only if we put stream
> > to idle list
> > -- minor code `nitpicks'
> >
> > v6->v7 (reviewed by Minchan Kim):
> > -- enable multi and single stream support out of the box (drop
> > ZRAM_MULTI_STREAM config option)
> > -- add set_max_stream knob, so we can adjust max number of compression
> > streams in runtime (for multi stream backend at the moment)
> > -- minor code `nitpicks'
> >
> > v5->v6 (reviewed by Minchan Kim):
> > -- handle single compression stream case separately, using mutex locking,
> > to address perfomance regression
> > -- handle multi compression stream using spin lock and wait_event()/wake_up()
> > -- make multi compression stream support configurable (ZRAM_MULTI_STREAM
> > config option)
> >
> > v4->v5 (reviewed by Minchan Kim):
> > -- renamed zcomp buffer_lock; removed src len and dst len from
> > compress() and decompress(); not using term `buffer' and
> > `workmem' in code and documentation; define compress() and
> > decompress() functions for LZO backend; not using goto's;
> > do not put idle zcomp_strm to idle list tail.
> >
> > v3->v4 (reviewed by Minchan Kim):
> > -- renamed compression backend and working memory structs as requested
> > by Minchan Kim; fixed several issues noted by Minchan Kim.
> >
> > Sergey Senozhatsky (6):
> > zram: introduce compressing backend abstraction
> > zram: use zcomp compressing backends
> > zram: factor out single stream compression
> > zram: add multi stream functionality
> > zram: add set_max_streams knob
> > zram: document max_comp_streams
> >
> > Documentation/ABI/testing/sysfs-block-zram | 9 +-
> > Documentation/blockdev/zram.txt | 31 ++-
> > drivers/block/zram/Makefile | 2 +-
> > drivers/block/zram/zcomp.c | 318 +++++++++++++++++++++++++++++
> > drivers/block/zram/zcomp.h | 59 ++++++
> > drivers/block/zram/zcomp_lzo.c | 48 +++++
> > drivers/block/zram/zram_drv.c | 98 +++++----
> > drivers/block/zram/zram_drv.h | 10 +-
> > 8 files changed, 528 insertions(+), 47 deletions(-)
> > create mode 100644 drivers/block/zram/zcomp.c
> > create mode 100644 drivers/block/zram/zcomp.h
> > create mode 100644 drivers/block/zram/zcomp_lzo.c
> >
> > --
> > 1.9.0.291.g027825b
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> --
> Kind regards,
> Minchan Kim
>
^ permalink raw reply [flat|nested] 16+ messages in thread