* [PATCH 0/7] zram: switch to crypto api
@ 2016-05-25 14:29 Sergey Senozhatsky
2016-05-25 14:30 ` [PATCH 1/7] zram: rename zstrm find-release functions Sergey Senozhatsky
` (8 more replies)
0 siblings, 9 replies; 29+ messages in thread
From: Sergey Senozhatsky @ 2016-05-25 14:29 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.
Sergey Senozhatsky (7):
zram: rename zstrm find-release functions
zram: switch to crypto compress API
zram: drop zcomp param from compress/decompress
zram: align zcomp interface to crypto comp API
zram: use crypto api to check alg availability
zram: delete custom lzo/lz4
zram: add more compression algorithms
drivers/block/zram/Kconfig | 15 +------
drivers/block/zram/Makefile | 4 +-
drivers/block/zram/zcomp.c | 91 +++++++++++++++++++++++++++---------------
drivers/block/zram/zcomp.h | 29 ++++----------
drivers/block/zram/zcomp_lz4.c | 56 --------------------------
drivers/block/zram/zcomp_lz4.h | 17 --------
drivers/block/zram/zcomp_lzo.c | 56 --------------------------
drivers/block/zram/zcomp_lzo.h | 17 --------
drivers/block/zram/zram_drv.c | 26 +++++++-----
9 files changed, 84 insertions(+), 227 deletions(-)
delete mode 100644 drivers/block/zram/zcomp_lz4.c
delete mode 100644 drivers/block/zram/zcomp_lz4.h
delete mode 100644 drivers/block/zram/zcomp_lzo.c
delete mode 100644 drivers/block/zram/zcomp_lzo.h
--
2.8.3.394.g3916adf
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/7] zram: rename zstrm find-release functions 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky @ 2016-05-25 14:30 ` Sergey Senozhatsky 2016-05-26 0:44 ` Minchan Kim 2016-05-25 14:30 ` [PATCH 2/7] zram: switch to crypto compress API Sergey Senozhatsky ` (7 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-25 14:30 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> --- 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
* Re: [PATCH 1/7] zram: rename zstrm find-release functions 2016-05-25 14:30 ` [PATCH 1/7] zram: rename zstrm find-release functions Sergey Senozhatsky @ 2016-05-26 0:44 ` Minchan Kim 2016-05-26 1:07 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-26 0:44 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Wed, May 25, 2016 at 11:30:00PM +0900, Sergey Senozhatsky wrote: > We don't perform any zstream idle list lookup anymore, so > zcomp_strm_find()/zcomp_strm_release() names are not > representative. > > Rename to zcomp_stream_get()/zcomp_stream_put(). Actually, I wanted it when we applied percpu but didn't say to you because 1. It's preference of author. Frankly speaking, I prefer get to find but you might think different with me so I want to respect patch author's right if it's not huge pain to me. :) Now I realized you were on same page. 2. We might roll back to stream list. In that case, find is proper word again but it's too trivial. So, I want to merge this patch regardless of this patchset. :) > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> Acked-by: Minchan Kim <minchan@kernel.org> ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/7] zram: rename zstrm find-release functions 2016-05-26 0:44 ` Minchan Kim @ 2016-05-26 1:07 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-26 1:07 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/26/16 09:44), Minchan Kim wrote: > On Wed, May 25, 2016 at 11:30:00PM +0900, Sergey Senozhatsky wrote: > > We don't perform any zstream idle list lookup anymore, so > > zcomp_strm_find()/zcomp_strm_release() names are not > > representative. > > > > Rename to zcomp_stream_get()/zcomp_stream_put(). > > Actually, I wanted it when we applied percpu but didn't say to you because > > 1. It's preference of author. > > Frankly speaking, I prefer get to find but you might think different > with me so I want to respect patch author's right if it's not huge pain > to me. :) > Now I realized you were on same page. > > 2. We might roll back to stream list. > > In that case, find is proper word again but it's too trivial. > So, I want to merge this patch regardless of this patchset. :) > > > > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > > Cc: Minchan Kim <minchan@kernel.org> > Acked-by: Minchan Kim <minchan@kernel.org> thanks. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/7] zram: switch to crypto compress API 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 1/7] zram: rename zstrm find-release functions Sergey Senozhatsky @ 2016-05-25 14:30 ` Sergey Senozhatsky 2016-05-27 4:22 ` Minchan Kim 2016-05-25 14:30 ` [PATCH 3/7] zram: drop zcomp param from compress/decompress Sergey Senozhatsky ` (6 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-25 14:30 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. With out the idle streams list we are free to use compression algorithms that require compression stream for decompression (read), because streams are now placed in per-cpu data and each write path has to disable preemption for compression op, almost completely eliminating the aforementioned case (technically, we still have a small chance, because write path has a fast and a slow paths and the slow path is executed with the preemption enabled; but the frequency of failed fast path is too low). TEST ==== - 4 CPUs, x86_64 system - 3G zram, lzo - fio tests: read, randread, write, randwrite, rw, randrw test script [3] command: ZRAM_SIZE=3G LOG_SUFFIX=XXXX FIO_LOOPS=5 ./zram-fio-test.sh BASE PATCHED jobs1 READ: 2527.2MB/s 2482.7MB/s READ: 2102.7MB/s 2045.0MB/s WRITE: 1284.3MB/s 1324.3MB/s WRITE: 1080.7MB/s 1101.9MB/s READ: 430125KB/s 437498KB/s WRITE: 430538KB/s 437919KB/s READ: 399593KB/s 403987KB/s WRITE: 399910KB/s 404308KB/s jobs2 READ: 8133.5MB/s 7854.8MB/s READ: 7086.6MB/s 6912.8MB/s WRITE: 3177.2MB/s 3298.3MB/s WRITE: 2810.2MB/s 2871.4MB/s READ: 1017.6MB/s 1023.4MB/s WRITE: 1018.2MB/s 1023.1MB/s READ: 977836KB/s 984205KB/s WRITE: 979435KB/s 985814KB/s jobs3 READ: 13557MB/s 13391MB/s READ: 11876MB/s 11752MB/s WRITE: 4641.5MB/s 4682.1MB/s WRITE: 4164.9MB/s 4179.3MB/s READ: 1453.8MB/s 1455.1MB/s WRITE: 1455.1MB/s 1458.2MB/s READ: 1387.7MB/s 1395.7MB/s WRITE: 1386.1MB/s 1394.9MB/s jobs4 READ: 20271MB/s 20078MB/s READ: 18033MB/s 17928MB/s WRITE: 6176.8MB/s 6180.5MB/s WRITE: 5686.3MB/s 5705.3MB/s READ: 2009.4MB/s 2006.7MB/s WRITE: 2007.5MB/s 2004.9MB/s READ: 1929.7MB/s 1935.6MB/s WRITE: 1926.8MB/s 1932.6MB/s jobs5 READ: 18823MB/s 19024MB/s READ: 18968MB/s 19071MB/s WRITE: 6191.6MB/s 6372.1MB/s WRITE: 5818.7MB/s 5787.1MB/s READ: 2011.7MB/s 1981.3MB/s WRITE: 2011.4MB/s 1980.1MB/s READ: 1949.3MB/s 1935.7MB/s WRITE: 1940.4MB/s 1926.1MB/s jobs6 READ: 21870MB/s 21715MB/s READ: 19957MB/s 19879MB/s WRITE: 6528.4MB/s 6537.6MB/s WRITE: 6098.9MB/s 6073.6MB/s READ: 2048.6MB/s 2049.9MB/s WRITE: 2041.7MB/s 2042.9MB/s READ: 2013.4MB/s 1990.4MB/s WRITE: 2009.4MB/s 1986.5MB/s jobs7 READ: 21359MB/s 21124MB/s READ: 19746MB/s 19293MB/s WRITE: 6660.4MB/s 6518.8MB/s WRITE: 6211.6MB/s 6193.1MB/s READ: 2089.7MB/s 2080.6MB/s WRITE: 2085.8MB/s 2076.5MB/s READ: 2041.2MB/s 2052.5MB/s WRITE: 2037.5MB/s 2048.8MB/s jobs8 READ: 20477MB/s 19974MB/s READ: 18922MB/s 18576MB/s WRITE: 6851.9MB/s 6788.3MB/s WRITE: 6407.7MB/s 6347.5MB/s READ: 2134.8MB/s 2136.1MB/s WRITE: 2132.8MB/s 2134.4MB/s READ: 2074.2MB/s 2069.6MB/s WRITE: 2087.3MB/s 2082.4MB/s jobs9 READ: 19797MB/s 19994MB/s READ: 18806MB/s 18581MB/s WRITE: 6878.7MB/s 6822.7MB/s WRITE: 6456.8MB/s 6447.2MB/s READ: 2141.1MB/s 2154.7MB/s WRITE: 2144.4MB/s 2157.3MB/s READ: 2084.1MB/s 2085.1MB/s WRITE: 2091.5MB/s 2092.5MB/s jobs10 READ: 19794MB/s 19784MB/s READ: 18794MB/s 18745MB/s WRITE: 6984.4MB/s 6676.3MB/s WRITE: 6532.3MB/s 6342.7MB/s READ: 2150.6MB/s 2155.4MB/s WRITE: 2156.8MB/s 2161.5MB/s READ: 2106.4MB/s 2095.6MB/s WRITE: 2109.7MB/s 2098.4MB/s BASE PATCHED jobs1 perfstat stalled-cycles-frontend 102,480,595,419 ( 41.53%) 114,508,864,804 ( 46.92%) stalled-cycles-backend 51,941,417,832 ( 21.05%) 46,836,112,388 ( 19.19%) instructions 283,612,054,215 ( 1.15) 283,918,134,959 ( 1.16) branches 56,372,560,385 ( 724.923) 56,449,814,753 ( 733.766) branch-misses 374,826,000 ( 0.66%) 326,935,859 ( 0.58%) jobs2 perfstat stalled-cycles-frontend 155,142,745,777 ( 40.99%) 164,170,979,198 ( 43.82%) stalled-cycles-backend 70,813,866,387 ( 18.71%) 66,456,858,165 ( 17.74%) instructions 463,436,648,173 ( 1.22) 464,221,890,191 ( 1.24) branches 91,088,733,902 ( 760.088) 91,278,144,546 ( 769.133) branch-misses 504,460,363 ( 0.55%) 394,033,842 ( 0.43%) jobs3 perfstat stalled-cycles-frontend 201,300,397,212 ( 39.84%) 223,969,902,257 ( 44.44%) stalled-cycles-backend 87,712,593,974 ( 17.36%) 81,618,888,712 ( 16.19%) instructions 642,869,545,023 ( 1.27) 644,677,354,132 ( 1.28) branches 125,724,560,594 ( 690.682) 126,133,159,521 ( 694.542) branch-misses 527,941,798 ( 0.42%) 444,782,220 ( 0.35%) jobs4 perfstat stalled-cycles-frontend 246,701,197,429 ( 38.12%) 280,076,030,886 ( 43.29%) stalled-cycles-backend 119,050,341,112 ( 18.40%) 110,955,641,671 ( 17.15%) instructions 822,716,962,127 ( 1.27) 825,536,969,320 ( 1.28) branches 160,590,028,545 ( 688.614) 161,152,996,915 ( 691.068) branch-misses 650,295,287 ( 0.40%) 550,229,113 ( 0.34%) jobs5 perfstat stalled-cycles-frontend 298,958,462,516 ( 38.30%) 344,852,200,358 ( 44.16%) stalled-cycles-backend 137,558,742,122 ( 17.62%) 129,465,067,102 ( 16.58%) instructions 1,005,714,688,752 ( 1.29) 1,007,657,999,432 ( 1.29) branches 195,988,773,962 ( 697.730) 196,446,873,984 ( 700.319) branch-misses 695,818,940 ( 0.36%) 624,823,263 ( 0.32%) jobs6 perfstat stalled-cycles-frontend 334,497,602,856 ( 36.71%) 387,590,419,779 ( 42.38%) stalled-cycles-backend 163,539,365,335 ( 17.95%) 152,640,193,639 ( 16.69%) instructions 1,184,738,177,851 ( 1.30) 1,187,396,281,677 ( 1.30) branches 230,592,915,640 ( 702.902) 231,253,802,882 ( 702.356) branch-misses 747,934,786 ( 0.32%) 643,902,424 ( 0.28%) jobs7 perfstat stalled-cycles-frontend 396,724,684,187 ( 37.71%) 460,705,858,952 ( 43.84%) stalled-cycles-backend 188,096,616,496 ( 17.88%) 175,785,787,036 ( 16.73%) instructions 1,364,041,136,608 ( 1.30) 1,366,689,075,112 ( 1.30) branches 265,253,096,936 ( 700.078) 265,890,524,883 ( 702.839) branch-misses 784,991,589 ( 0.30%) 729,196,689 ( 0.27%) jobs8 perfstat stalled-cycles-frontend 440,248,299,870 ( 36.92%) 509,554,793,816 ( 42.46%) stalled-cycles-backend 222,575,930,616 ( 18.67%) 213,401,248,432 ( 17.78%) instructions 1,542,262,045,114 ( 1.29) 1,545,233,932,257 ( 1.29) branches 299,775,178,439 ( 697.666) 300,528,458,505 ( 694.769) branch-misses 847,496,084 ( 0.28%) 748,794,308 ( 0.25%) jobs9 perfstat stalled-cycles-frontend 506,269,882,480 ( 37.86%) 592,798,032,820 ( 44.43%) stalled-cycles-backend 253,192,498,861 ( 18.93%) 233,727,666,185 ( 17.52%) instructions 1,721,985,080,913 ( 1.29) 1,724,666,236,005 ( 1.29) branches 334,517,360,255 ( 694.134) 335,199,758,164 ( 697.131) branch-misses 873,496,730 ( 0.26%) 815,379,236 ( 0.24%) jobs10 perfstat stalled-cycles-frontend 549,063,363,749 ( 37.18%) 651,302,376,662 ( 43.61%) stalled-cycles-backend 281,680,986,810 ( 19.07%) 277,005,235,582 ( 18.55%) instructions 1,901,859,271,180 ( 1.29) 1,906,311,064,230 ( 1.28) branches 369,398,536,153 ( 694.004) 370,527,696,358 ( 688.409) branch-misses 967,929,335 ( 0.26%) 890,125,056 ( 0.24%) BASE PATCHED seconds elapsed 79.421641008 78.735285546 seconds elapsed 61.471246133 60.869085949 seconds elapsed 62.317058173 62.224188495 seconds elapsed 60.030739363 60.081102518 seconds elapsed 74.070398362 74.317582865 seconds elapsed 84.985953007 85.414364176 seconds elapsed 97.724553255 98.173311344 seconds elapsed 109.488066758 110.268399318 seconds elapsed 122.768189405 122.967164498 seconds elapsed 135.130035105 136.934770801 On my other system (8 x86_64 CPUs, short version of test results): BASE PATCHED seconds elapsed 19.518065994 19.806320662 seconds elapsed 15.172772749 15.594718291 seconds elapsed 13.820925970 13.821708564 seconds elapsed 13.293097816 14.585206405 seconds elapsed 16.207284118 16.064431606 seconds elapsed 17.958376158 17.771825767 seconds elapsed 19.478009164 19.602961508 seconds elapsed 21.347152811 21.352318709 seconds elapsed 24.478121126 24.171088735 seconds elapsed 26.865057442 26.767327618 So performance-wise the numbers are quite similar. [1] http://marc.info/?l=linux-kernel&m=144480832108927&w=2 [2] http://marc.info/?l=linux-kernel&m=145379613507518&w=2 [3] https://github.com/sergey-senozhatsky/zram-perf-test Signed-off-by: Sergey Senozhatsky <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 | 66 ++++++++++++++++++++++++++++--------------- drivers/block/zram/zcomp.h | 7 +++-- drivers/block/zram/zram_drv.c | 14 +++++---- 4 files changed, 61 insertions(+), 36 deletions(-) diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig index 386ba3d..2252cd7 100644 --- a/drivers/block/zram/Kconfig +++ b/drivers/block/zram/Kconfig @@ -1,8 +1,7 @@ config ZRAM tristate "Compressed RAM block device support" - depends on BLOCK && SYSFS && ZSMALLOC - select LZO_COMPRESS - select LZO_DECOMPRESS + depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO + select CRYPTO_LZO default n help Creates virtual block devices called /dev/zramX (X = 0, 1, ...). @@ -18,9 +17,8 @@ config ZRAM config ZRAM_LZ4_COMPRESS bool "Enable LZ4 algorithm support" depends on ZRAM - select LZ4_COMPRESS - select LZ4_DECOMPRESS + select CRYPTO_LZ4 default n help This option enables LZ4 compression algorithm support. Compression - algorithm can be changed using `comp_algorithm' device attribute. \ No newline at end of file + algorithm can be changed using `comp_algorithm' device attribute. diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 400f826..82b568e 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -14,26 +14,23 @@ #include <linux/wait.h> #include <linux/sched.h> #include <linux/cpu.h> +#include <linux/crypto.h> #include "zcomp.h" -#include "zcomp_lzo.h" -#ifdef CONFIG_ZRAM_LZ4_COMPRESS -#include "zcomp_lz4.h" -#endif -static struct zcomp_backend *backends[] = { - &zcomp_lzo, +static const char * const backends[] = { + "lzo", #ifdef CONFIG_ZRAM_LZ4_COMPRESS - &zcomp_lz4, + "lz4", #endif NULL }; -static struct zcomp_backend *find_backend(const char *compress) +static const char *find_backend(const char *compress) { int i = 0; while (backends[i]) { - if (sysfs_streq(compress, backends[i]->name)) + if (sysfs_streq(compress, backends[i])) break; i++; } @@ -42,8 +39,8 @@ static struct zcomp_backend *find_backend(const char *compress) static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm) { - if (zstrm->private) - comp->backend->destroy(zstrm->private); + if (!IS_ERR_OR_NULL(zstrm->private)) + crypto_free_comp(zstrm->private); free_pages((unsigned long)zstrm->buffer, 1); kfree(zstrm); } @@ -58,13 +55,13 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) if (!zstrm) return NULL; - zstrm->private = comp->backend->create(flags); + zstrm->private = crypto_alloc_comp(comp->name, 0, 0); /* * allocate 2 pages. 1 for compressed data, plus 1 extra for the * case when compressed size is larger than the original one */ zstrm->buffer = (void *)__get_free_pages(flags | __GFP_ZERO, 1); - if (!zstrm->private || !zstrm->buffer) { + if (IS_ERR_OR_NULL(zstrm->private) || !zstrm->buffer) { zcomp_strm_free(comp, zstrm); zstrm = NULL; } @@ -78,12 +75,12 @@ ssize_t zcomp_available_show(const char *comp, char *buf) int i = 0; while (backends[i]) { - if (!strcmp(comp, backends[i]->name)) + if (!strcmp(comp, backends[i])) sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, - "[%s] ", backends[i]->name); + "[%s] ", backends[i]); else sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, - "%s ", backends[i]->name); + "%s ", backends[i]); i++; } sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n"); @@ -106,16 +103,39 @@ void zcomp_stream_put(struct zcomp *comp) } int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, - const unsigned char *src, size_t *dst_len) + const unsigned char *src, unsigned int *dst_len) { - return comp->backend->compress(src, zstrm->buffer, dst_len, - zstrm->private); + /* + * Our dst memory (zstrm->buffer) is always `2 * PAGE_SIZE' sized + * because sometimes we can endup having a bigger compressed data + * due to various reasons: for example compression algorithms tend + * to add some padding to the compressed buffer. Speaking of padding, + * comp algorithm `842' pads the compressed length to multiple of 8 + * and returns -ENOSP when the dst memory is not big enough, which + * is not something that ZRAM wants to see. We can handle the + * `compressed_size > PAGE_SIZE' case easily in ZRAM, but when we + * receive -ERRNO from the compressing backend we can't help it + * anymore. To make `842' happy we need to tell the exact size of + * the dst buffer, zram_drv will take care of the fact that + * compressed buffer is too big. + */ + *dst_len = PAGE_SIZE * 2; + + return crypto_comp_compress(zstrm->private, + src, PAGE_SIZE, + zstrm->buffer, dst_len); } -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, +int zcomp_decompress(struct zcomp *comp, + struct zcomp_strm *zstrm, + const unsigned char *src, size_t src_len, unsigned char *dst) { - return comp->backend->decompress(src, src_len, dst); + unsigned int dst_len = PAGE_SIZE; + + return crypto_comp_decompress(zstrm->private, + src, src_len, + dst, &dst_len); } static int __zcomp_cpu_notifier(struct zcomp *comp, @@ -209,7 +229,7 @@ void zcomp_destroy(struct zcomp *comp) struct zcomp *zcomp_create(const char *compress) { struct zcomp *comp; - struct zcomp_backend *backend; + const char *backend; int error; backend = find_backend(compress); @@ -220,7 +240,7 @@ struct zcomp *zcomp_create(const char *compress) if (!comp) return ERR_PTR(-ENOMEM); - comp->backend = backend; + comp->name = backend; error = zcomp_init(comp); if (error) { kfree(comp); diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index 944b8e6..ba36bde 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -40,6 +40,8 @@ struct zcomp { struct zcomp_strm * __percpu *stream; struct zcomp_backend *backend; struct notifier_block notifier; + + const char *name; }; ssize_t zcomp_available_show(const char *comp, char *buf); @@ -52,9 +54,10 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp); void zcomp_stream_put(struct zcomp *comp); int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, - const unsigned char *src, size_t *dst_len); + const unsigned char *src, unsigned int *dst_len); -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm, + const unsigned char *src, size_t src_len, unsigned char *dst); bool zcomp_set_max_streams(struct zcomp *comp, int num_strm); diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 9361a5d..fa22a32 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -576,10 +576,14 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index) } cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_RO); - if (size == PAGE_SIZE) + if (size == PAGE_SIZE) { copy_page(mem, cmem); - else - ret = zcomp_decompress(zram->comp, cmem, size, mem); + } else { + struct zcomp_strm *zstrm = zcomp_stream_get(zram->comp); + + ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem); + zcomp_stream_put(zram->comp); + } zs_unmap_object(meta->mem_pool, handle); bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value); @@ -646,7 +650,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index, int offset) { int ret = 0; - size_t clen; + unsigned int clen; unsigned long handle = 0; struct page *page; unsigned char *user_mem, *cmem, *src, *uncmem = NULL; @@ -744,7 +748,7 @@ compress_again: if (handle) goto compress_again; - pr_err("Error allocating memory for compressed page: %u, size=%zu\n", + pr_err("Error allocating memory for compressed page: %u, size=%u\n", index, clen); ret = -ENOMEM; goto out; -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] zram: switch to crypto compress API 2016-05-25 14:30 ` [PATCH 2/7] zram: switch to crypto compress API Sergey Senozhatsky @ 2016-05-27 4:22 ` Minchan Kim 2016-05-27 7:59 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-27 4:22 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Wed, May 25, 2016 at 11:30:01PM +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. With out the Without > 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). Nice description. > > 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 | 66 ++++++++++++++++++++++++++++--------------- > drivers/block/zram/zcomp.h | 7 +++-- > drivers/block/zram/zram_drv.c | 14 +++++---- > 4 files changed, 61 insertions(+), 36 deletions(-) > > diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig > index 386ba3d..2252cd7 100644 > --- a/drivers/block/zram/Kconfig > +++ b/drivers/block/zram/Kconfig > @@ -1,8 +1,7 @@ > config ZRAM > tristate "Compressed RAM block device support" > - depends on BLOCK && SYSFS && ZSMALLOC > - select LZO_COMPRESS > - select LZO_DECOMPRESS > + depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO > + select CRYPTO_LZO > default n > help > Creates virtual block devices called /dev/zramX (X = 0, 1, ...). > @@ -18,9 +17,8 @@ config ZRAM > config ZRAM_LZ4_COMPRESS > bool "Enable LZ4 algorithm support" > depends on ZRAM > - select LZ4_COMPRESS > - select LZ4_DECOMPRESS > + select CRYPTO_LZ4 > default n > help > This option enables LZ4 compression algorithm support. Compression > - algorithm can be changed using `comp_algorithm' device attribute. > \ No newline at end of file > + algorithm can be changed using `comp_algorithm' device attribute. > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index 400f826..82b568e 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -14,26 +14,23 @@ > #include <linux/wait.h> > #include <linux/sched.h> > #include <linux/cpu.h> > +#include <linux/crypto.h> > > #include "zcomp.h" > -#include "zcomp_lzo.h" > -#ifdef CONFIG_ZRAM_LZ4_COMPRESS > -#include "zcomp_lz4.h" > -#endif > > -static struct zcomp_backend *backends[] = { > - &zcomp_lzo, > +static const char * const backends[] = { > + "lzo", > #ifdef CONFIG_ZRAM_LZ4_COMPRESS > - &zcomp_lz4, > + "lz4", > #endif > NULL > }; > > -static struct zcomp_backend *find_backend(const char *compress) > +static const char *find_backend(const char *compress) > { > int i = 0; > while (backends[i]) { > - if (sysfs_streq(compress, backends[i]->name)) > + if (sysfs_streq(compress, backends[i])) > break; > i++; > } > @@ -42,8 +39,8 @@ static struct zcomp_backend *find_backend(const char *compress) > > static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm) > { > - if (zstrm->private) > - comp->backend->destroy(zstrm->private); > + if (!IS_ERR_OR_NULL(zstrm->private)) Let's change private with tfm. > + crypto_free_comp(zstrm->private); > free_pages((unsigned long)zstrm->buffer, 1); > kfree(zstrm); > } > @@ -58,13 +55,13 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) > if (!zstrm) > return NULL; > > - zstrm->private = comp->backend->create(flags); > + zstrm->private = crypto_alloc_comp(comp->name, 0, 0); crypto_alloc_comp uses GPF_KERNEL for allocating tfm and zram uses GFP_KERNEL for zcomp_strm_alloc now so there is no point to pass gfp_t so let's clean it up. Otherwise, looks good to me at af first glance. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] zram: switch to crypto compress API 2016-05-27 4:22 ` Minchan Kim @ 2016-05-27 7:59 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-27 7:59 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/27/16 13:22), Minchan Kim wrote: [..] > > static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm) > > { > > - if (zstrm->private) > > - comp->backend->destroy(zstrm->private); > > + if (!IS_ERR_OR_NULL(zstrm->private)) > > Let's change private with tfm. ok. > > > + crypto_free_comp(zstrm->private); > > free_pages((unsigned long)zstrm->buffer, 1); > > kfree(zstrm); > > } > > @@ -58,13 +55,13 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp, gfp_t flags) > > if (!zstrm) > > return NULL; > > > > - zstrm->private = comp->backend->create(flags); > > + zstrm->private = crypto_alloc_comp(comp->name, 0, 0); > > crypto_alloc_comp uses GPF_KERNEL for allocating tfm and zram uses > GFP_KERNEL for zcomp_strm_alloc now so there is no point to pass > gfp_t so let's clean it up. yes, I realized that after I sent out the patch. it's basically a revert of 75d8947a36d0c9 ("zram: pass gfp from zcomp frontend to backend"), can do this as a separate patch in the series, as it's more of a clenup. I also realized that passing a zcomp pointer to zcomp_strm_free() is useless; will clean that up. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/7] zram: drop zcomp param from compress/decompress 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 1/7] zram: rename zstrm find-release functions Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 2/7] zram: switch to crypto compress API Sergey Senozhatsky @ 2016-05-25 14:30 ` Sergey Senozhatsky 2016-05-27 4:22 ` Minchan Kim 2016-05-25 14:30 ` [PATCH 4/7] zram: align zcomp interface to crypto comp API Sergey Senozhatsky ` (5 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-25 14:30 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, Sergey Senozhatsky We don't need to supply a zcomp pointer to compress/decompress functions, drop it. 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 | 4 ++-- drivers/block/zram/zram_drv.c | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 82b568e..6e16fa2 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -102,7 +102,7 @@ void zcomp_stream_put(struct zcomp *comp) put_cpu_ptr(comp->stream); } -int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, +int zcomp_compress(struct zcomp_strm *zstrm, const unsigned char *src, unsigned int *dst_len) { /* @@ -126,8 +126,7 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, zstrm->buffer, dst_len); } -int zcomp_decompress(struct zcomp *comp, - struct zcomp_strm *zstrm, +int zcomp_decompress(struct zcomp_strm *zstrm, const unsigned char *src, size_t src_len, unsigned char *dst) { diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index ba36bde..3fb9dc4 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -53,10 +53,10 @@ void zcomp_destroy(struct zcomp *comp); struct zcomp_strm *zcomp_stream_get(struct zcomp *comp); void zcomp_stream_put(struct zcomp *comp); -int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, +int zcomp_compress(struct zcomp_strm *zstrm, const unsigned char *src, unsigned int *dst_len); -int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm, +int zcomp_decompress(struct zcomp_strm *zstrm, const unsigned char *src, size_t src_len, unsigned char *dst); diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index fa22a32..99c8be7 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -581,7 +581,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index) } else { struct zcomp_strm *zstrm = zcomp_stream_get(zram->comp); - ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem); + ret = zcomp_decompress(zstrm, cmem, size, mem); zcomp_stream_put(zram->comp); } zs_unmap_object(meta->mem_pool, handle); @@ -700,7 +700,7 @@ compress_again: } zstrm = zcomp_stream_get(zram->comp); - ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen); + ret = zcomp_compress(zstrm, uncmem, &clen); if (!is_partial_io(bvec)) { kunmap_atomic(user_mem); user_mem = NULL; -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] zram: drop zcomp param from compress/decompress 2016-05-25 14:30 ` [PATCH 3/7] zram: drop zcomp param from compress/decompress Sergey Senozhatsky @ 2016-05-27 4:22 ` Minchan Kim 2016-05-27 7:31 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-27 4:22 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Wed, May 25, 2016 at 11:30:02PM +0900, Sergey Senozhatsky wrote: > We don't need to supply a zcomp pointer to compress/decompress > functions, drop it. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Could you fold this patch into previous one? > --- > drivers/block/zram/zcomp.c | 5 ++--- > drivers/block/zram/zcomp.h | 4 ++-- > drivers/block/zram/zram_drv.c | 4 ++-- > 3 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index 82b568e..6e16fa2 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -102,7 +102,7 @@ void zcomp_stream_put(struct zcomp *comp) > put_cpu_ptr(comp->stream); > } > > -int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, > +int zcomp_compress(struct zcomp_strm *zstrm, > const unsigned char *src, unsigned int *dst_len) > { > /* > @@ -126,8 +126,7 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, > zstrm->buffer, dst_len); > } > > -int zcomp_decompress(struct zcomp *comp, > - struct zcomp_strm *zstrm, > +int zcomp_decompress(struct zcomp_strm *zstrm, > const unsigned char *src, > size_t src_len, unsigned char *dst) > { > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h > index ba36bde..3fb9dc4 100644 > --- a/drivers/block/zram/zcomp.h > +++ b/drivers/block/zram/zcomp.h > @@ -53,10 +53,10 @@ void zcomp_destroy(struct zcomp *comp); > struct zcomp_strm *zcomp_stream_get(struct zcomp *comp); > void zcomp_stream_put(struct zcomp *comp); > > -int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, > +int zcomp_compress(struct zcomp_strm *zstrm, > const unsigned char *src, unsigned int *dst_len); > > -int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm, > +int zcomp_decompress(struct zcomp_strm *zstrm, > const unsigned char *src, > size_t src_len, unsigned char *dst); > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index fa22a32..99c8be7 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -581,7 +581,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index) > } else { > struct zcomp_strm *zstrm = zcomp_stream_get(zram->comp); > > - ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem); > + ret = zcomp_decompress(zstrm, cmem, size, mem); > zcomp_stream_put(zram->comp); > } > zs_unmap_object(meta->mem_pool, handle); > @@ -700,7 +700,7 @@ compress_again: > } > > zstrm = zcomp_stream_get(zram->comp); > - ret = zcomp_compress(zram->comp, zstrm, uncmem, &clen); > + ret = zcomp_compress(zstrm, uncmem, &clen); > if (!is_partial_io(bvec)) { > kunmap_atomic(user_mem); > user_mem = NULL; > -- > 2.8.3.394.g3916adf > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/7] zram: drop zcomp param from compress/decompress 2016-05-27 4:22 ` Minchan Kim @ 2016-05-27 7:31 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-27 7:31 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/27/16 13:22), Minchan Kim wrote: > On Wed, May 25, 2016 at 11:30:02PM +0900, Sergey Senozhatsky wrote: > > We don't need to supply a zcomp pointer to compress/decompress > > functions, drop it. > > > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > > Cc: Minchan Kim <minchan@kernel.org> > > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > > Could you fold this patch into previous one? > yes, I thought about it. decided to keep it separated to ease the review. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 4/7] zram: align zcomp interface to crypto comp API 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky ` (2 preceding siblings ...) 2016-05-25 14:30 ` [PATCH 3/7] zram: drop zcomp param from compress/decompress Sergey Senozhatsky @ 2016-05-25 14:30 ` Sergey Senozhatsky 2016-05-27 4:31 ` Minchan Kim 2016-05-25 14:30 ` [PATCH 5/7] zram: use crypto api to check alg availability Sergey Senozhatsky ` (4 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-25 14:30 UTC (permalink / raw) To: Minchan Kim Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky, Sergey Senozhatsky A cosmetic change: use the same datatypes as crypto API does. 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 6e16fa2..79b30d7 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -103,7 +103,7 @@ void zcomp_stream_put(struct zcomp *comp) } int zcomp_compress(struct zcomp_strm *zstrm, - const unsigned char *src, unsigned int *dst_len) + const u8 *src, unsigned int *dst_len) { /* * Our dst memory (zstrm->buffer) is always `2 * PAGE_SIZE' sized @@ -127,8 +127,7 @@ int zcomp_compress(struct zcomp_strm *zstrm, } int zcomp_decompress(struct zcomp_strm *zstrm, - const unsigned char *src, - size_t src_len, unsigned char *dst) + const u8 *src, unsigned int src_len, u8 *dst) { unsigned int dst_len = PAGE_SIZE; diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index 3fb9dc4..dcc0951 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -54,11 +54,10 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp); void zcomp_stream_put(struct zcomp *comp); int zcomp_compress(struct zcomp_strm *zstrm, - const unsigned char *src, unsigned int *dst_len); + const u8 *src, unsigned int *dst_len); int zcomp_decompress(struct zcomp_strm *zstrm, - const unsigned char *src, - size_t src_len, unsigned char *dst); + const u8 *src, unsigned int src_len, u8 *dst); bool zcomp_set_max_streams(struct zcomp *comp, int num_strm); #endif /* _ZCOMP_H_ */ diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 99c8be7..65d1403 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -563,7 +563,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index) unsigned char *cmem; struct zram_meta *meta = zram->meta; unsigned long handle; - size_t size; + unsigned int size; bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value); handle = meta->table[index].handle; -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 4/7] zram: align zcomp interface to crypto comp API 2016-05-25 14:30 ` [PATCH 4/7] zram: align zcomp interface to crypto comp API Sergey Senozhatsky @ 2016-05-27 4:31 ` Minchan Kim 2016-05-27 8:00 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-27 4:31 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Wed, May 25, 2016 at 11:30:03PM +0900, Sergey Senozhatsky wrote: > A cosmetic change: use the same datatypes as crypto API does. > > 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 6e16fa2..79b30d7 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -103,7 +103,7 @@ void zcomp_stream_put(struct zcomp *comp) > } > > int zcomp_compress(struct zcomp_strm *zstrm, > - const unsigned char *src, unsigned int *dst_len) > + const u8 *src, unsigned int *dst_len) Nitpick: The zcomp doesn't need to depend on implementation(i.e., crypto) so zcomp_compress should pass void * for src and dst. I'm not strong aginst changing u8 but your description makes me think about that. > { > /* > * Our dst memory (zstrm->buffer) is always `2 * PAGE_SIZE' sized > @@ -127,8 +127,7 @@ int zcomp_compress(struct zcomp_strm *zstrm, > } > > int zcomp_decompress(struct zcomp_strm *zstrm, > - const unsigned char *src, > - size_t src_len, unsigned char *dst) > + const u8 *src, unsigned int src_len, u8 *dst) > { > unsigned int dst_len = PAGE_SIZE; > > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h > index 3fb9dc4..dcc0951 100644 > --- a/drivers/block/zram/zcomp.h > +++ b/drivers/block/zram/zcomp.h > @@ -54,11 +54,10 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp); > void zcomp_stream_put(struct zcomp *comp); > > int zcomp_compress(struct zcomp_strm *zstrm, > - const unsigned char *src, unsigned int *dst_len); > + const u8 *src, unsigned int *dst_len); > > int zcomp_decompress(struct zcomp_strm *zstrm, > - const unsigned char *src, > - size_t src_len, unsigned char *dst); > + const u8 *src, unsigned int src_len, u8 *dst); > > bool zcomp_set_max_streams(struct zcomp *comp, int num_strm); > #endif /* _ZCOMP_H_ */ > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index 99c8be7..65d1403 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -563,7 +563,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index) > unsigned char *cmem; > struct zram_meta *meta = zram->meta; > unsigned long handle; > - size_t size; > + unsigned int size; > > bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value); > handle = meta->table[index].handle; > -- > 2.8.3.394.g3916adf > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/7] zram: align zcomp interface to crypto comp API 2016-05-27 4:31 ` Minchan Kim @ 2016-05-27 8:00 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-27 8:00 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/27/16 13:31), Minchan Kim wrote: [..] > > int zcomp_compress(struct zcomp_strm *zstrm, > > - const unsigned char *src, unsigned int *dst_len) > > + const u8 *src, unsigned int *dst_len) > > Nitpick: > > The zcomp doesn't need to depend on implementation(i.e., crypto) so > zcomp_compress should pass void * for src and dst. > > I'm not strong aginst changing u8 but your description makes me > think about that. no strong opinion on this either. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky ` (3 preceding siblings ...) 2016-05-25 14:30 ` [PATCH 4/7] zram: align zcomp interface to crypto comp API Sergey Senozhatsky @ 2016-05-25 14:30 ` Sergey Senozhatsky 2016-05-27 4:43 ` Minchan Kim 2016-05-25 14:30 ` [PATCH 6/7] zram: delete custom lzo/lz4 Sergey Senozhatsky ` (3 subsequent siblings) 8 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-25 14:30 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 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 | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 79b30d7..a8593e9 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -29,12 +29,16 @@ static const char * const backends[] = { static const char *find_backend(const char *compress) { int i = 0; + while (backends[i]) { if (sysfs_streq(compress, backends[i])) break; i++; } - return backends[i]; + + if (backends[i] && crypto_has_comp(backends[i], 0, 0)) + return backends[i]; + return NULL; } static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm) @@ -74,14 +78,16 @@ ssize_t zcomp_available_show(const char *comp, char *buf) ssize_t sz = 0; int i = 0; - while (backends[i]) { + for (; backends[i]; i++) { + if (!crypto_has_comp(backends[i], 0, 0)) + continue; + if (!strcmp(comp, backends[i])) sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, "[%s] ", backends[i]); else sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, "%s ", backends[i]); - i++; } sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n"); return sz; -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-25 14:30 ` [PATCH 5/7] zram: use crypto api to check alg availability Sergey Senozhatsky @ 2016-05-27 4:43 ` Minchan Kim 2016-05-27 7:50 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-27 4:43 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky Hello Sergey, I want to know more how it works so below questions goes. On Wed, May 25, 2016 at 11:30:04PM +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(). crypto_has_comp works regardless of that whether module is loading or not? IOW, currently, if lz4 modules is not loading, but crypto_has_comp return true about lz4 module. Right? > We store all the compression algorithms names in zcomp's `backends' > array, regardless the CONFIG_CRYPTO_FOO configuration, but show Then, you mean we should add new string into backend array whenever adding new crypto compatible compression algorithm? > 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 So, when lzo module is loading? > > 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 | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index 79b30d7..a8593e9 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -29,12 +29,16 @@ static const char * const backends[] = { > static const char *find_backend(const char *compress) > { > int i = 0; > + > while (backends[i]) { > if (sysfs_streq(compress, backends[i])) > break; > i++; > } > - return backends[i]; > + > + if (backends[i] && crypto_has_comp(backends[i], 0, 0)) > + return backends[i]; > + return NULL; > } > > static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm) > @@ -74,14 +78,16 @@ ssize_t zcomp_available_show(const char *comp, char *buf) > ssize_t sz = 0; > int i = 0; > > - while (backends[i]) { > + for (; backends[i]; i++) { > + if (!crypto_has_comp(backends[i], 0, 0)) > + continue; > + > if (!strcmp(comp, backends[i])) > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > "[%s] ", backends[i]); > else > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > "%s ", backends[i]); > - i++; > } > sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n"); > return sz; > -- > 2.8.3.394.g3916adf > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-27 4:43 ` Minchan Kim @ 2016-05-27 7:50 ` Sergey Senozhatsky 2016-05-27 8:27 ` Minchan Kim 0 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-27 7:50 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/27/16 13:43), Minchan Kim wrote: [..] > > 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(). > > crypto_has_comp works regardless of that whether module is loading or not? > IOW, currently, if lz4 modules is not loading, but crypto_has_comp return > true about lz4 module. > Right? correct. crypto_has_comp() regardless the module being loaded. # modprobe zram # cat /proc/crypto | egrep -e "lzo|lz4|deflate" # echo lzo > /sys/block/zram0/comp_algorithm # cat /proc/crypto | egrep -e "lzo|lz4|deflate" name : lzo driver : lzo-generic module : lzo # echo lz4 > /sys/block/zram0/comp_algorithm # cat /proc/crypto | egrep -e "lzo|lz4|deflate" name : lz4 driver : lz4-generic module : lz4 name : lzo driver : lzo-generic module : lzo # echo deflate > /sys/block/zram0/comp_algorithm # cat /proc/crypto | egrep -e "lzo|lz4|deflate" name : deflate driver : deflate-generic module : deflate name : lz4 driver : lz4-generic module : lz4 name : lzo driver : lzo-generic module : lzo what is does, tho, it modprobs() the module upon the first request: crypto_has_comp(...) crypto_has_alg() crypto_alg_mod_lookup() crypto_larval_lookup() request_module("crypto-%s", name) __request_module() call_modprobe() and this is when /proc/crypto is getting updated. otherwise user has no information (well, unless modules were loaded by something/someome else, or crypto compressors were built-in into the kernel). I'm not aware of any other way to achieve this functionality for zram. > > We store all the compression algorithms names in zcomp's `backends' > > array, regardless the CONFIG_CRYPTO_FOO configuration, but show > > Then, you mean we should add new string into backend array whenever > adding new crypto compatible compression algorithm? yes. which looks quite trivial: adding or removing a string to/from the array. > > cat /proc/crypto | grep -i lz4 > > > > cat /sys/block/zram0/comp_algorithm > > [lzo] lz4 deflate lz4hc 842 > > So, when lzo module is loading? when we execute crypto_has_comp("lzo") for the first time. that's why doing just # modprobe zram will not cause /proc/crypto update # modprobe zram # cat /proc/crypto | egrep -e "lzo|lz4|deflate" crypto_has_comp() updates it -- when we read or write from/to comp_algorithm: # cat /sys/block/zram0/comp_algorithm [lzo] lz4 deflate lz4hc 842 # cat /proc/crypto | egrep -e "lzo|lz4|deflate" name : lzo driver : lzo-generic module : lzo but I didn't want zram to depend on this, or to depend on /proc/crypto content; that's why I did it the way it is. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-27 7:50 ` Sergey Senozhatsky @ 2016-05-27 8:27 ` Minchan Kim 2016-05-27 8:43 ` Herbert Xu 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-27 8:27 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Herbert Xu On Fri, May 27, 2016 at 04:50:52PM +0900, Sergey Senozhatsky wrote: > On (05/27/16 13:43), Minchan Kim wrote: > [..] > > > 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(). > > > > crypto_has_comp works regardless of that whether module is loading or not? > > IOW, currently, if lz4 modules is not loading, but crypto_has_comp return > > true about lz4 module. > > Right? > > correct. crypto_has_comp() regardless the module being loaded. > > # modprobe zram > # cat /proc/crypto | egrep -e "lzo|lz4|deflate" > > # echo lzo > /sys/block/zram0/comp_algorithm > # cat /proc/crypto | egrep -e "lzo|lz4|deflate" > name : lzo > driver : lzo-generic > module : lzo > > # echo lz4 > /sys/block/zram0/comp_algorithm > # cat /proc/crypto | egrep -e "lzo|lz4|deflate" > name : lz4 > driver : lz4-generic > module : lz4 > name : lzo > driver : lzo-generic > module : lzo > > # echo deflate > /sys/block/zram0/comp_algorithm > # cat /proc/crypto | egrep -e "lzo|lz4|deflate" > name : deflate > driver : deflate-generic > module : deflate > name : lz4 > driver : lz4-generic > module : lz4 > name : lzo > driver : lzo-generic > module : lzo > > > whab it does, tho, it modprobs() the module upon the first > request: > > crypto_has_comp(...) > crypto_has_alg() > crypto_alg_mod_lookup() > crypto_larval_lookup() > request_module("crypto-%s", name) > __request_module() > call_modprobe() > > and this is when /proc/crypto is getting updated. otherwise user has > no information (well, unless modules were loaded by something/someome > else, or crypto compressors were built-in into the kernel). I'm not > aware of any other way to achieve this functionality for zram. Now I got it. Thanks for spending time for me. > > > > > We store all the compression algorithms names in zcomp's `backends' > > > array, regardless the CONFIG_CRYPTO_FOO configuration, but show > > > > Then, you mean we should add new string into backend array whenever > > adding new crypto compatible compression algorithm? > > yes. which looks quite trivial: adding or removing a string to/from > the array. Cc'ing Herbert. Yes, it might be trivial to adding new "string" into the backend array if we consider frequency of adding new compressoin algorithm in linux but it would be better if we can get names of supported compression algorithm name by crypto API. If it's not good idea or something hard to implement, let's go with hardcoding. Herbert, Could you give us thought? > > > > > cat /proc/crypto | grep -i lz4 > > > > > > cat /sys/block/zram0/comp_algorithm > > > [lzo] lz4 deflate lz4hc 842 > > > > So, when lzo module is loading? > > when we execute crypto_has_comp("lzo") for the first time. > > > that's why doing just > > # modprobe zram > > will not cause /proc/crypto update > > > # modprobe zram > # cat /proc/crypto | egrep -e "lzo|lz4|deflate" > > > crypto_has_comp() updates it -- when we read or write > from/to comp_algorithm: > > # cat /sys/block/zram0/comp_algorithm > [lzo] lz4 deflate lz4hc 842 > > # cat /proc/crypto | egrep -e "lzo|lz4|deflate" > name : lzo > driver : lzo-generic > module : lzo > > > but I didn't want zram to depend on this, or to depend on > /proc/crypto content; that's why I did it the way it is. > > -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-27 8:27 ` Minchan Kim @ 2016-05-27 8:43 ` Herbert Xu 2016-05-27 9:04 ` Minchan Kim 0 siblings, 1 reply; 29+ messages in thread From: Herbert Xu @ 2016-05-27 8:43 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On Fri, May 27, 2016 at 05:27:44PM +0900, Minchan Kim wrote: > > Yes, it might be trivial to adding new "string" into the backend array > if we consider frequency of adding new compressoin algorithm in linux > but it would be better if we can get names of supported compression > algorithm name by crypto API. > > If it's not good idea or something hard to implement, let's go with > hardcoding. > > Herbert, Could you give us thought? It is fundamentally impossible to get a list of all *potential* algorithms if you allow loadable modules. By definition someone could load a new module and thus introduce a new algorithm. Given a specific algorithm name you could determine whether it is present on the system. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-27 8:43 ` Herbert Xu @ 2016-05-27 9:04 ` Minchan Kim 2016-05-29 3:24 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-27 9:04 UTC (permalink / raw) To: Herbert Xu Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel On Fri, May 27, 2016 at 04:43:45PM +0800, Herbert Xu wrote: > On Fri, May 27, 2016 at 05:27:44PM +0900, Minchan Kim wrote: > > > > Yes, it might be trivial to adding new "string" into the backend array > > if we consider frequency of adding new compressoin algorithm in linux > > but it would be better if we can get names of supported compression > > algorithm name by crypto API. > > > > If it's not good idea or something hard to implement, let's go with > > hardcoding. > > > > Herbert, Could you give us thought? > > It is fundamentally impossible to get a list of all *potential* > algorithms if you allow loadable modules. By definition someone > could load a new module and thus introduce a new algorithm. Thanks for the quick response. I understand now. Sergey, Then, shouldn't we add functionality to load new algorithm although there is no entry in zcomp backend array? The main reason we changed to crypto is to support various compression algorithm for zram so we should be able to support anyone who want to use custom crypto compression module. A idea is that let's not adding new compression string into backend array from now on and deprecates /zram/comp_algorithm to show name. So, from now on, users should be aware of the compression naming and load module by oneself. It's not kind but I hope admins already knows what cryto compressor they have. I don't have better idea. > > Given a specific algorithm name you could determine whether it > is present on the system. > > Cheers, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-27 9:04 ` Minchan Kim @ 2016-05-29 3:24 ` Sergey Senozhatsky 2016-05-30 4:47 ` Minchan Kim 0 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-29 3:24 UTC (permalink / raw) To: Minchan Kim Cc: Herbert Xu, Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel Hello, On (05/27/16 18:04), Minchan Kim wrote: > > It is fundamentally impossible to get a list of all *potential* > > algorithms if you allow loadable modules. By definition someone > > could load a new module and thus introduce a new algorithm. > > Thanks for the quick response. I understand now. > > Sergey, > > Then, shouldn't we add functionality to load new algorithm although > there is no entry in zcomp backend array? can do, sure: make find_backend() depend on crypto_has_comp() only, *may be* printk a message when we have a mismatch in backends array and crypto_has_comp(). but not really sure about the latter one. > The main reason we changed to crypto is to support various compression > algorithm for zram so we should be able to support anyone who want to > use custom crypto compression module. yes. > A idea is that let's not adding new compression string into backend > array from now on and deprecates /zram/comp_algorithm to show name. I'd really prefer not to deprecate /zram/comp_algorithm now. > So, from now on, users should be aware of the compression naming and > load module by oneself. It's not kind but I hope admins already knows > what cryto compressor they have. um, I'm not in love with this approach. what admins? zram users may be completely unaware of the existence of the crypto API and the kernel rebuild process. for some people switching to new zram will be a matter of "apt-get upgrade/pacman -Syu", not "git am ...; make menuconfig; make -j...". this looks like a major step back. keeping this dummy list of compressing backends around does not take a lot of effort, especially if we will make it unimportant for find_backend() and only use it for that nice user-friendly `cat /zram/comp_algorithm` output. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-29 3:24 ` Sergey Senozhatsky @ 2016-05-30 4:47 ` Minchan Kim 2016-05-30 4:57 ` Sergey Senozhatsky 0 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-30 4:47 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Herbert Xu, Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel Hi Sergey, On Sun, May 29, 2016 at 12:24:58PM +0900, Sergey Senozhatsky wrote: > Hello, > > On (05/27/16 18:04), Minchan Kim wrote: > > > It is fundamentally impossible to get a list of all *potential* > > > algorithms if you allow loadable modules. By definition someone > > > could load a new module and thus introduce a new algorithm. > > > > Thanks for the quick response. I understand now. > > > > Sergey, > > > > Then, shouldn't we add functionality to load new algorithm although > > there is no entry in zcomp backend array? > > can do, sure: make find_backend() depend on crypto_has_comp() only, > *may be* printk a message when we have a mismatch in backends array > and crypto_has_comp(). but not really sure about the latter one. Me, too. We don't need to warn about that. Instead, we should allow loading other algorithm which doesn't live in backend array more clearly. We need to update documentation to clear it out. > > > The main reason we changed to crypto is to support various compression > > algorithm for zram so we should be able to support anyone who want to > > use custom crypto compression module. > > yes. > > > A idea is that let's not adding new compression string into backend > > array from now on and deprecates /zram/comp_algorithm to show name. > > I'd really prefer not to deprecate /zram/comp_algorithm now. > > > So, from now on, users should be aware of the compression naming and > > load module by oneself. It's not kind but I hope admins already knows > > what cryto compressor they have. > > um, I'm not in love with this approach. > > what admins? zram users may be completely unaware of the existence of > the crypto API and the kernel rebuild process. for some people switching > to new zram will be a matter of "apt-get upgrade/pacman -Syu", not > "git am ...; make menuconfig; make -j...". this looks like a major step > back. keeping this dummy list of compressing backends around does not take > a lot of effort, especially if we will make it unimportant for find_backend() > and only use it for that nice user-friendly `cat /zram/comp_algorithm` > output. I am not against with it. Instead, please clear it out to write down docuement "comp_algorithm is just optional so user can load any new algorithm like this blah blah" Thanks. > > -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] zram: use crypto api to check alg availability 2016-05-30 4:47 ` Minchan Kim @ 2016-05-30 4:57 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-30 4:57 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Herbert Xu, Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel Hello Minchan, On (05/30/16 13:47), Minchan Kim wrote: [..] > > can do, sure: make find_backend() depend on crypto_has_comp() only, > > *may be* printk a message when we have a mismatch in backends array > > and crypto_has_comp(). but not really sure about the latter one. > > Me, too. We don't need to warn about that. Instead, we should allow loading > other algorithm which doesn't live in backend array more clearly. > We need to update documentation to clear it out. yes, entirely forgot to update the Documentation. > > > So, from now on, users should be aware of the compression naming and > > > load module by oneself. It's not kind but I hope admins already knows > > > what cryto compressor they have. > > > > um, I'm not in love with this approach. > > > > what admins? zram users may be completely unaware of the existence of > > the crypto API and the kernel rebuild process. for some people switching > > to new zram will be a matter of "apt-get upgrade/pacman -Syu", not > > "git am ...; make menuconfig; make -j...". this looks like a major step > > back. keeping this dummy list of compressing backends around does not take > > a lot of effort, especially if we will make it unimportant for find_backend() > > and only use it for that nice user-friendly `cat /zram/comp_algorithm` > > output. > > I am not against with it. Instead, please clear it out to write down docuement > "comp_algorithm is just optional so user can load any new algorithm like this > blah blah" ok, will do. thanks. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/7] zram: delete custom lzo/lz4 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky ` (4 preceding siblings ...) 2016-05-25 14:30 ` [PATCH 5/7] zram: use crypto api to check alg availability Sergey Senozhatsky @ 2016-05-25 14:30 ` Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 7/7] zram: add more compression algorithms Sergey Senozhatsky ` (2 subsequent siblings) 8 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-25 14:30 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 a8593e9..5ec0eb2 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -20,9 +20,7 @@ static const char * const backends[] = { "lzo", -#ifdef CONFIG_ZRAM_LZ4_COMPRESS "lz4", -#endif NULL }; diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index dcc0951..3cb13f4 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -21,24 +21,9 @@ struct zcomp_strm { void *private; }; -/* static compression backend */ -struct zcomp_backend { - int (*compress)(const unsigned char *src, unsigned char *dst, - size_t *dst_len, void *private); - - int (*decompress)(const unsigned char *src, size_t src_len, - unsigned char *dst); - - void *(*create)(gfp_t flags); - void (*destroy)(void *private); - - const char *name; -}; - /* dynamic per-device compression frontend */ struct zcomp { struct zcomp_strm * __percpu *stream; - struct zcomp_backend *backend; struct notifier_block notifier; const char *name; diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c deleted file mode 100644 index 0110086..0000000 --- a/drivers/block/zram/zcomp_lz4.c +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright (C) 2014 Sergey Senozhatsky. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version - * 2 of the License, or (at your option) any later version. - */ - -#include <linux/kernel.h> -#include <linux/slab.h> -#include <linux/lz4.h> -#include <linux/vmalloc.h> -#include <linux/mm.h> - -#include "zcomp_lz4.h" - -static void *zcomp_lz4_create(gfp_t flags) -{ - void *ret; - - ret = kmalloc(LZ4_MEM_COMPRESS, flags); - if (!ret) - ret = __vmalloc(LZ4_MEM_COMPRESS, - flags | __GFP_HIGHMEM, - PAGE_KERNEL); - return ret; -} - -static void zcomp_lz4_destroy(void *private) -{ - kvfree(private); -} - -static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst, - size_t *dst_len, void *private) -{ - /* return : Success if return 0 */ - return lz4_compress(src, PAGE_SIZE, dst, dst_len, private); -} - -static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len, - unsigned char *dst) -{ - size_t dst_len = PAGE_SIZE; - /* return : Success if return 0 */ - return lz4_decompress_unknownoutputsize(src, src_len, dst, &dst_len); -} - -struct zcomp_backend zcomp_lz4 = { - .compress = zcomp_lz4_compress, - .decompress = zcomp_lz4_decompress, - .create = zcomp_lz4_create, - .destroy = zcomp_lz4_destroy, - .name = "lz4", -}; diff --git a/drivers/block/zram/zcomp_lz4.h b/drivers/block/zram/zcomp_lz4.h deleted file mode 100644 index 60613fb..0000000 --- a/drivers/block/zram/zcomp_lz4.h +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright (C) 2014 Sergey Senozhatsky. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version - * 2 of the License, or (at your option) any later version. - */ - -#ifndef _ZCOMP_LZ4_H_ -#define _ZCOMP_LZ4_H_ - -#include "zcomp.h" - -extern struct zcomp_backend zcomp_lz4; - -#endif /* _ZCOMP_LZ4_H_ */ diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c deleted file mode 100644 index ed7a1f0..0000000 --- a/drivers/block/zram/zcomp_lzo.c +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright (C) 2014 Sergey Senozhatsky. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version - * 2 of the License, or (at your option) any later version. - */ - -#include <linux/kernel.h> -#include <linux/slab.h> -#include <linux/lzo.h> -#include <linux/vmalloc.h> -#include <linux/mm.h> - -#include "zcomp_lzo.h" - -static void *lzo_create(gfp_t flags) -{ - void *ret; - - ret = kmalloc(LZO1X_MEM_COMPRESS, flags); - if (!ret) - ret = __vmalloc(LZO1X_MEM_COMPRESS, - flags | __GFP_HIGHMEM, - PAGE_KERNEL); - return ret; -} - -static void lzo_destroy(void *private) -{ - kvfree(private); -} - -static int lzo_compress(const unsigned char *src, unsigned char *dst, - size_t *dst_len, void *private) -{ - int ret = lzo1x_1_compress(src, PAGE_SIZE, dst, dst_len, private); - return ret == LZO_E_OK ? 0 : ret; -} - -static int lzo_decompress(const unsigned char *src, size_t src_len, - unsigned char *dst) -{ - size_t dst_len = PAGE_SIZE; - int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len); - return ret == LZO_E_OK ? 0 : ret; -} - -struct zcomp_backend zcomp_lzo = { - .compress = lzo_compress, - .decompress = lzo_decompress, - .create = lzo_create, - .destroy = lzo_destroy, - .name = "lzo", -}; diff --git a/drivers/block/zram/zcomp_lzo.h b/drivers/block/zram/zcomp_lzo.h deleted file mode 100644 index 128c580..0000000 --- a/drivers/block/zram/zcomp_lzo.h +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright (C) 2014 Sergey Senozhatsky. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version - * 2 of the License, or (at your option) any later version. - */ - -#ifndef _ZCOMP_LZO_H_ -#define _ZCOMP_LZO_H_ - -#include "zcomp.h" - -extern struct zcomp_backend zcomp_lzo; - -#endif /* _ZCOMP_LZO_H_ */ -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 7/7] zram: add more compression algorithms 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky ` (5 preceding siblings ...) 2016-05-25 14:30 ` [PATCH 6/7] zram: delete custom lzo/lz4 Sergey Senozhatsky @ 2016-05-25 14:30 ` Sergey Senozhatsky 2016-05-26 0:43 ` [PATCH 0/7] zram: switch to crypto api Joonsoo Kim 2016-05-26 0:52 ` Minchan Kim 8 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-25 14:30 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 5ec0eb2..d7d65eb 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -21,6 +21,9 @@ static const char * const backends[] = { "lzo", "lz4", + "deflate", + "lz4hc", + "842", NULL }; -- 2.8.3.394.g3916adf ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 0/7] zram: switch to crypto api 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky ` (6 preceding siblings ...) 2016-05-25 14:30 ` [PATCH 7/7] zram: add more compression algorithms Sergey Senozhatsky @ 2016-05-26 0:43 ` Joonsoo Kim 2016-05-26 1:12 ` Sergey Senozhatsky 2016-05-26 0:52 ` Minchan Kim 8 siblings, 1 reply; 29+ messages in thread From: Joonsoo Kim @ 2016-05-26 0:43 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Minchan Kim, Andrew Morton, linux-kernel, Sergey Senozhatsky On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote: > Hello, > > This has started as a 'add zlib support' work, but after some > thinking I saw no blockers for a bigger change -- a switch to > crypto API. > > We don't have an idle zstreams list anymore and our write path > now works absolutely differently, preventing preemption during > compression. This removes possibilities of read paths preempting > writes at wrong places and opens the door for a move from custom > LZO/LZ4 compression backends implementation to a more generic one, > using crypto compress API. Hello, Sergey. I don't look at each patches deeply but nice work! I didn't notice that rececnt zram changes makes thing simpler. :) Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/7] zram: switch to crypto api 2016-05-26 0:43 ` [PATCH 0/7] zram: switch to crypto api Joonsoo Kim @ 2016-05-26 1:12 ` Sergey Senozhatsky 2016-05-26 1:52 ` Joonsoo Kim 0 siblings, 1 reply; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-26 1:12 UTC (permalink / raw) To: Joonsoo Kim Cc: Sergey Senozhatsky, Minchan Kim, Andrew Morton, linux-kernel, Sergey Senozhatsky On (05/26/16 09:43), Joonsoo Kim wrote: [..] > Hello, Sergey. > > I don't look at each patches deeply but nice work! I didn't notice that > rececnt zram changes makes thing simpler. :) Hello Joonsoo, thanks. I owe you a drink for pushing it in the context-less crypto API direction. sorry about that. -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/7] zram: switch to crypto api 2016-05-26 1:12 ` Sergey Senozhatsky @ 2016-05-26 1:52 ` Joonsoo Kim 0 siblings, 0 replies; 29+ messages in thread From: Joonsoo Kim @ 2016-05-26 1:52 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Sergey Senozhatsky, Minchan Kim, Andrew Morton, linux-kernel On Thu, May 26, 2016 at 10:12:16AM +0900, Sergey Senozhatsky wrote: > On (05/26/16 09:43), Joonsoo Kim wrote: > [..] > > Hello, Sergey. > > > > I don't look at each patches deeply but nice work! I didn't notice that > > rececnt zram changes makes thing simpler. :) > > Hello Joonsoo, > > thanks. > > I owe you a drink for pushing it in the context-less crypto > API direction. sorry about that. Good to hear. I will wait for your invitation. :) Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/7] zram: switch to crypto api 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky ` (7 preceding siblings ...) 2016-05-26 0:43 ` [PATCH 0/7] zram: switch to crypto api Joonsoo Kim @ 2016-05-26 0:52 ` Minchan Kim 2016-05-26 1:08 ` Sergey Senozhatsky 8 siblings, 1 reply; 29+ messages in thread From: Minchan Kim @ 2016-05-26 0:52 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote: > Hello, > > This has started as a 'add zlib support' work, but after some > thinking I saw no blockers for a bigger change -- a switch to > crypto API. > > We don't have an idle zstreams list anymore and our write path > now works absolutely differently, preventing preemption during > compression. This removes possibilities of read paths preempting > writes at wrong places and opens the door for a move from custom > LZO/LZ4 compression backends implementation to a more generic one, > using crypto compress API. > > This patch set also eliminates the need of a new context-less > crypto API interface, which was quite hard to sell, so we can > move along faster. Super fast Sergey. At a first glance, patchset looks nice. I will review as soon as possible. Thanks a lot! > > > Sergey Senozhatsky (7): > zram: rename zstrm find-release functions > zram: switch to crypto compress API > zram: drop zcomp param from compress/decompress > zram: align zcomp interface to crypto comp API > zram: use crypto api to check alg availability > zram: delete custom lzo/lz4 > zram: add more compression algorithms > > drivers/block/zram/Kconfig | 15 +------ > drivers/block/zram/Makefile | 4 +- > drivers/block/zram/zcomp.c | 91 +++++++++++++++++++++++++++--------------- > drivers/block/zram/zcomp.h | 29 ++++---------- > drivers/block/zram/zcomp_lz4.c | 56 -------------------------- > drivers/block/zram/zcomp_lz4.h | 17 -------- > drivers/block/zram/zcomp_lzo.c | 56 -------------------------- > drivers/block/zram/zcomp_lzo.h | 17 -------- > drivers/block/zram/zram_drv.c | 26 +++++++----- > 9 files changed, 84 insertions(+), 227 deletions(-) > delete mode 100644 drivers/block/zram/zcomp_lz4.c > delete mode 100644 drivers/block/zram/zcomp_lz4.h > delete mode 100644 drivers/block/zram/zcomp_lzo.c > delete mode 100644 drivers/block/zram/zcomp_lzo.h > > -- > 2.8.3.394.g3916adf > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/7] zram: switch to crypto api 2016-05-26 0:52 ` Minchan Kim @ 2016-05-26 1:08 ` Sergey Senozhatsky 0 siblings, 0 replies; 29+ messages in thread From: Sergey Senozhatsky @ 2016-05-26 1:08 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, Joonsoo Kim, linux-kernel, Sergey Senozhatsky On (05/26/16 09:52), Minchan Kim wrote: > On Wed, May 25, 2016 at 11:29:59PM +0900, Sergey Senozhatsky wrote: > > Hello, > > > > This has started as a 'add zlib support' work, but after some > > thinking I saw no blockers for a bigger change -- a switch to > > crypto API. > > > > We don't have an idle zstreams list anymore and our write path > > now works absolutely differently, preventing preemption during > > compression. This removes possibilities of read paths preempting > > writes at wrong places and opens the door for a move from custom > > LZO/LZ4 compression backends implementation to a more generic one, > > using crypto compress API. > > > > This patch set also eliminates the need of a new context-less > > crypto API interface, which was quite hard to sell, so we can > > move along faster. > > Super fast Sergey. hahaha :) > At a first glance, patchset looks nice. > I will review as soon as possible. > > Thanks a lot! thanks! -ss ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2016-05-30 4:57 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-25 14:29 [PATCH 0/7] zram: switch to crypto api Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 1/7] zram: rename zstrm find-release functions Sergey Senozhatsky 2016-05-26 0:44 ` Minchan Kim 2016-05-26 1:07 ` Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 2/7] zram: switch to crypto compress API Sergey Senozhatsky 2016-05-27 4:22 ` Minchan Kim 2016-05-27 7:59 ` Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 3/7] zram: drop zcomp param from compress/decompress Sergey Senozhatsky 2016-05-27 4:22 ` Minchan Kim 2016-05-27 7:31 ` Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 4/7] zram: align zcomp interface to crypto comp API Sergey Senozhatsky 2016-05-27 4:31 ` Minchan Kim 2016-05-27 8:00 ` Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 5/7] zram: use crypto api to check alg availability Sergey Senozhatsky 2016-05-27 4:43 ` Minchan Kim 2016-05-27 7:50 ` Sergey Senozhatsky 2016-05-27 8:27 ` Minchan Kim 2016-05-27 8:43 ` Herbert Xu 2016-05-27 9:04 ` Minchan Kim 2016-05-29 3:24 ` Sergey Senozhatsky 2016-05-30 4:47 ` Minchan Kim 2016-05-30 4:57 ` Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 6/7] zram: delete custom lzo/lz4 Sergey Senozhatsky 2016-05-25 14:30 ` [PATCH 7/7] zram: add more compression algorithms Sergey Senozhatsky 2016-05-26 0:43 ` [PATCH 0/7] zram: switch to crypto api Joonsoo Kim 2016-05-26 1:12 ` Sergey Senozhatsky 2016-05-26 1:52 ` Joonsoo Kim 2016-05-26 0:52 ` Minchan Kim 2016-05-26 1:08 ` 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).