[PATCH 0/7] zram: switch to crypto api

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/7] zram: switch to crypto api

Sergey Senozhatsky
Hello,

  This has started as a 'add zlib support' work, but after some
thinking I saw no blockers for a bigger change -- a switch to
crypto API.

  We don't have an idle zstreams list anymore and our write path
now works absolutely differently, preventing preemption during
compression. This removes possibilities of read paths preempting
writes at wrong places and opens the door for a move from custom
LZO/LZ4 compression backends implementation to a more generic one,
using crypto compress API.

  This patch set also eliminates the need of a new context-less
crypto API interface, which was quite hard to sell, so we can
move along faster.


Sergey Senozhatsky (7):
  zram: rename zstrm find-release functions
  zram: switch to crypto compress API
  zram: drop zcomp param from compress/decompress
  zram: align zcomp interface to crypto comp API
  zram: use crypto api to check alg availability
  zram: delete custom lzo/lz4
  zram: add more compression algorithms

 drivers/block/zram/Kconfig     | 15 +------
 drivers/block/zram/Makefile    |  4 +-
 drivers/block/zram/zcomp.c     | 91 +++++++++++++++++++++++++++---------------
 drivers/block/zram/zcomp.h     | 29 ++++----------
 drivers/block/zram/zcomp_lz4.c | 56 --------------------------
 drivers/block/zram/zcomp_lz4.h | 17 --------
 drivers/block/zram/zcomp_lzo.c | 56 --------------------------
 drivers/block/zram/zcomp_lzo.h | 17 --------
 drivers/block/zram/zram_drv.c  | 26 +++++++-----
 9 files changed, 84 insertions(+), 227 deletions(-)
 delete mode 100644 drivers/block/zram/zcomp_lz4.c
 delete mode 100644 drivers/block/zram/zcomp_lz4.h
 delete mode 100644 drivers/block/zram/zcomp_lzo.c
 delete mode 100644 drivers/block/zram/zcomp_lzo.h

--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/7] zram: rename zstrm find-release functions

Sergey Senozhatsky
We don't perform any zstream idle list lookup anymore, so
zcomp_strm_find()/zcomp_strm_release() names are not
representative.

Rename to zcomp_stream_get()/zcomp_stream_put().

Signed-off-by: Sergey Senozhatsky <[hidden email]>
Cc: Minchan Kim <[hidden email]>
Cc: Joonsoo Kim <[hidden email]>
---
 drivers/block/zram/zcomp.c    | 4 ++--
 drivers/block/zram/zcomp.h    | 4 ++--
 drivers/block/zram/zram_drv.c | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index b51a816..400f826 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -95,12 +95,12 @@ bool zcomp_available_algorithm(const char *comp)
  return find_backend(comp) != NULL;
 }
 
-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
+struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
 {
  return *get_cpu_ptr(comp->stream);
 }
 
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm)
+void zcomp_stream_put(struct zcomp *comp)
 {
  put_cpu_ptr(comp->stream);
 }
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index ffd88cb..944b8e6 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -48,8 +48,8 @@ bool zcomp_available_algorithm(const char *comp);
 struct zcomp *zcomp_create(const char *comp);
 void zcomp_destroy(struct zcomp *comp);
 
-struct zcomp_strm *zcomp_strm_find(struct zcomp *comp);
-void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm);
+struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
+void zcomp_stream_put(struct zcomp *comp);
 
 int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
  const unsigned char *src, size_t *dst_len);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 8fcad8b..9361a5d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -695,7 +695,7 @@ compress_again:
  goto out;
  }
 
- zstrm = zcomp_strm_find(zram->comp);
+ zstrm = zcomp_stream_get(zram->comp);
  ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen);
  if (!is_partial_io(bvec)) {
  kunmap_atomic(user_mem);
@@ -734,7 +734,7 @@ compress_again:
  __GFP_NOWARN |
  __GFP_HIGHMEM);
  if (!handle) {
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_stream_put(zram->comp);
  zstrm = NULL;
 
  atomic64_inc(&zram->stats.writestall);
@@ -769,7 +769,7 @@ compress_again:
  memcpy(cmem, src, clen);
  }
 
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_stream_put(zram->comp);
  zstrm = NULL;
  zs_unmap_object(meta->mem_pool, handle);
 
@@ -789,7 +789,7 @@ compress_again:
  atomic64_inc(&zram->stats.pages_stored);
 out:
  if (zstrm)
- zcomp_strm_release(zram->comp, zstrm);
+ zcomp_stream_put(zram->comp);
  if (is_partial_io(bvec))
  kfree(uncmem);
  return ret;
--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/7] zram: switch to crypto compress API

Sergey Senozhatsky
In reply to this post by Sergey Senozhatsky
We don't have an idle zstreams list anymore and our write path
now works absolutely differently, preventing preemption during
compression. This removes possibilities of read paths preempting
writes at wrong places (which could badly affect the performance
of both paths) and at the same time opens the door for a move
from custom LZO/LZ4 compression backends implementation to a more
generic one, using crypto compress API.

Joonsoo Kim [1] attempted to do this a while ago, but faced with
the need of introducing a new crypto API interface. The root cause
was the fact that crypto API compression algorithms require a
compression stream structure (in zram terminology) for both
compression and decompression ops, while in reality only several
of compression algorithms really need it. This resulted in a
concept of context-less crypto API compression backends [2]. Both
write and read paths, though, would have been executed with the
preemption enabled, which in the worst case could have resulted
in a decreased worst-case performance, e.g. consider the
following case:

        CPU0

        zram_write()
          spin_lock()
            take the last idle stream
          spin_unlock()

        << preempted >>

                zram_read()
                  spin_lock()
                   no idle streams
                          spin_unlock()
                          schedule()

        resuming zram_write compression()

