On (02/18/14 15:04), Minchan Kim wrote:
[..]
> 1.8.5.3
> 
> As you can see, your patch made zram too much regressed when it
> used one stream buffer. The reason I guessed is overhead of
> scheduling by sleep/wakeup when others couldn't find a idle stream
> so I had an experiment with below simple patch to restore old behavior
> so it works well again. The reason old behaivor was really good is
> it uses mutex with spin_on_owner so that it could avoid frequent
> sleep/wakeup.
> 
> A solution I could think is that we could grow up the number of
> buffer dynamically up to max_buffers(ex, 2 * number of CPU) so we
> could grab a idle stream easily for each CPU while we could release
> buffers by shrinker when the memory pressure is severe.
> Of course, we should keep one buffer to work.
> 
> Another solution I can imagaine is to define CONFIG_ZRAM_MULTI_STREAM
> and CONFIG_ZRAM_SINGLE_STREAM(ie, default) so we couldn't break good
> performance for old cases and new users who want write parallel
> could use ZRAM_MULTI which should be several streams at a default
> rather than a stream. And we could ehhacne it further by dynamic control
> as I mentioned.
> 
> Thoughts?
>

like the following one (this patch limits the number of streams to
num_online_cpus(). I really don't mind to limit zstrm to
N * num_online_cpus() where N could be 2)

        iozone -t 3 -R -r 16K -s 60M -I +Z

write test:
(old core i5 laptop, 2 cpus + 2 HyperThreading)

                       base            multi buffer

"  Initial write "   574938.14           716623.64
"        Rewrite "   573541.22          1499074.62
"   Random write "   587016.14          1393996.66
"         Pwrite "   595616.45           711028.70
"         Fwrite "  1482843.62          1493398.12


zcomp_strm_get() and zcomp_strm_put() were updated.
zcomp keeps the number allocated and still active (not freed)
streams (atomic_t). zcomp performs zstrm free() only if
current number of streams exceeds the number of online
cpus (cpu went offline).


not a properly `aligned' patch, just to demonstrate the idea.

thoughts?

thanks.

---

>From 0e16f26480933d22339675ca938049dfb21557e2 Mon Sep 17 00:00:00 2001
From: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Date: Tue, 11 Feb 2014 00:28:46 +0300
Subject: [PATCH] zram: multi stream compressing backend abstraction

Signed-off-by: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
---
 drivers/block/zram/zcomp.c     | 174 +++++++++++++++++++++++++++++++++++++++++
 drivers/block/zram/zcomp.h     |  58 ++++++++++++++
 drivers/block/zram/zcomp_lzo.c |  48 ++++++++++++
 3 files changed, 280 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..6bf49fc
--- /dev/null
+++ b/drivers/block/zram/zcomp.c
@@ -0,0 +1,174 @@
+/*
+ * 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;
+}
+
+/*
+ * get available idle stream or allocate a new one
+ */
+struct zcomp_strm *zcomp_strm_get(struct zcomp *comp)
+{
+       struct zcomp_strm *zstrm;
+
+       while (1) {
+               spin_lock(&comp->strm_lock);
+               if (!list_empty(&comp->idle_strm)) {
+                       zstrm = list_entry(comp->idle_strm.next,
+                                       struct zcomp_strm, list);
+                       list_del(&zstrm->list);
+                       spin_unlock(&comp->strm_lock);
+                       return zstrm;
+               }
+
+               if (atomic_read(&comp->num_strm) >= num_online_cpus()) {
+                       spin_unlock(&comp->strm_lock);
+                       wait_event(comp->strm_wait,
+                                       !list_empty(&comp->idle_strm));
+                       continue;
+               }
+
+               atomic_inc(&comp->num_strm);
+               spin_unlock(&comp->strm_lock);
+
+               zstrm = zcomp_strm_alloc(comp);
+               if (!zstrm) {
+                       atomic_dec(&comp->num_strm);
+                       wait_event(comp->strm_wait,
+                                       !list_empty(&comp->idle_strm));
+                       continue;
+               }
+               break;
+       }
+       return zstrm;
+}
+
+/*
+ * put zstrm back to idle list and wake up waiters or free it if
+ * current number of streams is above the limit
+ */
+void zcomp_strm_put(struct zcomp *comp, struct zcomp_strm *zstrm)
+{
+       spin_lock(&comp->strm_lock);
+       if (atomic_read(&comp->num_strm) <= num_online_cpus()) {
+               list_add(&zstrm->list, &comp->idle_strm);
+               spin_unlock(&comp->strm_lock);
+       } else {
+               atomic_dec(&comp->num_strm);
+               spin_unlock(&comp->strm_lock);
+               zcomp_strm_free(comp, zstrm);
+       }
+
+       wake_up(&comp->strm_wait);
+}
+
+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)
+{
+       struct zcomp_strm *zstrm;
+       while (!list_empty(&comp->idle_strm)) {
+               zstrm = list_entry(comp->idle_strm.next,
+                               struct zcomp_strm, list);
+               list_del(&zstrm->list);
+               zcomp_strm_free(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;
+       struct zcomp_strm *zstrm;
+
+       backend = find_backend(compress);
+       if (!backend)
+               return NULL;
+
+       comp = kmalloc(sizeof(struct zcomp), GFP_KERNEL);
+       if (!comp)
+               return NULL;
+
+       comp->backend = backend;
+       spin_lock_init(&comp->strm_lock);
+       INIT_LIST_HEAD(&comp->idle_strm);
+       init_waitqueue_head(&comp->strm_wait);
+       atomic_set(&comp->num_strm, 1);
+
+       zstrm = zcomp_strm_alloc(comp);
+       if (!zstrm) {
+               zcomp_destroy(comp);
+               return NULL;
+       }
+       list_add(&zstrm->list, &comp->idle_strm);
+       return comp;
+}
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
new file mode 100644
index 0000000..fd3871d
--- /dev/null
+++ b/drivers/block/zram/zcomp.h
@@ -0,0 +1,58 @@
+/*
+ * 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/spinlock.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 {
+       /* protect strm list */
+       spinlock_t strm_lock;
+       /* list of available strms */
+       struct list_head idle_strm;
+       wait_queue_head_t strm_wait;
+       /* number of allocated strms */
+       atomic_t num_strm;
+       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.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to