* [PATCH v2 0/8] zram: switch to crypto api
@ 2016-05-31 12:20 Sergey Senozhatsky
2016-05-31 12:20 ` [PATCH v2 1/8] zram: rename zstrm find-release functions Sergey Senozhatsky
` (8 more replies)
0 siblings, 9 replies; 29+ messages in thread
From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky,
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.
v2:
-- addressed Minchan's review points
-- allow out-of-tree comp algorithms, per Minchan
-- some other cleanups, reworks and improvements
Sergey Senozhatsky (8):
zram: rename zstrm find-release functions
zram: switch to crypto compress API
zram: align zcomp interface to crypto comp API
zram: use crypto api to check alg availability
zram: cosmetic: cleanup documentation
zram: delete custom lzo/lz4
zram: add more compression algorithms
zram: drop gfp_t from zcomp_strm_alloc()
Documentation/blockdev/zram.txt | 82 +++++++++++++----------
drivers/block/zram/Kconfig | 15 +----
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 143 ++++++++++++++++++++++++----------------
drivers/block/zram/zcomp.h | 36 +++-------
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 | 42 +++++++-----
drivers/block/zram/zram_drv.h | 5 +-
11 files changed, 171 insertions(+), 302 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
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH v2 1/8] zram: rename zstrm find-release functions 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 2/8] zram: switch to crypto compress API Sergey Senozhatsky ` (7 subsequent siblings) 8 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, 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 <sergey.senozhatsky@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Acked-by: Minchan Kim <minchan@kernel.org> --- 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 2/8] zram: switch to crypto compress API 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 1/8] zram: rename zstrm find-release functions Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-05-31 23:40 ` Minchan Kim 2016-05-31 23:44 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 3/8] zram: align zcomp interface to crypto comp API Sergey Senozhatsky ` (6 subsequent siblings) 8 siblings, 2 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, 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. Without 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 <sergey.senozhatsky@gmail.com> Suggested-by: Minchan Kim <minchan@kernel.org> Suggested-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- drivers/block/zram/Kconfig | 10 +++--- drivers/block/zram/zcomp.c | 75 +++++++++++++++++++++++++++---------------- drivers/block/zram/zcomp.h | 16 ++++----- drivers/block/zram/zram_drv.c | 16 +++++---- 4 files changed, 68 insertions(+), 49 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..241d3c8 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -14,42 +14,39 @@ #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++; } return backends[i]; } -static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm) +static void zcomp_strm_free(struct zcomp_strm *zstrm) { - if (zstrm->private) - comp->backend->destroy(zstrm->private); + if (!IS_ERR_OR_NULL(zstrm->tfm)) + crypto_free_comp(zstrm->tfm); free_pages((unsigned long)zstrm->buffer, 1); kfree(zstrm); } /* - * allocate new zcomp_strm structure with ->private initialized by + * allocate new zcomp_strm structure with ->tfm initialized by * backend, return NULL on error */ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) @@ -58,14 +55,14 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) if (!zstrm) return NULL; - zstrm->private = comp->backend->create(flags); + zstrm->tfm = 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) { - zcomp_strm_free(comp, zstrm); + if (IS_ERR_OR_NULL(zstrm->tfm) || !zstrm->buffer) { + zcomp_strm_free(zstrm); zstrm = NULL; } return zstrm; @@ -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"); @@ -105,17 +102,39 @@ void zcomp_stream_put(struct zcomp *comp) put_cpu_ptr(comp->stream); } -int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, - const unsigned char *src, size_t *dst_len) +int zcomp_compress(struct zcomp_strm *zstrm, + 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->tfm, + src, PAGE_SIZE, + zstrm->buffer, dst_len); } -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, +int zcomp_decompress(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->tfm, + src, src_len, + dst, &dst_len); } static int __zcomp_cpu_notifier(struct zcomp *comp, @@ -138,7 +157,7 @@ static int __zcomp_cpu_notifier(struct zcomp *comp, case CPU_UP_CANCELED: zstrm = *per_cpu_ptr(comp->stream, cpu); if (!IS_ERR_OR_NULL(zstrm)) - zcomp_strm_free(comp, zstrm); + zcomp_strm_free(zstrm); *per_cpu_ptr(comp->stream, cpu) = NULL; break; default: @@ -209,7 +228,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 +239,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..5ad1ffe 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -13,12 +13,7 @@ struct zcomp_strm { /* compression/decompression buffer */ void *buffer; - /* - * The private data of the compression stream, only compression - * stream backend can touch this (e.g. compression algorithm - * working memory) - */ - void *private; + struct crypto_comp *tfm; }; /* static compression backend */ @@ -40,6 +35,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); @@ -51,10 +48,11 @@ 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, - const unsigned char *src, size_t *dst_len); +int zcomp_compress(struct zcomp_strm *zstrm, + const unsigned char *src, unsigned int *dst_len); -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, +int zcomp_decompress(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..99c8be7 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(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; @@ -696,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; @@ -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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/8] zram: switch to crypto compress API 2016-05-31 12:20 ` [PATCH v2 2/8] zram: switch to crypto compress API Sergey Senozhatsky @ 2016-05-31 23:40 ` Minchan Kim 2016-05-31 23:44 ` Minchan Kim 1 sibling, 0 replies; 29+ messages in thread From: Minchan Kim @ 2016-05-31 23:40 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, May 31, 2016 at 09:20:11PM +0900, Sergey Senozhatsky wrote: > 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. Without 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 <sergey.senozhatsky@gmail.com> > Suggested-by: Minchan Kim <minchan@kernel.org> > Suggested-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> Acked-by: Minchan Kim <minchan@kernel.org> ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/8] zram: switch to crypto compress API 2016-05-31 12:20 ` [PATCH v2 2/8] zram: switch to crypto compress API Sergey Senozhatsky 2016-05-31 23:40 ` Minchan Kim @ 2016-05-31 23:44 ` Minchan Kim 2016-06-01 1:17 ` Sergey Senozhatsky 1 sibling, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-31 23:44 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, May 31, 2016 at 09:20:11PM +0900, Sergey Senozhatsky wrote: <snip> trivial: One thing I got missed in review. > -int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, > - const unsigned char *src, size_t *dst_len); > +int zcomp_compress(struct zcomp_strm *zstrm, > + const unsigned char *src, unsigned int *dst_len); unsigned int for dst_len > > -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, > +int zcomp_decompress(struct zcomp_strm *zstrm, > + const unsigned char *src, > size_t src_len, unsigned char *dst); > size_t src_len? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/8] zram: switch to crypto compress API 2016-05-31 23:44 ` Minchan Kim @ 2016-06-01 1:17 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-06-01 1:17 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (06/01/16 08:44), Minchan Kim wrote: > <snip> > > trivial: > > One thing I got missed in review. > > > -int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, > > - const unsigned char *src, size_t *dst_len); > > +int zcomp_compress(struct zcomp_strm *zstrm, > > + const unsigned char *src, unsigned int *dst_len); > > unsigned int for dst_len > > > > > -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, > > +int zcomp_decompress(struct zcomp_strm *zstrm, > > + const unsigned char *src, > > size_t src_len, unsigned char *dst); > > > > size_t src_len? thanks for spotting it! -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 3/8] zram: align zcomp interface to crypto comp API 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 1/8] zram: rename zstrm find-release functions Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 2/8] zram: switch to crypto compress API Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-05-31 23:48 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 4/8] zram: use crypto api to check alg availability Sergey Senozhatsky ` (5 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, Sergey Senozhatsky A cosmetic change: update zcomp interface to be more aligned with the crypto API. Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- 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 241d3c8..f357268 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 void *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 void *src, unsigned int src_len, void *dst) { unsigned int dst_len = PAGE_SIZE; diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index 5ad1ffe..c914ab7 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -49,11 +49,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 void *src, unsigned int *dst_len); int zcomp_decompress(struct zcomp_strm *zstrm, - const unsigned char *src, - size_t src_len, unsigned char *dst); + const void *src, unsigned int src_len, void *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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/8] zram: align zcomp interface to crypto comp API 2016-05-31 12:20 ` [PATCH v2 3/8] zram: align zcomp interface to crypto comp API Sergey Senozhatsky @ 2016-05-31 23:48 ` Minchan Kim 2016-06-01 1:13 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-31 23:48 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, May 31, 2016 at 09:20:12PM +0900, Sergey Senozhatsky wrote: > A cosmetic change: > update zcomp interface to be more aligned with the crypto API. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Acked-by: Minchan Kim <minchan@kernel.org> Aha, you changed src_len in this patchset. :) ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/8] zram: align zcomp interface to crypto comp API 2016-05-31 23:48 ` Minchan Kim @ 2016-06-01 1:13 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-06-01 1:13 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (06/01/16 08:48), Minchan Kim wrote: > On Tue, May 31, 2016 at 09:20:12PM +0900, Sergey Senozhatsky wrote: > > A cosmetic change: > > update zcomp interface to be more aligned with the crypto API. > > > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > > Cc: Minchan Kim <minchan@kernel.org> > > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > > Acked-by: Minchan Kim <minchan@kernel.org> > > Aha, you changed src_len in this patchset. :) oh, thanks. hmm... I didn't want to add `cosmetic noise' to the 0002, but probably 0003 better be part of 0002. the patch is not so big so it won't complicate 0002 a lot. I'll ask Andrew to squash, or will squash on my side and resend the whole series. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky ` (2 preceding siblings ...) 2016-05-31 12:20 ` [PATCH v2 3/8] zram: align zcomp interface to crypto comp API Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-06-01 0:03 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 5/8] zram: cosmetic: cleanup documentation Sergey Senozhatsky ` (4 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, 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 We also now fully rely on crypto_has_comp() when configure a new device. The existing `backends' array is kept for user's convenience only -- there is no way to list all of the compression algorithms supported by crypto -- and is not guaranteed to contain every compression module name supported by the kernel. Switch to crypto_has_comp() has an advantage of permitting the usage of out-of-tree crypto compression modules (implementing S/W or H/W compression). Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- Documentation/blockdev/zram.txt | 11 ++++++++ drivers/block/zram/zcomp.c | 58 ++++++++++++++++++++++++----------------- drivers/block/zram/zram_drv.c | 16 +++++++----- drivers/block/zram/zram_drv.h | 5 ++-- 4 files changed, 57 insertions(+), 33 deletions(-) diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt index 13100fb..7c05357 100644 --- a/Documentation/blockdev/zram.txt +++ b/Documentation/blockdev/zram.txt @@ -83,6 +83,17 @@ pre-created. Default: 1. #select lzo compression algorithm echo lzo > /sys/block/zram0/comp_algorithm + For the time being, the `comp_algorithm' content does not necessarily + show every compression algorithm supported by the kernel. We keep this + list primarily to simplify device configuration and one can configure + a new device with a compression algorithm that is not listed in + `comp_algorithm'. The thing is that, internally, ZRAM uses Crypto API + and, if some of the algorithms were built as modules, it's impossible + to list all of them using, for instance, /proc/crypto or any other + method. This, however, has an advantage of permitting the usage of + custom crypto compression modules (implementing S/W or H/W + compression). + 4) 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. diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index f357268..2381ca9 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -26,17 +26,6 @@ static const char * const backends[] = { NULL }; -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]; -} - static void zcomp_strm_free(struct zcomp_strm *zstrm) { if (!IS_ERR_OR_NULL(zstrm->tfm)) @@ -68,30 +57,53 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) return zstrm; } +bool zcomp_available_algorithm(const char *comp) +{ + /* + * Crypto does not ignore a trailing new line symbol, + * so make sure you don't supply a string containing + * one. + * This also means that we keep `backends' array for + * zcomp_available_show() only and will init a new zram + * device with any compressing algorithm known to crypto + * api. + */ + return crypto_has_comp(comp, 0, 0) == 1; +} + /* show available compressors */ ssize_t zcomp_available_show(const char *comp, char *buf) { + bool known_algorithm = false; ssize_t sz = 0; int i = 0; - while (backends[i]) { - if (!strcmp(comp, backends[i])) + for (; backends[i]; i++) { + if (!zcomp_available_algorithm(backends[i])) + continue; + + if (!strcmp(comp, backends[i])) { + known_algorithm = true; sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, "[%s] ", backends[i]); - else + } else { sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, "%s ", backends[i]); - i++; + } } + + /* + * Out-of-tree module known to crypto api or a missing + * entry in `backends'. + */ + if (!known_algorithm && zcomp_available_algorithm(comp)) + sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, + "[%s] ", comp); + sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n"); return sz; } -bool zcomp_available_algorithm(const char *comp) -{ - return find_backend(comp) != NULL; -} - struct zcomp_strm *zcomp_stream_get(struct zcomp *comp) { return *get_cpu_ptr(comp->stream); @@ -227,18 +239,16 @@ void zcomp_destroy(struct zcomp *comp) struct zcomp *zcomp_create(const char *compress) { struct zcomp *comp; - const char *backend; int error; - backend = find_backend(compress); - if (!backend) + if (!zcomp_available_algorithm(compress)) return ERR_PTR(-EINVAL); comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL); if (!comp) return ERR_PTR(-ENOMEM); - comp->name = backend; + comp->name = compress; error = zcomp_init(comp); if (error) { kfree(comp); diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 65d1403..c2a1d7d 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -342,9 +342,16 @@ static ssize_t comp_algorithm_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t len) { struct zram *zram = dev_to_zram(dev); + char compressor[CRYPTO_MAX_ALG_NAME]; size_t sz; - if (!zcomp_available_algorithm(buf)) + strlcpy(compressor, buf, sizeof(compressor)); + /* ignore trailing newline */ + sz = strlen(compressor); + if (sz > 0 && compressor[sz - 1] == '\n') + compressor[sz - 1] = 0x00; + + if (!zcomp_available_algorithm(compressor)) return -EINVAL; down_write(&zram->init_lock); @@ -353,13 +360,8 @@ static ssize_t comp_algorithm_store(struct device *dev, pr_info("Can't change algorithm for initialized device\n"); return -EBUSY; } - strlcpy(zram->compressor, buf, sizeof(zram->compressor)); - - /* ignore trailing newline */ - sz = strlen(zram->compressor); - if (sz > 0 && zram->compressor[sz - 1] == '\n') - zram->compressor[sz - 1] = 0x00; + strlcpy(zram->compressor, compressor, sizeof(compressor)); up_write(&zram->init_lock); return len; } diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h index 3f5bf66..74fcf10 100644 --- a/drivers/block/zram/zram_drv.h +++ b/drivers/block/zram/zram_drv.h @@ -15,8 +15,9 @@ #ifndef _ZRAM_DRV_H_ #define _ZRAM_DRV_H_ -#include <linux/spinlock.h> +#include <linux/rwsem.h> #include <linux/zsmalloc.h> +#include <linux/crypto.h> #include "zcomp.h" @@ -113,7 +114,7 @@ struct zram { * we can store in a disk. */ u64 disksize; /* bytes */ - char compressor[10]; + char compressor[CRYPTO_MAX_ALG_NAME]; /* * zram is claimed so open request will be failed */ -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-05-31 12:20 ` [PATCH v2 4/8] zram: use crypto api to check alg availability Sergey Senozhatsky @ 2016-06-01 0:03 ` Minchan Kim 2016-06-01 1:07 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-06-01 0:03 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, May 31, 2016 at 09:20:13PM +0900, Sergey Senozhatsky wrote: > 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. So, if we do 'cat /sys/block/zram0/comp_algorithm", every crypto modules in the backend array are loaded in memory and not unloaded until admin executes rmmod? Right? > > 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 > > We also now fully rely on crypto_has_comp() when configure a new > device. The existing `backends' array is kept for user's convenience > only -- there is no way to list all of the compression algorithms > supported by crypto -- and is not guaranteed to contain every > compression module name supported by the kernel. Switch to > crypto_has_comp() has an advantage of permitting the usage of > out-of-tree crypto compression modules (implementing S/W or H/W > compression). If user load out-of-tree crypto compression module, what's status of comp_algorithm? #> insmod foo_crypto.ko #> echo foo > /sys/block/zram0/comp_algorithm #> cat /sys/block/zram0/comp_algorithm lzo lz4 [foo] ? > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > --- > Documentation/blockdev/zram.txt | 11 ++++++++ > drivers/block/zram/zcomp.c | 58 ++++++++++++++++++++++++----------------- > drivers/block/zram/zram_drv.c | 16 +++++++----- > drivers/block/zram/zram_drv.h | 5 ++-- > 4 files changed, 57 insertions(+), 33 deletions(-) > > diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt > index 13100fb..7c05357 100644 > --- a/Documentation/blockdev/zram.txt > +++ b/Documentation/blockdev/zram.txt > @@ -83,6 +83,17 @@ pre-created. Default: 1. > #select lzo compression algorithm > echo lzo > /sys/block/zram0/comp_algorithm > > + For the time being, the `comp_algorithm' content does not necessarily > + show every compression algorithm supported by the kernel. We keep this > + list primarily to simplify device configuration and one can configure > + a new device with a compression algorithm that is not listed in > + `comp_algorithm'. The thing is that, internally, ZRAM uses Crypto API > + and, if some of the algorithms were built as modules, it's impossible > + to list all of them using, for instance, /proc/crypto or any other > + method. This, however, has an advantage of permitting the usage of > + custom crypto compression modules (implementing S/W or H/W > + compression). > + > 4) 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. > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index f357268..2381ca9 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -26,17 +26,6 @@ static const char * const backends[] = { > NULL > }; > > -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]; > -} > - > static void zcomp_strm_free(struct zcomp_strm *zstrm) > { > if (!IS_ERR_OR_NULL(zstrm->tfm)) > @@ -68,30 +57,53 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) > return zstrm; > } > > +bool zcomp_available_algorithm(const char *comp) > +{ > + /* > + * Crypto does not ignore a trailing new line symbol, > + * so make sure you don't supply a string containing > + * one. > + * This also means that we keep `backends' array for > + * zcomp_available_show() only and will init a new zram > + * device with any compressing algorithm known to crypto > + * api. > + */ > + return crypto_has_comp(comp, 0, 0) == 1; > +} > + > /* show available compressors */ > ssize_t zcomp_available_show(const char *comp, char *buf) > { > + bool known_algorithm = false; > ssize_t sz = 0; > int i = 0; > > - while (backends[i]) { > - if (!strcmp(comp, backends[i])) > + for (; backends[i]; i++) { > + if (!zcomp_available_algorithm(backends[i])) > + continue; > + > + if (!strcmp(comp, backends[i])) { > + known_algorithm = true; > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > "[%s] ", backends[i]); > - else > + } else { > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > "%s ", backends[i]); > - i++; > + } > } > + > + /* > + * Out-of-tree module known to crypto api or a missing > + * entry in `backends'. > + */ > + if (!known_algorithm && zcomp_available_algorithm(comp)) > + sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > + "[%s] ", comp); > + > sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n"); > return sz; > } > > -bool zcomp_available_algorithm(const char *comp) > -{ > - return find_backend(comp) != NULL; > -} > - > struct zcomp_strm *zcomp_stream_get(struct zcomp *comp) > { > return *get_cpu_ptr(comp->stream); > @@ -227,18 +239,16 @@ void zcomp_destroy(struct zcomp *comp) > struct zcomp *zcomp_create(const char *compress) > { > struct zcomp *comp; > - const char *backend; > int error; > > - backend = find_backend(compress); > - if (!backend) > + if (!zcomp_available_algorithm(compress)) > return ERR_PTR(-EINVAL); > > comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL); > if (!comp) > return ERR_PTR(-ENOMEM); > > - comp->name = backend; > + comp->name = compress; > error = zcomp_init(comp); > if (error) { > kfree(comp); > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index 65d1403..c2a1d7d 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -342,9 +342,16 @@ static ssize_t comp_algorithm_store(struct device *dev, > struct device_attribute *attr, const char *buf, size_t len) > { > struct zram *zram = dev_to_zram(dev); > + char compressor[CRYPTO_MAX_ALG_NAME]; > size_t sz; > > - if (!zcomp_available_algorithm(buf)) > + strlcpy(compressor, buf, sizeof(compressor)); > + /* ignore trailing newline */ > + sz = strlen(compressor); > + if (sz > 0 && compressor[sz - 1] == '\n') > + compressor[sz - 1] = 0x00; > + > + if (!zcomp_available_algorithm(compressor)) > return -EINVAL; > > down_write(&zram->init_lock); > @@ -353,13 +360,8 @@ static ssize_t comp_algorithm_store(struct device *dev, > pr_info("Can't change algorithm for initialized device\n"); > return -EBUSY; > } > - strlcpy(zram->compressor, buf, sizeof(zram->compressor)); > - > - /* ignore trailing newline */ > - sz = strlen(zram->compressor); > - if (sz > 0 && zram->compressor[sz - 1] == '\n') > - zram->compressor[sz - 1] = 0x00; > > + strlcpy(zram->compressor, compressor, sizeof(compressor)); > up_write(&zram->init_lock); > return len; > } > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h > index 3f5bf66..74fcf10 100644 > --- a/drivers/block/zram/zram_drv.h > +++ b/drivers/block/zram/zram_drv.h > @@ -15,8 +15,9 @@ > #ifndef _ZRAM_DRV_H_ > #define _ZRAM_DRV_H_ > > -#include <linux/spinlock.h> > +#include <linux/rwsem.h> > #include <linux/zsmalloc.h> > +#include <linux/crypto.h> > > #include "zcomp.h" > > @@ -113,7 +114,7 @@ struct zram { > * we can store in a disk. > */ > u64 disksize; /* bytes */ > - char compressor[10]; > + char compressor[CRYPTO_MAX_ALG_NAME]; > /* > * zram is claimed so open request will be failed > */ > -- > 2.8.3.394.g3916adf > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-06-01 0:03 ` Minchan Kim @ 2016-06-01 1:07 ` Sergey Senozhatsky 2016-06-01 2:27 ` Minchan Kim 0 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-06-01 1:07 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky Hello Minchan, On (06/01/16 09:03), Minchan Kim wrote: [..] > So, if we do 'cat /sys/block/zram0/comp_algorithm", every crypto modules > in the backend array are loaded in memory and not unloaded until admin > executes rmmod? Right? yes, I think so. [..] > If user load out-of-tree crypto compression module, what's status of > comp_algorithm? > > #> insmod foo_crypto.ko > #> echo foo > /sys/block/zram0/comp_algorithm > #> cat /sys/block/zram0/comp_algorithm > lzo lz4 [foo] > ? yes, "lzo lz4 [out-of-tree-module-name]". -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-06-01 1:07 ` Sergey Senozhatsky @ 2016-06-01 2:27 ` Minchan Kim 2016-06-01 3:17 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-06-01 2:27 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On Wed, Jun 01, 2016 at 10:07:07AM +0900, Sergey Senozhatsky wrote: > Hello Minchan, > > On (06/01/16 09:03), Minchan Kim wrote: > [..] > > So, if we do 'cat /sys/block/zram0/comp_algorithm", every crypto modules > > in the backend array are loaded in memory and not unloaded until admin > > executes rmmod? Right? > > yes, I think so. It scares me. Common case, except one we choosed, every loaded modules will be not used. I think it's really not good. Although the wastage might be not big now, it will be heavy as crypto comp modules are increased. What do you think about it? > > [..] > > If user load out-of-tree crypto compression module, what's status of > > comp_algorithm? > > > > #> insmod foo_crypto.ko > > #> echo foo > /sys/block/zram0/comp_algorithm > > #> cat /sys/block/zram0/comp_algorithm > > lzo lz4 [foo] > > ? > > yes, "lzo lz4 [out-of-tree-module-name]". Makes sense! ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-06-01 2:27 ` Minchan Kim @ 2016-06-01 3:17 ` Sergey Senozhatsky 2016-06-01 6:47 ` Minchan Kim 0 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-06-01 3:17 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On (06/01/16 11:27), Minchan Kim wrote: [..] > > > So, if we do 'cat /sys/block/zram0/comp_algorithm", every crypto modules > > > in the backend array are loaded in memory and not unloaded until admin > > > executes rmmod? Right? > > > > yes, I think so. > > It scares me. Common case, except one we choosed, every loaded modules > will be not used. I think it's really not good. Although the wastage > might be not big now, it will be heavy as crypto comp modules are > increased. well... if you have those modules enabled then you somehow expect them to be loaded at some point, if not by zram, then by something else (networking, etc.). /* not speaking of the systems that have those modules built-in */ I'm not saying that what we have is optimal, of course, but it's not so senseless at the same time. > What do you think about it? I can do something like this: diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 1a4bd20..9b704cc 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -20,10 +20,18 @@ static const char * const backends[] = { "lzo", +#if IS_ENABLED(CONFIG_CRYPTO_LZ4) "lz4", +#endif +#if IS_ENABLED(CONFIG_CRYPTO_DEFLATE) "deflate", +#endif +#if IS_ENABLED(CONFIG_CRYPTO_LZ4HC) "lz4hc", +#endif +#if IS_ENABLED(CONFIG_CRYPTO_842) "842", +#endif NULL }; so both BUILTIN and BUILT-AS-A-MODULE cases are handled at compile time now and we can avoid crypto_has_comp() checks for most of the comp_algorithm calls, except for the case when someone requests an out-of-tree module. -ss ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-06-01 3:17 ` Sergey Senozhatsky @ 2016-06-01 6:47 ` Minchan Kim 2016-06-01 7:48 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-06-01 6:47 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On Wed, Jun 01, 2016 at 12:17:35PM +0900, Sergey Senozhatsky wrote: > On (06/01/16 11:27), Minchan Kim wrote: > [..] > > > > So, if we do 'cat /sys/block/zram0/comp_algorithm", every crypto modules > > > > in the backend array are loaded in memory and not unloaded until admin > > > > executes rmmod? Right? > > > > > > yes, I think so. > > > > It scares me. Common case, except one we choosed, every loaded modules > > will be not used. I think it's really not good. Although the wastage > > might be not big now, it will be heavy as crypto comp modules are > > increased. > > well... if you have those modules enabled then you somehow expect > them to be loaded at some point, if not by zram, then by something > else (networking, etc.). /* not speaking of the systems that have > those modules built-in */ I'm not saying that what we have is > optimal, of course, but it's not so senseless at the same time. In my local system, there are a lot of modules I am not using but distro installed for some point but I believe that some point will not come. IMO, they shouldn't be loaded by the reason they will be used some point, potentially. > > > > What do you think about it? > > I can do something like this: > > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index 1a4bd20..9b704cc 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -20,10 +20,18 @@ > > static const char * const backends[] = { > "lzo", > +#if IS_ENABLED(CONFIG_CRYPTO_LZ4) > "lz4", > +#endif > +#if IS_ENABLED(CONFIG_CRYPTO_DEFLATE) > "deflate", > +#endif > +#if IS_ENABLED(CONFIG_CRYPTO_LZ4HC) > "lz4hc", > +#endif > +#if IS_ENABLED(CONFIG_CRYPTO_842) > "842", > +#endif > NULL > }; > > > so both BUILTIN and BUILT-AS-A-MODULE cases are handled at compile > time now and we can avoid crypto_has_comp() checks for most of the > comp_algorithm calls, except for the case when someone requests an > out-of-tree module. Hmm, isn't it problem, either? That module was built but not installed. In that case, setting the algorithm will be failed. IOW, we are lying to user. For solving the problem, if we check it with crypto_has_comp, again, it will load module into memory. :( 1) Use IS_BUILTIN, not IS_ENABLED for backend For module-crypto, user should set directly without supporting from backend. 2) Deprecated /sys/block/zram0/comp_alrogithm totally Admin should set algorithm by himself like zswap and other cyrpto users. 3) Supporting from zramctl. Maybe, we can teach zramctl so zramctl can find /lib/modules/`uname-r`/ into not-yet-known-modules and use lsmod to find loaded-known-module for zram to use compression algorithm. So, 1,3 combination can be or 2,3 combination can be. Welcome other suggestion. > > -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-06-01 6:47 ` Minchan Kim @ 2016-06-01 7:48 ` Sergey Senozhatsky 2016-06-01 14:59 ` Austin S. Hemmelgarn 2016-06-02 2:40 ` Minchan Kim 0 siblings, 2 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-06-01 7:48 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On (06/01/16 15:47), Minchan Kim wrote: [..] > > so both BUILTIN and BUILT-AS-A-MODULE cases are handled at compile > > time now and we can avoid crypto_has_comp() checks for most of the > > comp_algorithm calls, except for the case when someone requests an > > out-of-tree module. > > Hmm, isn't it problem, either? > > That module was built but not installed. In that case, setting the > algorithm will be failed. IOW, we are lying to user. have you ever seen this? really, why should we even bother? if there is no requested algorithm we will fallback to LZO. and how is that different from: user enabled LZO in .config (because it's a prerequisite for zram) but forgot to install the module? do we have to "fix" this as well?... implement our own LZO compression in zram? or `cp lib/lzo/* drivers/block/zram/'? > For solving the problem, if we check it with crypto_has_comp, again, > it will load module into memory. :( this will require a *VERY* non-standard behaviour from user cat /sys/block/zram0/comp_algorithm [lzo] lz4 # um... echo 842 > /sys/block/zram0/comp_algorithm and I'm quite confident that anyone who does this actually want to init the device with the requested out-of-tree module right after `echo FOO > comp_algorithm', rather than anything else. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-06-01 7:48 ` Sergey Senozhatsky @ 2016-06-01 14:59 ` Austin S. Hemmelgarn 2016-06-02 2:40 ` Minchan Kim 1 sibling, 0 replies; 29+ messages in thread From: Austin S. Hemmelgarn @ 2016-06-01 14:59 UTC (permalink / raw) To: Sergey Senozhatsky, Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On 2016-06-01 03:48, Sergey Senozhatsky wrote: > On (06/01/16 15:47), Minchan Kim wrote: > [..] >>> so both BUILTIN and BUILT-AS-A-MODULE cases are handled at compile >>> time now and we can avoid crypto_has_comp() checks for most of the >>> comp_algorithm calls, except for the case when someone requests an >>> out-of-tree module. >> >> Hmm, isn't it problem, either? >> >> That module was built but not installed. In that case, setting the >> algorithm will be failed. IOW, we are lying to user. > > have you ever seen this? really, why should we even bother? > if there is no requested algorithm we will fallback to LZO. > > and how is that different from: user enabled LZO in .config (because it's > a prerequisite for zram) but forgot to install the module? do we have to > "fix" this as well?... implement our own LZO compression in zram? > or `cp lib/lzo/* drivers/block/zram/'? Ideally, it should fall back to whatever algorithm it can find that is supported, preferably in the following order: lzo lz4 lz4hc deflate 842 LZO first will keep backwards compatibility, while the rest is roughly in decreasing order of performance on most hardware (except on PPC systems which have hardware support for 842 compression). Handling not being able to find any algorithm gets trickier, and the choices are pretty much use a null algorithm and just store flat data, or refuse to store anything. > >> For solving the problem, if we check it with crypto_has_comp, again, >> it will load module into memory. :( > > this will require a *VERY* non-standard behaviour from user > > cat /sys/block/zram0/comp_algorithm > [lzo] lz4 > # um... > echo 842 > /sys/block/zram0/comp_algorithm > > and I'm quite confident that anyone who does this actually want > to init the device with the requested out-of-tree module right > after `echo FOO > comp_algorithm', rather than anything else. Just from the perspective of a system administrator, most people probably aren't going to be directly touching the sysfs entries themselves except possibly for testing, and I don't think I've ever seen anything that actually reads zram/comp_algorithm except to verify that it's set to the requested algorithm. Given that, the behavior I'd expect from zram/comp_algorithm as an administrator would be: 1. List the currently used algorithm together with all algorithm's supported in the mainline kernel. 2. If somebody writes an algorithm we don't know about, check it with crypto_has_comp, and switch to it if successful. 3. Cache positive lookups of unknown algorithms so that you don't have to check again on other devices, and list those algorithms in zram/comp_algorithm even if they're not being used. This would provide relative compatibility with the current behavior, while still allowing people using unknown compression modules to use them. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 4/8] zram: use crypto api to check alg availability 2016-06-01 7:48 ` Sergey Senozhatsky 2016-06-01 14:59 ` Austin S. Hemmelgarn @ 2016-06-02 2:40 ` Minchan Kim 1 sibling, 0 replies; 29+ messages in thread From: Minchan Kim @ 2016-06-02 2:40 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On Wed, Jun 01, 2016 at 04:48:45PM +0900, Sergey Senozhatsky wrote: > On (06/01/16 15:47), Minchan Kim wrote: > [..] > > > so both BUILTIN and BUILT-AS-A-MODULE cases are handled at compile > > > time now and we can avoid crypto_has_comp() checks for most of the > > > comp_algorithm calls, except for the case when someone requests an > > > out-of-tree module. > > > > Hmm, isn't it problem, either? > > > > That module was built but not installed. In that case, setting the > > algorithm will be failed. IOW, we are lying to user. > > have you ever seen this? really, why should we even bother? > if there is no requested algorithm we will fallback to LZO. Yeb, it seems I am too paranoid. Let's not take care about the case. We can simple return -EINVAL and fallback lzo. Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 5/8] zram: cosmetic: cleanup documentation 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky ` (3 preceding siblings ...) 2016-05-31 12:20 ` [PATCH v2 4/8] zram: use crypto api to check alg availability Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-06-01 0:06 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 6/8] zram: delete custom lzo/lz4 Sergey Senozhatsky ` (3 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, Sergey Senozhatsky, Jonathan Corbet zram documentation is a mix of different styles: spaces, tabs, tabs + spaces, etc. clean it up. Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Cc: Jonathan Corbet <corbet@lwn.net> --- Documentation/blockdev/zram.txt | 91 ++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt index 7c05357..0535ae1 100644 --- a/Documentation/blockdev/zram.txt +++ b/Documentation/blockdev/zram.txt @@ -59,23 +59,23 @@ num_devices parameter is optional and tells zram how many devices should be pre-created. Default: 1. 2) Set max number of compression streams - Regardless the value passed to this attribute, ZRAM will always - allocate multiple compression streams - one per online CPUs - thus - allowing several concurrent compression operations. The number of - allocated compression streams goes down when some of the CPUs - become offline. There is no single-compression-stream mode anymore, - unless you are running a UP system or has only 1 CPU online. - - To find out how many streams are currently available: +Regardless the value passed to this attribute, ZRAM will always +allocate multiple compression streams - one per online CPUs - thus +allowing several concurrent compression operations. The number of +allocated compression streams goes down when some of the CPUs +become offline. There is no single-compression-stream mode anymore, +unless you are running a UP system or has only 1 CPU online. + +To find out how many streams are currently available: cat /sys/block/zram0/max_comp_streams 3) Select compression algorithm - Using comp_algorithm device attribute one can see available and - currently selected (shown in square brackets) compression algorithms, - change selected compression algorithm (once the device is initialised - there is no way to change compression algorithm). +Using comp_algorithm device attribute one can see available and +currently selected (shown in square brackets) compression algorithms, +change selected compression algorithm (once the device is initialised +there is no way to change compression algorithm). - Examples: +Examples: #show supported compression algorithms cat /sys/block/zram0/comp_algorithm lzo [lz4] @@ -83,28 +83,27 @@ pre-created. Default: 1. #select lzo compression algorithm echo lzo > /sys/block/zram0/comp_algorithm - For the time being, the `comp_algorithm' content does not necessarily - show every compression algorithm supported by the kernel. We keep this - list primarily to simplify device configuration and one can configure - a new device with a compression algorithm that is not listed in - `comp_algorithm'. The thing is that, internally, ZRAM uses Crypto API - and, if some of the algorithms were built as modules, it's impossible - to list all of them using, for instance, /proc/crypto or any other - method. This, however, has an advantage of permitting the usage of - custom crypto compression modules (implementing S/W or H/W - compression). +For the time being, the `comp_algorithm' content does not necessarily +show every compression algorithm supported by the kernel. We keep this +list primarily to simplify device configuration and one can configure +a new device with a compression algorithm that is not listed in +`comp_algorithm'. The thing is that, internally, ZRAM uses Crypto API +and, if some of the algorithms were built as modules, it's impossible +to list all of them using, for instance, /proc/crypto or any other +method. This, however, has an advantage of permitting the usage of +custom crypto compression modules (implementing S/W or H/W compression). 4) 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: - # Initialize /dev/zram0 with 50MB disksize - echo $((50*1024*1024)) > /sys/block/zram0/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: + # Initialize /dev/zram0 with 50MB disksize + echo $((50*1024*1024)) > /sys/block/zram0/disksize - # Using mem suffixes - echo 256K > /sys/block/zram0/disksize - echo 512M > /sys/block/zram0/disksize - echo 1G > /sys/block/zram0/disksize + # Using mem suffixes + echo 256K > /sys/block/zram0/disksize + echo 512M > /sys/block/zram0/disksize + echo 1G > /sys/block/zram0/disksize Note: There is little point creating a zram of greater than twice the size of memory @@ -112,20 +111,20 @@ 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. 5) Set memory limit: Optional - Set memory limit by writing the value to sysfs node 'mem_limit'. - The value can be either in bytes or you can use mem suffixes. - In addition, you could change the value in runtime. - Examples: - # limit /dev/zram0 with 50MB memory - echo $((50*1024*1024)) > /sys/block/zram0/mem_limit - - # Using mem suffixes - echo 256K > /sys/block/zram0/mem_limit - echo 512M > /sys/block/zram0/mem_limit - echo 1G > /sys/block/zram0/mem_limit - - # To disable memory limit - echo 0 > /sys/block/zram0/mem_limit +Set memory limit by writing the value to sysfs node 'mem_limit'. +The value can be either in bytes or you can use mem suffixes. +In addition, you could change the value in runtime. +Examples: + # limit /dev/zram0 with 50MB memory + echo $((50*1024*1024)) > /sys/block/zram0/mem_limit + + # Using mem suffixes + echo 256K > /sys/block/zram0/mem_limit + echo 512M > /sys/block/zram0/mem_limit + echo 1G > /sys/block/zram0/mem_limit + + # To disable memory limit + echo 0 > /sys/block/zram0/mem_limit 6) Activate: mkswap /dev/zram0 -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 5/8] zram: cosmetic: cleanup documentation 2016-05-31 12:20 ` [PATCH v2 5/8] zram: cosmetic: cleanup documentation Sergey Senozhatsky @ 2016-06-01 0:06 ` Minchan Kim 0 siblings, 0 replies; 29+ messages in thread From: Minchan Kim @ 2016-06-01 0:06 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, Jonathan Corbet On Tue, May 31, 2016 at 09:20:14PM +0900, Sergey Senozhatsky wrote: > zram documentation is a mix of different > styles: spaces, tabs, tabs + spaces, etc. > > clean it up. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Cc: Jonathan Corbet <corbet@lwn.net> Acked-by: Minchan Kim <minchan@kernel.org> ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 6/8] zram: delete custom lzo/lz4 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky ` (4 preceding siblings ...) 2016-05-31 12:20 ` [PATCH v2 5/8] zram: cosmetic: cleanup documentation Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-06-01 0:08 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 7/8] zram: add more compression algorithms Sergey Senozhatsky ` (2 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, Sergey Senozhatsky Remove lzo/lz4 backends, we use crypto API now. Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- 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 2381ca9..0c8606b 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 c914ab7..478cac2 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -16,24 +16,9 @@ struct zcomp_strm { struct crypto_comp *tfm; }; -/* 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 6/8] zram: delete custom lzo/lz4 2016-05-31 12:20 ` [PATCH v2 6/8] zram: delete custom lzo/lz4 Sergey Senozhatsky @ 2016-06-01 0:08 ` Minchan Kim 0 siblings, 0 replies; 29+ messages in thread From: Minchan Kim @ 2016-06-01 0:08 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, May 31, 2016 at 09:20:15PM +0900, Sergey Senozhatsky wrote: > Remove lzo/lz4 backends, we use crypto API now. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Acked-by: Minchan Kim <minchan@kernel.org> ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 7/8] zram: add more compression algorithms 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky ` (5 preceding siblings ...) 2016-05-31 12:20 ` [PATCH v2 6/8] zram: delete custom lzo/lz4 Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-06-01 0:24 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 8/8] zram: drop gfp_t from zcomp_strm_alloc() Sergey Senozhatsky 2016-05-31 12:29 ` [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, 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 <sergey.senozhatsky@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- 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 0c8606b..1e0fc1d1 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 7/8] zram: add more compression algorithms 2016-05-31 12:20 ` [PATCH v2 7/8] zram: add more compression algorithms Sergey Senozhatsky @ 2016-06-01 0:24 ` Minchan Kim 0 siblings, 0 replies; 29+ messages in thread From: Minchan Kim @ 2016-06-01 0:24 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, May 31, 2016 at 09:20:16PM +0900, Sergey Senozhatsky wrote: > 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 <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Acked-by: Minchan Kim <minchan@kernel.org> ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 8/8] zram: drop gfp_t from zcomp_strm_alloc() 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky ` (6 preceding siblings ...) 2016-05-31 12:20 ` [PATCH v2 7/8] zram: add more compression algorithms Sergey Senozhatsky @ 2016-05-31 12:20 ` Sergey Senozhatsky 2016-06-01 0:41 ` Minchan Kim 2016-05-31 12:29 ` [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:20 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, Sergey Senozhatsky We now allocate streams from CPU_UP hot-plug path, there are no context-dependent stream allocations anymore and we can schedule from zcomp_strm_alloc(). Use GFP_KERNEL directly and drop a gfp_t parameter. Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- drivers/block/zram/zcomp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 1e0fc1d1..1a4bd20 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -39,9 +39,9 @@ static void zcomp_strm_free(struct zcomp_strm *zstrm) * allocate new zcomp_strm structure with ->tfm initialized by * backend, return NULL on error */ -static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) +static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp) { - struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), flags); + struct zcomp_strm *zstrm = kmalloc(sizeof(*zstrm), GFP_KERNEL); if (!zstrm) return NULL; @@ -50,7 +50,7 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) * 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); + zstrm->buffer = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1); if (IS_ERR_OR_NULL(zstrm->tfm) || !zstrm->buffer) { zcomp_strm_free(zstrm); zstrm = NULL; @@ -158,7 +158,7 @@ static int __zcomp_cpu_notifier(struct zcomp *comp, case CPU_UP_PREPARE: if (WARN_ON(*per_cpu_ptr(comp->stream, cpu))) break; - zstrm = zcomp_strm_alloc(comp, GFP_KERNEL); + zstrm = zcomp_strm_alloc(comp); if (IS_ERR_OR_NULL(zstrm)) { pr_err("Can't allocate a compression stream\n"); return NOTIFY_BAD; -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 8/8] zram: drop gfp_t from zcomp_strm_alloc() 2016-05-31 12:20 ` [PATCH v2 8/8] zram: drop gfp_t from zcomp_strm_alloc() Sergey Senozhatsky @ 2016-06-01 0:41 ` Minchan Kim 0 siblings, 0 replies; 29+ messages in thread From: Minchan Kim @ 2016-06-01 0:41 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, May 31, 2016 at 09:20:17PM +0900, Sergey Senozhatsky wrote: > We now allocate streams from CPU_UP hot-plug path, there are > no context-dependent stream allocations anymore and we can > schedule from zcomp_strm_alloc(). Use GFP_KERNEL directly and > drop a gfp_t parameter. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Acked-by: Minchan Kim <minchan@kernel.org> ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/8] zram: switch to crypto api 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky ` (7 preceding siblings ...) 2016-05-31 12:20 ` [PATCH v2 8/8] zram: drop gfp_t from zcomp_strm_alloc() Sergey Senozhatsky @ 2016-05-31 12:29 ` Sergey Senozhatsky 2016-05-31 19:07 ` Andrew Morton 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-31 12:29 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Minchan Kim, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/31/16 21:20), Sergey Senozhatsky wrote: > 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. forgot to attach some benchmarks: (x86_64, 4GB, zram-perf script) perf reported run-time fio (max jobs=3) test-fio-zram-842 197.907655282 seconds time elapsed 201.623142884 seconds time elapsed 226.854291345 seconds time elapsed test-fio-zram-DEFLATE 253.259516155 seconds time elapsed 258.148563401 seconds time elapsed 290.251909365 seconds time elapsed test-fio-zram-LZ4 27.022598717 seconds time elapsed 29.580522717 seconds time elapsed 33.293463430 seconds time elapsed test-fio-zram-LZ4HC 56.393954615 seconds time elapsed 74.904659747 seconds time elapsed 101.940998564 seconds time elapsed test-fio-zram-LZO 28.155948075 seconds time elapsed 30.390036330 seconds time elapsed 34.455773159 seconds time elapsed zram mm_stat-s (max fio jobs=3) test-fio-zram-842 mm_stat (jobs1): 3221225472 673185792 690266112 0 690266112 0 0 mm_stat (jobs2): 3221225472 673185792 690266112 0 690266112 0 0 mm_stat (jobs3): 3221225472 673185792 690266112 0 690266112 0 0 test-fio-zram-DEFLATE mm_stat (jobs1): 3221225472 24379392 37761024 0 37761024 0 0 mm_stat (jobs2): 3221225472 24379392 37761024 0 37761024 0 0 mm_stat (jobs3): 3221225472 24379392 37761024 0 37761024 0 0 test-fio-zram-LZ4 mm_stat (jobs1): 3221225472 23592960 37761024 0 37761024 0 0 mm_stat (jobs2): 3221225472 23592960 37761024 0 37761024 0 0 mm_stat (jobs3): 3221225472 23592960 37761024 0 37761024 0 0 test-fio-zram-LZ4HC mm_stat (jobs1): 3221225472 23592960 37761024 0 37761024 0 0 mm_stat (jobs2): 3221225472 23592960 37761024 0 37761024 0 0 mm_stat (jobs3): 3221225472 23592960 37761024 0 37761024 0 0 test-fio-zram-LZO mm_stat (jobs1): 3221225472 34603008 50335744 0 50335744 0 0 mm_stat (jobs2): 3221225472 34603008 50335744 0 50335744 0 0 mm_stat (jobs3): 3221225472 34603008 50335744 0 50339840 0 0 -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/8] zram: switch to crypto api 2016-05-31 12:29 ` [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky @ 2016-05-31 19:07 ` Andrew Morton 2016-06-01 0:58 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Andrew Morton @ 2016-05-31 19:07 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Minchan Kim, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Tue, 31 May 2016 21:29:18 +0900 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote: > forgot to attach some benchmarks: > > (x86_64, 4GB, zram-perf script) > > perf reported run-time fio (max jobs=3) > > test-fio-zram-842 > 197.907655282 seconds time elapsed > 201.623142884 seconds time elapsed > 226.854291345 seconds time elapsed > test-fio-zram-DEFLATE > 253.259516155 seconds time elapsed > 258.148563401 seconds time elapsed > 290.251909365 seconds time elapsed > test-fio-zram-LZ4 > 27.022598717 seconds time elapsed > 29.580522717 seconds time elapsed > 33.293463430 seconds time elapsed > test-fio-zram-LZ4HC > 56.393954615 seconds time elapsed > 74.904659747 seconds time elapsed > 101.940998564 seconds time elapsed > test-fio-zram-LZO > 28.155948075 seconds time elapsed > 30.390036330 seconds time elapsed > 34.455773159 seconds time elapsed I'm having trouble understanding the benchmark results. What is being compared to what and which was faster and how much? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/8] zram: switch to crypto api 2016-05-31 19:07 ` Andrew Morton @ 2016-06-01 0:58 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-06-01 0:58 UTC (permalink / raw) To: Andrew Morton Cc: Sergey Senozhatsky, Minchan Kim, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/31/16 12:07), Andrew Morton wrote: > > test-fio-zram-842 > > 197.907655282 seconds time elapsed > > 201.623142884 seconds time elapsed > > 226.854291345 seconds time elapsed > > test-fio-zram-DEFLATE > > 253.259516155 seconds time elapsed > > 258.148563401 seconds time elapsed > > 290.251909365 seconds time elapsed > > test-fio-zram-LZ4 > > 27.022598717 seconds time elapsed > > 29.580522717 seconds time elapsed > > 33.293463430 seconds time elapsed > > test-fio-zram-LZ4HC > > 56.393954615 seconds time elapsed > > 74.904659747 seconds time elapsed > > 101.940998564 seconds time elapsed > > test-fio-zram-LZO > > 28.155948075 seconds time elapsed > > 30.390036330 seconds time elapsed > > 34.455773159 seconds time elapsed > > I'm having trouble understanding the benchmark results. What is being > compared to what and which was faster and how much? Hello, 'benchmarking' was probably a bit too strong word to use here. basically, I performed fio test with the increasing number of parallel jobs (max to 3) on a 3G zram device, using `static' data and the following crypto comp algorithms: 842, deflate, lz4, lz4hc, lzo the output was: - test running time (which can tell us what algorithms performs faster) and - zram mm_stat (which tells the compressed memory size, max used memory, etc). it's just for information. for example, LZ4HC has twice the running time of LZO, but the compressed memory size is: 23592960 vs 34603008 bytes. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2016-06-02 2:40 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-31 12:20 [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 1/8] zram: rename zstrm find-release functions Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 2/8] zram: switch to crypto compress API Sergey Senozhatsky 2016-05-31 23:40 ` Minchan Kim 2016-05-31 23:44 ` Minchan Kim 2016-06-01 1:17 ` Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 3/8] zram: align zcomp interface to crypto comp API Sergey Senozhatsky 2016-05-31 23:48 ` Minchan Kim 2016-06-01 1:13 ` Sergey Senozhatsky 2016-05-31 12:20 ` [PATCH v2 4/8] zram: use crypto api to check alg availability Sergey Senozhatsky 2016-06-01 0:03 ` Minchan Kim 2016-06-01 1:07 ` Sergey Senozhatsky 2016-06-01 2:27 ` Minchan Kim 2016-06-01 3:17 ` Sergey Senozhatsky 2016-06-01 6:47 ` Minchan Kim 2016-06-01 7:48 ` Sergey Senozhatsky 2016-06-01 14:59 ` Austin S. Hemmelgarn 2016-06-02 2:40 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 5/8] zram: cosmetic: cleanup documentation Sergey Senozhatsky 2016-06-01 0:06 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 6/8] zram: delete custom lzo/lz4 Sergey Senozhatsky 2016-06-01 0:08 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 7/8] zram: add more compression algorithms Sergey Senozhatsky 2016-06-01 0:24 ` Minchan Kim 2016-05-31 12:20 ` [PATCH v2 8/8] zram: drop gfp_t from zcomp_strm_alloc() Sergey Senozhatsky 2016-06-01 0:41 ` Minchan Kim 2016-05-31 12:29 ` [PATCH v2 0/8] zram: switch to crypto api Sergey Senozhatsky 2016-05-31 19:07 ` Andrew Morton 2016-06-01 0:58 ` Sergey Senozhatsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).