but it took me some time to realize that, and it took even longer
to evolve zram and to make it ready for crypto API. The key turned
out to be -- drop the idle streams list entirely. With out the
idle streams list we are free to use compression algorithms that
require compression stream for decompression (read), because streams
are now placed in per-cpu data and each write path has to disable
preemption for compression op, almost completely eliminating the
aforementioned case (technically, we still have a small chance,
because write path has a fast and a slow paths and the slow path
is executed with the preemption enabled; but the frequency of
failed fast path is too low).

TEST
====

- 4 CPUs, x86_64 system
- 3G zram, lzo
- fio tests: read, randread, write, randwrite, rw, randrw

test script [3] command:
 ZRAM_SIZE=3G LOG_SUFFIX=XXXX FIO_LOOPS=5 ./zram-fio-test.sh

                   BASE           PATCHED
jobs1
READ:           2527.2MB/s 2482.7MB/s
READ:           2102.7MB/s 2045.0MB/s
WRITE:          1284.3MB/s 1324.3MB/s
WRITE:          1080.7MB/s 1101.9MB/s
READ:           430125KB/s 437498KB/s
WRITE:          430538KB/s 437919KB/s
READ:           399593KB/s 403987KB/s
WRITE:          399910KB/s 404308KB/s
jobs2
READ:           8133.5MB/s 7854.8MB/s
READ:           7086.6MB/s 6912.8MB/s
WRITE:          3177.2MB/s 3298.3MB/s
WRITE:          2810.2MB/s 2871.4MB/s
READ:           1017.6MB/s 1023.4MB/s
WRITE:          1018.2MB/s 1023.1MB/s
READ:           977836KB/s 984205KB/s
WRITE:          979435KB/s 985814KB/s
jobs3
READ:           13557MB/s 13391MB/s
READ:           11876MB/s 11752MB/s
WRITE:          4641.5MB/s 4682.1MB/s
WRITE:          4164.9MB/s 4179.3MB/s
READ:           1453.8MB/s 1455.1MB/s
WRITE:          1455.1MB/s 1458.2MB/s
READ:           1387.7MB/s 1395.7MB/s
WRITE:          1386.1MB/s 1394.9MB/s
jobs4
READ:           20271MB/s 20078MB/s
READ:           18033MB/s 17928MB/s
WRITE:          6176.8MB/s 6180.5MB/s
WRITE:          5686.3MB/s 5705.3MB/s
READ:           2009.4MB/s 2006.7MB/s
WRITE:          2007.5MB/s 2004.9MB/s
READ:           1929.7MB/s 1935.6MB/s
WRITE:          1926.8MB/s 1932.6MB/s
jobs5
READ:           18823MB/s 19024MB/s
READ:           18968MB/s 19071MB/s
WRITE:          6191.6MB/s 6372.1MB/s
WRITE:          5818.7MB/s 5787.1MB/s
READ:           2011.7MB/s 1981.3MB/s
WRITE:          2011.4MB/s 1980.1MB/s
READ:           1949.3MB/s 1935.7MB/s
WRITE:          1940.4MB/s 1926.1MB/s
jobs6
READ:           21870MB/s 21715MB/s
READ:           19957MB/s 19879MB/s
WRITE:          6528.4MB/s 6537.6MB/s
WRITE:          6098.9MB/s 6073.6MB/s
READ:           2048.6MB/s 2049.9MB/s
WRITE:          2041.7MB/s 2042.9MB/s
READ:           2013.4MB/s 1990.4MB/s
WRITE:          2009.4MB/s 1986.5MB/s
jobs7
READ:           21359MB/s 21124MB/s
READ:           19746MB/s 19293MB/s
WRITE:          6660.4MB/s 6518.8MB/s
WRITE:          6211.6MB/s 6193.1MB/s
READ:           2089.7MB/s 2080.6MB/s
WRITE:          2085.8MB/s 2076.5MB/s
READ:           2041.2MB/s 2052.5MB/s
WRITE:          2037.5MB/s 2048.8MB/s
jobs8
READ:           20477MB/s 19974MB/s
READ:           18922MB/s 18576MB/s
WRITE:          6851.9MB/s 6788.3MB/s
WRITE:          6407.7MB/s 6347.5MB/s
READ:           2134.8MB/s 2136.1MB/s
WRITE:          2132.8MB/s 2134.4MB/s
READ:           2074.2MB/s 2069.6MB/s
WRITE:          2087.3MB/s 2082.4MB/s
jobs9
READ:           19797MB/s 19994MB/s
READ:           18806MB/s 18581MB/s
WRITE:          6878.7MB/s 6822.7MB/s
WRITE:          6456.8MB/s 6447.2MB/s
READ:           2141.1MB/s 2154.7MB/s
WRITE:          2144.4MB/s 2157.3MB/s
READ:           2084.1MB/s 2085.1MB/s
WRITE:          2091.5MB/s 2092.5MB/s
jobs10
READ:           19794MB/s 19784MB/s
READ:           18794MB/s 18745MB/s
WRITE:          6984.4MB/s 6676.3MB/s
WRITE:          6532.3MB/s 6342.7MB/s
READ:           2150.6MB/s 2155.4MB/s
WRITE:          2156.8MB/s 2161.5MB/s
READ:           2106.4MB/s 2095.6MB/s
WRITE:          2109.7MB/s 2098.4MB/s

                                    BASE                       PATCHED
jobs1                              perfstat
stalled-cycles-frontend     102,480,595,419 (  41.53%)  114,508,864,804 (  46.92%)
stalled-cycles-backend       51,941,417,832 (  21.05%)   46,836,112,388 (  19.19%)
instructions                283,612,054,215 (    1.15)  283,918,134,959 (    1.16)
branches                     56,372,560,385 ( 724.923)   56,449,814,753 ( 733.766)
branch-misses                   374,826,000 (   0.66%)      326,935,859 (   0.58%)
jobs2                              perfstat
stalled-cycles-frontend     155,142,745,777 (  40.99%)  164,170,979,198 (  43.82%)
stalled-cycles-backend       70,813,866,387 (  18.71%)   66,456,858,165 (  17.74%)
instructions                463,436,648,173 (    1.22)  464,221,890,191 (    1.24)
branches                     91,088,733,902 ( 760.088)   91,278,144,546 ( 769.133)
branch-misses                   504,460,363 (   0.55%)      394,033,842 (   0.43%)
jobs3                              perfstat
stalled-cycles-frontend     201,300,397,212 (  39.84%)  223,969,902,257 (  44.44%)
stalled-cycles-backend       87,712,593,974 (  17.36%)   81,618,888,712 (  16.19%)
instructions                642,869,545,023 (    1.27)  644,677,354,132 (    1.28)
branches                    125,724,560,594 ( 690.682)  126,133,159,521 ( 694.542)
branch-misses                   527,941,798 (   0.42%)      444,782,220 (   0.35%)
jobs4                              perfstat
stalled-cycles-frontend     246,701,197,429 (  38.12%)  280,076,030,886 (  43.29%)
stalled-cycles-backend      119,050,341,112 (  18.40%)  110,955,641,671 (  17.15%)
instructions                822,716,962,127 (    1.27)  825,536,969,320 (    1.28)
branches                    160,590,028,545 ( 688.614)  161,152,996,915 ( 691.068)
branch-misses                   650,295,287 (   0.40%)      550,229,113 (   0.34%)
jobs5                              perfstat
stalled-cycles-frontend     298,958,462,516 (  38.30%)  344,852,200,358 (  44.16%)
stalled-cycles-backend      137,558,742,122 (  17.62%)  129,465,067,102 (  16.58%)
instructions              1,005,714,688,752 (    1.29) 1,007,657,999,432 (    1.29)
branches                    195,988,773,962 ( 697.730)  196,446,873,984 ( 700.319)
branch-misses                   695,818,940 (   0.36%)      624,823,263 (   0.32%)
jobs6                              perfstat
stalled-cycles-frontend     334,497,602,856 (  36.71%)  387,590,419,779 (  42.38%)
stalled-cycles-backend      163,539,365,335 (  17.95%)  152,640,193,639 (  16.69%)
instructions              1,184,738,177,851 (    1.30) 1,187,396,281,677 (    1.30)
branches                    230,592,915,640 ( 702.902)  231,253,802,882 ( 702.356)
branch-misses                   747,934,786 (   0.32%)      643,902,424 (   0.28%)
jobs7                              perfstat
stalled-cycles-frontend     396,724,684,187 (  37.71%)  460,705,858,952 (  43.84%)
stalled-cycles-backend      188,096,616,496 (  17.88%)  175,785,787,036 (  16.73%)
instructions              1,364,041,136,608 (    1.30) 1,366,689,075,112 (    1.30)
branches                    265,253,096,936 ( 700.078)  265,890,524,883 ( 702.839)
branch-misses                   784,991,589 (   0.30%)      729,196,689 (   0.27%)
jobs8                              perfstat
stalled-cycles-frontend     440,248,299,870 (  36.92%)  509,554,793,816 (  42.46%)
stalled-cycles-backend      222,575,930,616 (  18.67%)  213,401,248,432 (  17.78%)
instructions              1,542,262,045,114 (    1.29) 1,545,233,932,257 (    1.29)
branches                    299,775,178,439 ( 697.666)  300,528,458,505 ( 694.769)
branch-misses                   847,496,084 (   0.28%)      748,794,308 (   0.25%)
jobs9                              perfstat
stalled-cycles-frontend     506,269,882,480 (  37.86%)  592,798,032,820 (  44.43%)
stalled-cycles-backend      253,192,498,861 (  18.93%)  233,727,666,185 (  17.52%)
instructions              1,721,985,080,913 (    1.29) 1,724,666,236,005 (    1.29)
branches                    334,517,360,255 ( 694.134)  335,199,758,164 ( 697.131)
branch-misses                   873,496,730 (   0.26%)      815,379,236 (   0.24%)
jobs10                             perfstat
stalled-cycles-frontend     549,063,363,749 (  37.18%)  651,302,376,662 (  43.61%)
stalled-cycles-backend      281,680,986,810 (  19.07%)  277,005,235,582 (  18.55%)
instructions              1,901,859,271,180 (    1.29) 1,906,311,064,230 (    1.28)
branches                    369,398,536,153 ( 694.004)  370,527,696,358 ( 688.409)
branch-misses                   967,929,335 (   0.26%)      890,125,056 (   0.24%)

                            BASE           PATCHED
seconds elapsed        79.421641008 78.735285546
seconds elapsed        61.471246133 60.869085949
seconds elapsed        62.317058173 62.224188495
seconds elapsed        60.030739363 60.081102518
seconds elapsed        74.070398362 74.317582865
seconds elapsed        84.985953007 85.414364176
seconds elapsed        97.724553255 98.173311344
seconds elapsed        109.488066758 110.268399318
seconds elapsed        122.768189405 122.967164498
seconds elapsed        135.130035105 136.934770801

On my other system (8 x86_64 CPUs, short version of test results):

                            BASE           PATCHED
seconds elapsed        19.518065994 19.806320662
seconds elapsed        15.172772749 15.594718291
seconds elapsed        13.820925970 13.821708564
seconds elapsed        13.293097816 14.585206405
seconds elapsed        16.207284118 16.064431606
seconds elapsed        17.958376158 17.771825767
seconds elapsed        19.478009164 19.602961508
seconds elapsed        21.347152811 21.352318709
seconds elapsed        24.478121126 24.171088735
seconds elapsed        26.865057442 26.767327618

So performance-wise the numbers are quite similar.

[1] http://marc.info/?l=linux-kernel&m=144480832108927&w=2
[2] http://marc.info/?l=linux-kernel&m=145379613507518&w=2
[3] https://github.com/sergey-senozhatsky/zram-perf-test

Signed-off-by: Sergey Senozhatsky <[hidden email]>
Suggested-by: Minchan Kim <[hidden email]>
Suggested-by: Joonsoo Kim <[hidden email]>
---
 drivers/block/zram/Kconfig    | 10 +++----
 drivers/block/zram/zcomp.c    | 66 ++++++++++++++++++++++++++++---------------
 drivers/block/zram/zcomp.h    |  7 +++--
 drivers/block/zram/zram_drv.c | 14 +++++----
 4 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 386ba3d..2252cd7 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -1,8 +1,7 @@
 config ZRAM
  tristate "Compressed RAM block device support"
- depends on BLOCK && SYSFS && ZSMALLOC
- select LZO_COMPRESS
- select LZO_DECOMPRESS
+ depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
+ select CRYPTO_LZO
  default n
  help
   Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
@@ -18,9 +17,8 @@ config ZRAM
 config ZRAM_LZ4_COMPRESS
  bool "Enable LZ4 algorithm support"
  depends on ZRAM
- select LZ4_COMPRESS
- select LZ4_DECOMPRESS
+ select CRYPTO_LZ4
  default n
  help
   This option enables LZ4 compression algorithm support. Compression
-  algorithm can be changed using `comp_algorithm' device attribute.
\ No newline at end of file
+  algorithm can be changed using `comp_algorithm' device attribute.
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 400f826..82b568e 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -14,26 +14,23 @@
 #include <linux/wait.h>
 #include <linux/sched.h>
 #include <linux/cpu.h>
+#include <linux/crypto.h>
 
 #include "zcomp.h"
-#include "zcomp_lzo.h"
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
-#include "zcomp_lz4.h"
-#endif
 
-static struct zcomp_backend *backends[] = {
- &zcomp_lzo,
+static const char * const backends[] = {
+ "lzo",
 #ifdef CONFIG_ZRAM_LZ4_COMPRESS
- &zcomp_lz4,
+ "lz4",
 #endif
  NULL
 };
 
-static struct zcomp_backend *find_backend(const char *compress)
+static const char *find_backend(const char *compress)
 {
  int i = 0;
  while (backends[i]) {
- if (sysfs_streq(compress, backends[i]->name))
+ if (sysfs_streq(compress, backends[i]))
  break;
  i++;
  }
@@ -42,8 +39,8 @@ static struct zcomp_backend *find_backend(const char *compress)
 
 static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
 {
- if (zstrm->private)
- comp->backend->destroy(zstrm->private);
+ if (!IS_ERR_OR_NULL(zstrm->private))
+ crypto_free_comp(zstrm->private);
  free_pages((unsigned long)zstrm->buffer, 1);
  kfree(zstrm);
 }
@@ -58,13 +55,13 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags)
  if (!zstrm)
  return NULL;
 
- zstrm->private = comp->backend->create(flags);
+ zstrm->private = crypto_alloc_comp(comp->name, 0, 0);
  /*
  * 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(flags | __GFP_ZERO, 1);
- if (!zstrm->private || !zstrm->buffer) {
+ if (IS_ERR_OR_NULL(zstrm->private) || !zstrm->buffer) {
  zcomp_strm_free(comp, zstrm);
  zstrm = NULL;
  }
@@ -78,12 +75,12 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
  int i = 0;
 
  while (backends[i]) {
- if (!strcmp(comp, backends[i]->name))
+ if (!strcmp(comp, backends[i]))
  sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
- "[%s] ", backends[i]->name);
+ "[%s] ", backends[i]);
  else
  sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
- "%s ", backends[i]->name);
+ "%s ", backends[i]);
  i++;
  }
  sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
@@ -106,16 +103,39 @@ void zcomp_stream_put(struct zcomp *comp)
 }
 
 int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, size_t *dst_len)
+ const unsigned char *src, unsigned int *dst_len)
 {
- return comp->backend->compress(src, zstrm->buffer, dst_len,
- zstrm->private);
+ /*
+ * Our dst memory (zstrm->buffer) is always `2 * PAGE_SIZE' sized
+ * because sometimes we can endup having a bigger compressed data
+ * due to various reasons: for example compression algorithms tend
+ * to add some padding to the compressed buffer. Speaking of padding,
+ * comp algorithm `842' pads the compressed length to multiple of 8
+ * and returns -ENOSP when the dst memory is not big enough, which
+ * is not something that ZRAM wants to see. We can handle the
+ * `compressed_size > PAGE_SIZE' case easily in ZRAM, but when we
+ * receive -ERRNO from the compressing backend we can't help it
+ * anymore. To make `842' happy we need to tell the exact size of
+ * the dst buffer, zram_drv will take care of the fact that
+ * compressed buffer is too big.
+ */
+ *dst_len = PAGE_SIZE * 2;
+
+ return crypto_comp_compress(zstrm->private,
+ src, PAGE_SIZE,
+ zstrm->buffer, dst_len);
 }
 
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
+int zcomp_decompress(struct zcomp *comp,
+ struct zcomp_strm *zstrm,
+ const unsigned char *src,
  size_t src_len, unsigned char *dst)
 {
- return comp->backend->decompress(src, src_len, dst);
+ unsigned int dst_len = PAGE_SIZE;
+
+ return crypto_comp_decompress(zstrm->private,
+ src, src_len,
+ dst, &dst_len);
 }
 
 static int __zcomp_cpu_notifier(struct zcomp *comp,
@@ -209,7 +229,7 @@ void zcomp_destroy(struct zcomp *comp)
 struct zcomp *zcomp_create(const char *compress)
 {
  struct zcomp *comp;
- struct zcomp_backend *backend;
+ const char *backend;
  int error;
 
  backend = find_backend(compress);
@@ -220,7 +240,7 @@ struct zcomp *zcomp_create(const char *compress)
  if (!comp)
  return ERR_PTR(-ENOMEM);
 
- comp->backend = backend;
+ comp->name = backend;
  error = zcomp_init(comp);
  if (error) {
  kfree(comp);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 944b8e6..ba36bde 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -40,6 +40,8 @@ struct zcomp {
  struct zcomp_strm * __percpu *stream;
  struct zcomp_backend *backend;
  struct notifier_block notifier;
+
+ const char *name;
 };
 
 ssize_t zcomp_available_show(const char *comp, char *buf);
@@ -52,9 +54,10 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
 void zcomp_stream_put(struct zcomp *comp);
 
 int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
- const unsigned char *src, size_t *dst_len);
+ const unsigned char *src, unsigned int *dst_len);
 
-int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
+int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+ const unsigned char *src,
  size_t src_len, unsigned char *dst);
 
 bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 9361a5d..fa22a32 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -576,10 +576,14 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
  }
 
  cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_RO);
- if (size == PAGE_SIZE)
+ if (size == PAGE_SIZE) {
  copy_page(mem, cmem);
- else
- ret = zcomp_decompress(zram->comp, cmem, size, mem);
+ } else {
+ struct zcomp_strm *zstrm = zcomp_stream_get(zram->comp);
+
+ ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
+ zcomp_stream_put(zram->comp);
+ }
  zs_unmap_object(meta->mem_pool, handle);
  bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
 
@@ -646,7 +650,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
    int offset)
 {
  int ret = 0;
- size_t clen;
+ unsigned int clen;
  unsigned long handle = 0;
  struct page *page;
  unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
@@ -744,7 +748,7 @@ compress_again:
  if (handle)
  goto compress_again;
 
- pr_err("Error allocating memory for compressed page: %u, size=%zu\n",
+ pr_err("Error allocating memory for compressed page: %u, size=%u\n",
  index, clen);
  ret = -ENOMEM;
  goto out;
--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

[PATCH 5/7] zram: use crypto api to check alg availability

Sergey Senozhatsky
In reply to this post by Sergey Senozhatsky
There is no way to get a string with all the crypto comp
algorithms supported by the crypto comp engine, so we need
to maintain our own backends list. At the same time we
additionally need to use crypto_has_comp() to make sure
that the user has requested a compression algorithm that is
recognized by the crypto comp engine. Relying on /proc/crypto
is not an options here, because it does not show not-yet-inserted
compression modules.

Example:

 modprobe zram
 cat /proc/crypto | grep -i lz4
 modprobe lz4
 cat /proc/crypto | grep -i lz4
name         : lz4
driver       : lz4-generic
module       : lz4

So the user can't tell exactly if the lz4 is really supported
from /proc/crypto output, unless someone or something has loaded
it.

This patch also adds crypto_has_comp() to zcomp_available_show().
We store all the compression algorithms names in zcomp's `backends'
array, regardless the CONFIG_CRYPTO_FOO configuration, but show
only those that are also supported by crypto engine. This helps
user to know the exact list of compression algorithms that can be
used.

Example:
  module lz4 is not loaded yet, but is supported by the crypto
  engine. /proc/crypto has no information on this module, while
  zram's `comp_algorithm' lists it:

 cat /proc/crypto | grep -i lz4

 cat /sys/block/zram0/comp_algorithm
[lzo] lz4 deflate lz4hc 842

Signed-off-by: Sergey Senozhatsky <[hidden email]>
Cc: Minchan Kim <[hidden email]>
Cc: Joonsoo Kim <[hidden email]>
---
 drivers/block/zram/zcomp.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 79b30d7..a8593e9 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -29,12 +29,16 @@ static const char * const backends[] = {
 static const char *find_backend(const char *compress)
 {
  int i = 0;
+
  while (backends[i]) {
  if (sysfs_streq(compress, backends[i]))
  break;
  i++;
  }
- return backends[i];
+
+ if (backends[i] && crypto_has_comp(backends[i], 0, 0))
+ return backends[i];
+ return NULL;
 }
 
 static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
@@ -74,14 +78,16 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
  ssize_t sz = 0;
  int i = 0;
 
- while (backends[i]) {
+ for (; backends[i]; i++) {
+ if (!crypto_has_comp(backends[i], 0, 0))
+ continue;
+
  if (!strcmp(comp, backends[i]))
  sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
  "[%s] ", backends[i]);
  else
  sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
  "%s ", backends[i]);
- i++;
  }
  sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n");
  return sz;
--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

[PATCH 6/7] zram: delete custom lzo/lz4

Sergey Senozhatsky
In reply to this post by Sergey Senozhatsky
Remove lzo/lz4 backends, we use crypto API now.

Signed-off-by: Sergey Senozhatsky <[hidden email]>
Cc: Minchan Kim <[hidden email]>
Cc: Joonsoo Kim <[hidden email]>
---
 drivers/block/zram/Kconfig     |  9 -------
 drivers/block/zram/Makefile    |  4 +--
 drivers/block/zram/zcomp.c     |  2 --
 drivers/block/zram/zcomp.h     | 15 -----------
 drivers/block/zram/zcomp_lz4.c | 56 ------------------------------------------
 drivers/block/zram/zcomp_lz4.h | 17 -------------
 drivers/block/zram/zcomp_lzo.c | 56 ------------------------------------------
 drivers/block/zram/zcomp_lzo.h | 17 -------------
 8 files changed, 1 insertion(+), 175 deletions(-)
 delete mode 100644 drivers/block/zram/zcomp_lz4.c
 delete mode 100644 drivers/block/zram/zcomp_lz4.h
 delete mode 100644 drivers/block/zram/zcomp_lzo.c
 delete mode 100644 drivers/block/zram/zcomp_lzo.h

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 2252cd7..b8ecba6 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -13,12 +13,3 @@ config ZRAM
   disks and maybe many more.
 
   See zram.txt for more information.
-
-config ZRAM_LZ4_COMPRESS
- bool "Enable LZ4 algorithm support"
- depends on ZRAM
- select CRYPTO_LZ4
- default n
- help
-  This option enables LZ4 compression algorithm support. Compression
-  algorithm can be changed using `comp_algorithm' device attribute.
diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
index be0763f..9e2b79e 100644
--- a/drivers/block/zram/Makefile
+++ b/drivers/block/zram/Makefile
@@ -1,5 +1,3 @@
-zram-y := zcomp_lzo.o zcomp.o zram_drv.o
-
-zram-$(CONFIG_ZRAM_LZ4_COMPRESS) += zcomp_lz4.o
+zram-y := zcomp.o zram_drv.o
 
 obj-$(CONFIG_ZRAM) += zram.o
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index a8593e9..5ec0eb2 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -20,9 +20,7 @@
 
 static const char * const backends[] = {
  "lzo",
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
  "lz4",
-#endif
  NULL
 };
 
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index dcc0951..3cb13f4 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -21,24 +21,9 @@ struct zcomp_strm {
  void *private;
 };
 
-/* 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)(gfp_t flags);
- void (*destroy)(void *private);
-
- const char *name;
-};
-
 /* dynamic per-device compression frontend */
 struct zcomp {
  struct zcomp_strm * __percpu *stream;
- struct zcomp_backend *backend;
  struct notifier_block notifier;
 
  const char *name;
diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c
deleted file mode 100644
index 0110086..0000000
--- a/drivers/block/zram/zcomp_lz4.c
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/lz4.h>
-#include <linux/vmalloc.h>
-#include <linux/mm.h>
-
-#include "zcomp_lz4.h"
-
-static void *zcomp_lz4_create(gfp_t flags)
-{
- void *ret;
-
- ret = kmalloc(LZ4_MEM_COMPRESS, flags);
- if (!ret)
- ret = __vmalloc(LZ4_MEM_COMPRESS,
- flags | __GFP_HIGHMEM,
- PAGE_KERNEL);
- return ret;
-}
-
-static void zcomp_lz4_destroy(void *private)
-{
- kvfree(private);
-}
-
-static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private)
-{
- /* return  : Success if return 0 */
- return lz4_compress(src, PAGE_SIZE, dst, dst_len, private);
-}
-
-static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
-{
- size_t dst_len = PAGE_SIZE;
- /* return  : Success if return 0 */
- return lz4_decompress_unknownoutputsize(src, src_len, dst, &dst_len);
-}
-
-struct zcomp_backend zcomp_lz4 = {
- .compress = zcomp_lz4_compress,
- .decompress = zcomp_lz4_decompress,
- .create = zcomp_lz4_create,
- .destroy = zcomp_lz4_destroy,
- .name = "lz4",
-};
diff --git a/drivers/block/zram/zcomp_lz4.h b/drivers/block/zram/zcomp_lz4.h
deleted file mode 100644
index 60613fb..0000000
--- a/drivers/block/zram/zcomp_lz4.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _ZCOMP_LZ4_H_
-#define _ZCOMP_LZ4_H_
-
-#include "zcomp.h"
-
-extern struct zcomp_backend zcomp_lz4;
-
-#endif /* _ZCOMP_LZ4_H_ */
diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c
deleted file mode 100644
index ed7a1f0..0000000
--- a/drivers/block/zram/zcomp_lzo.c
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/lzo.h>
-#include <linux/vmalloc.h>
-#include <linux/mm.h>
-
-#include "zcomp_lzo.h"
-
-static void *lzo_create(gfp_t flags)
-{
- void *ret;
-
- ret = kmalloc(LZO1X_MEM_COMPRESS, flags);
- if (!ret)
- ret = __vmalloc(LZO1X_MEM_COMPRESS,
- flags | __GFP_HIGHMEM,
- PAGE_KERNEL);
- return ret;
-}
-
-static void lzo_destroy(void *private)
-{
- kvfree(private);
-}
-
-static int lzo_compress(const unsigned char *src, unsigned char *dst,
- size_t *dst_len, void *private)
-{
- int ret = lzo1x_1_compress(src, PAGE_SIZE, dst, dst_len, private);
- return ret == LZO_E_OK ? 0 : ret;
-}
-
-static int lzo_decompress(const unsigned char *src, size_t src_len,
- unsigned char *dst)
-{
- size_t dst_len = PAGE_SIZE;
- int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len);
- return ret == LZO_E_OK ? 0 : ret;
-}
-
-struct zcomp_backend zcomp_lzo = {
- .compress = lzo_compress,
- .decompress = lzo_decompress,
- .create = lzo_create,
- .destroy = lzo_destroy,
- .name = "lzo",
-};
diff --git a/drivers/block/zram/zcomp_lzo.h b/drivers/block/zram/zcomp_lzo.h
deleted file mode 100644
index 128c580..0000000
--- a/drivers/block/zram/zcomp_lzo.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2014 Sergey Senozhatsky.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _ZCOMP_LZO_H_
-#define _ZCOMP_LZO_H_
-
-#include "zcomp.h"
-
-extern struct zcomp_backend zcomp_lzo;
-
-#endif /* _ZCOMP_LZO_H_ */
--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

[PATCH 7/7] zram: add more compression algorithms

Sergey Senozhatsky
In reply to this post by Sergey Senozhatsky
Add "deflate", "lz4hc", "842" algorithms to the list of
known compression backends. The real availability of those
algorithms, however, depends on the corresponding
CONFIG_CRYPTO_FOO config options.

Signed-off-by: Sergey Senozhatsky <[hidden email]>
Cc: Minchan Kim <[hidden email]>
Cc: Joonsoo Kim <[hidden email]>
---
 drivers/block/zram/zcomp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 5ec0eb2..d7d65eb 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -21,6 +21,9 @@
 static const char * const backends[] = {
  "lzo",
  "lz4",
+ "deflate",
+ "lz4hc",
+ "842",
  NULL
 };
 
--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/7] zram: align zcomp interface to crypto comp API

Sergey Senozhatsky
In reply to this post by Sergey Senozhatsky
A cosmetic change: use the same datatypes as crypto API does.

Signed-off-by: Sergey Senozhatsky <[hidden email]>
Cc: Minchan Kim <[hidden email]>
Cc: Joonsoo Kim <[hidden email]>
---
 drivers/block/zram/zcomp.c    | 5 ++---
 drivers/block/zram/zcomp.h    | 5 ++---
 drivers/block/zram/zram_drv.c | 2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 6e16fa2..79b30d7 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -103,7 +103,7 @@ void zcomp_stream_put(struct zcomp *comp)
 }
 
 int zcomp_compress(struct zcomp_strm *zstrm,
- const unsigned char *src, unsigned int *dst_len)
+ const u8 *src, unsigned int *dst_len)
 {
  /*
  * Our dst memory (zstrm->buffer) is always `2 * PAGE_SIZE' sized
@@ -127,8 +127,7 @@ int zcomp_compress(struct zcomp_strm *zstrm,
 }
 
 int zcomp_decompress(struct zcomp_strm *zstrm,
- const unsigned char *src,
- size_t src_len, unsigned char *dst)
+ const u8 *src, unsigned int src_len, u8 *dst)
 {
  unsigned int dst_len = PAGE_SIZE;
 
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 3fb9dc4..dcc0951 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -54,11 +54,10 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
 void zcomp_stream_put(struct zcomp *comp);
 
 int zcomp_compress(struct zcomp_strm *zstrm,
- const unsigned char *src, unsigned int *dst_len);
+ const u8 *src, unsigned int *dst_len);
 
 int zcomp_decompress(struct zcomp_strm *zstrm,
- const unsigned char *src,
- size_t src_len, unsigned char *dst);
+ const u8 *src, unsigned int src_len, u8 *dst);
 
 bool 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 99c8be7..65d1403 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -563,7 +563,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
  unsigned char *cmem;
  struct zram_meta *meta = zram->meta;
  unsigned long handle;
- size_t size;
+ unsigned int size;
 
  bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
  handle = meta->table[index].handle;
--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/7] zram: drop zcomp param from compress/decompress

Sergey Senozhatsky
In reply to this post by Sergey Senozhatsky
We don't need to supply a zcomp pointer to compress/decompress
functions, drop it.

Signed-off-by: Sergey Senozhatsky <[hidden email]>
Cc: Minchan Kim <[hidden email]>
Cc: Joonsoo Kim <[hidden email]>
---
 drivers/block/zram/zcomp.c    | 5 ++---
 drivers/block/zram/zcomp.h    | 4 ++--
 drivers/block/zram/zram_drv.c | 4 ++--
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 82b568e..6e16fa2 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -102,7 +102,7 @@ void zcomp_stream_put(struct zcomp *comp)
  put_cpu_ptr(comp->stream);
 }
 
-int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
+int zcomp_compress(struct zcomp_strm *zstrm,
  const unsigned char *src, unsigned int *dst_len)
 {
  /*
@@ -126,8 +126,7 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
  zstrm->buffer, dst_len);
 }
 
-int zcomp_decompress(struct zcomp *comp,
- struct zcomp_strm *zstrm,
+int zcomp_decompress(struct zcomp_strm *zstrm,
  const unsigned char *src,
  size_t src_len, unsigned char *dst)
 {
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index ba36bde..3fb9dc4 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -53,10 +53,10 @@ void zcomp_destroy(struct zcomp *comp);
 struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
 void zcomp_stream_put(struct zcomp *comp);
 
-int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
+int zcomp_compress(struct zcomp_strm *zstrm,
  const unsigned char *src, unsigned int *dst_len);
 
-int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
+int zcomp_decompress(struct zcomp_strm *zstrm,
  const unsigned char *src,
  size_t src_len, unsigned char *dst);
 
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index fa22a32..99c8be7 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -581,7 +581,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
  } else {
  struct zcomp_strm *zstrm = zcomp_stream_get(zram->comp);
 
- ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem);
+ ret = zcomp_decompress(zstrm, cmem, size, mem);
  zcomp_stream_put(zram->comp);
  }
  zs_unmap_object(meta->mem_pool, handle);
@@ -700,7 +700,7 @@ compress_again:
  }
 
  zstrm = zcomp_stream_get(zram->comp);
- ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen);
+ ret = zcomp_compress(zstrm, uncmem, &clen);
  if (!is_partial_io(bvec)) {
  kunmap_atomic(user_mem);
  user_mem = NULL;
--
2.8.3.394.g3916adf

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] zram: switch to crypto api

Joonsoo Kim-2
In reply to this post by Sergey Senozhatsky
On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote:

> Hello,
>
>   This has started as a 'add zlib support' work, but after some
> thinking I saw no blockers for a bigger change -- a switch to
> crypto API.
>
>   We don't have an idle zstreams list anymore and our write path
> now works absolutely differently, preventing preemption during
> compression. This removes possibilities of read paths preempting
> writes at wrong places and opens the door for a move from custom
> LZO/LZ4 compression backends implementation to a more generic one,
> using crypto compress API.

Hello, Sergey.

I don't look at each patches deeply but nice work! I didn't notice that
rececnt zram changes makes thing simpler. :)

Thanks.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/7] zram: rename zstrm find-release functions

Minchan Kim-2
In reply to this post by Sergey Senozhatsky
On Wed, May 25, 2016 at 11:30:00PM +0900, Sergey Senozhatsky wrote:
> We don't perform any zstream idle list lookup anymore, so
> zcomp_strm_find()/zcomp_strm_release() names are not
> representative.
>
> Rename to zcomp_stream_get()/zcomp_stream_put().

Actually, I wanted it when we applied percpu but didn't say to you because

1. It's preference of author.

Frankly speaking, I prefer get to find but you might think different
with me so I want to respect patch author's right if it's not huge pain
to me. :)
Now I realized you were on same page.

2. We might roll back to stream list.

In that case, find is proper word again but it's too trivial.
So, I want to merge this patch regardless of this patchset. :)

>
> Signed-off-by: Sergey Senozhatsky <[hidden email]>
> Cc: Minchan Kim <[hidden email]>
Acked-by: Minchan Kim <[hidden email]>
 
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] zram: switch to crypto api

Minchan Kim-2
In reply to this post by Sergey Senozhatsky
On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote:

> Hello,
>
>   This has started as a 'add zlib support' work, but after some
> thinking I saw no blockers for a bigger change -- a switch to
> crypto API.
>
>   We don't have an idle zstreams list anymore and our write path
> now works absolutely differently, preventing preemption during
> compression. This removes possibilities of read paths preempting
> writes at wrong places and opens the door for a move from custom
> LZO/LZ4 compression backends implementation to a more generic one,
> using crypto compress API.
>
>   This patch set also eliminates the need of a new context-less
> crypto API interface, which was quite hard to sell, so we can
> move along faster.

Super fast Sergey.
At a first glance, patchset looks nice.
I will review as soon as possible.

Thanks a lot!

>
>
> Sergey Senozhatsky (7):
>   zram: rename zstrm find-release functions
>   zram: switch to crypto compress API
>   zram: drop zcomp param from compress/decompress
>   zram: align zcomp interface to crypto comp API
>   zram: use crypto api to check alg availability
>   zram: delete custom lzo/lz4
>   zram: add more compression algorithms
>
>  drivers/block/zram/Kconfig     | 15 +------
>  drivers/block/zram/Makefile    |  4 +-
>  drivers/block/zram/zcomp.c     | 91 +++++++++++++++++++++++++++---------------
>  drivers/block/zram/zcomp.h     | 29 ++++----------
>  drivers/block/zram/zcomp_lz4.c | 56 --------------------------
>  drivers/block/zram/zcomp_lz4.h | 17 --------
>  drivers/block/zram/zcomp_lzo.c | 56 --------------------------
>  drivers/block/zram/zcomp_lzo.h | 17 --------
>  drivers/block/zram/zram_drv.c  | 26 +++++++-----
>  9 files changed, 84 insertions(+), 227 deletions(-)
>  delete mode 100644 drivers/block/zram/zcomp_lz4.c
>  delete mode 100644 drivers/block/zram/zcomp_lz4.h
>  delete mode 100644 drivers/block/zram/zcomp_lzo.c
>  delete mode 100644 drivers/block/zram/zcomp_lzo.h
>
> --
> 2.8.3.394.g3916adf
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/7] zram: rename zstrm find-release functions

Sergey Senozhatsky-3
In reply to this post by Minchan Kim-2
On (05/26/16 09:44), Minchan Kim wrote:

> On Wed, May 25, 2016 at 11:30:00PM +0900, Sergey Senozhatsky wrote:
> > We don't perform any zstream idle list lookup anymore, so
> > zcomp_strm_find()/zcomp_strm_release() names are not
> > representative.
> >
> > Rename to zcomp_stream_get()/zcomp_stream_put().
>
> Actually, I wanted it when we applied percpu but didn't say to you because
>
> 1. It's preference of author.
>
> Frankly speaking, I prefer get to find but you might think different
> with me so I want to respect patch author's right if it's not huge pain
> to me. :)
> Now I realized you were on same page.
>
> 2. We might roll back to stream list.
>
> In that case, find is proper word again but it's too trivial.
> So, I want to merge this patch regardless of this patchset. :)
>
> >
> > Signed-off-by: Sergey Senozhatsky <[hidden email]>
> > Cc: Minchan Kim <[hidden email]>
> Acked-by: Minchan Kim <[hidden email]>

thanks.

        -ss
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] zram: switch to crypto api

Sergey Senozhatsky-3
In reply to this post by Minchan Kim-2
On (05/26/16 09:52), Minchan Kim wrote:

> On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote:
> > Hello,
> >
> >   This has started as a 'add zlib support' work, but after some
> > thinking I saw no blockers for a bigger change -- a switch to
> > crypto API.
> >
> >   We don't have an idle zstreams list anymore and our write path
> > now works absolutely differently, preventing preemption during
> > compression. This removes possibilities of read paths preempting
> > writes at wrong places and opens the door for a move from custom
> > LZO/LZ4 compression backends implementation to a more generic one,
> > using crypto compress API.
> >
> >   This patch set also eliminates the need of a new context-less
> > crypto API interface, which was quite hard to sell, so we can
> > move along faster.
>
> Super fast Sergey.

hahaha :)

> At a first glance, patchset looks nice.
> I will review as soon as possible.
>
> Thanks a lot!

thanks!

        -ss
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] zram: switch to crypto api

Sergey Senozhatsky-3
In reply to this post by Joonsoo Kim-2
On (05/26/16 09:43), Joonsoo Kim wrote:
[..]
> Hello, Sergey.
>
> I don't look at each patches deeply but nice work! I didn't notice that
> rececnt zram changes makes thing simpler. :)

Hello Joonsoo,

thanks.

I owe you a drink for pushing it in the context-less crypto
API direction. sorry about that.

        -ss
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] zram: switch to crypto api

Joonsoo Kim-2
On Thu, May 26, 2016 at 10:12:16AM +0900, Sergey Senozhatsky wrote:

> On (05/26/16 09:43), Joonsoo Kim wrote:
> [..]
> > Hello, Sergey.
> >
> > I don't look at each patches deeply but nice work! I didn't notice that
> > rececnt zram changes makes thing simpler. :)
>
> Hello Joonsoo,
>
> thanks.
>
> I owe you a drink for pushing it in the context-less crypto
> API direction. sorry about that.

Good to hear. I will wait for your invitation. :)

Thanks.