From the beginning, the zram block device always enabled CRYPTO_LZO, since
lzo-rle is hardcoded as the fallback compression algorithm. As a consequence, on
systems where another compression algorithm is chosen (e.g. CRYPTO_ZSTD), the
lzo kernel module becomes unused, while still having to be built/loaded.
This patch removes the hardcoded lzo-rle dependency and allows the user to
select the default compression algorithm for zram at build time. The previous
behaviour is kept, as the default algorithm is still lzo-rle.
Suggested-by: Sergey Senozhatsky <[email protected]>
Suggested-by: Minchan Kim <[email protected]>
Signed-off-by: Rui Salvaterra <[email protected]>
---
v5: incorporate Minchan's feedback. Allow the user to choose a default algorithm.
v4: incorporate Sergey's feedback and fix a small typo.
v3: fix the default selection when lzo isn't present. Rebase against 5.10-rc1.
v2: fix the dependency on CRYPTO.
I believe this is the final version, but it does deserve some comment. Given the
choice of having more preprocessor #if/#endif directives in C files or making
the kconfig a bit more complex, I went for the latter. However, since kconfig
choices can only be boolean, I had to devise a way to make a string selection
based on the boolean choice, hence the ZRAM_DEF_COMP symbol.
I also tried to make the ZRAM_AUTOSEL_ALGO definition a bit less painful to the
eye, while still allowing for the compression algorithms to be selected as
modules, as per Sergey's suggestion.
drivers/block/zram/Kconfig | 49 ++++++++++++++++++++++++++++++++++-
drivers/block/zram/zcomp.c | 2 ++
drivers/block/zram/zram_drv.c | 2 +-
3 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..cde089c30ffb 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,6 @@
config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
- select CRYPTO_LZO
help
Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
Pages written to these disks are compressed and stored in memory
@@ -14,6 +13,54 @@ config ZRAM
See Documentation/admin-guide/blockdev/zram.rst for more information.
+choice
+ prompt "Default zram compression algorithm"
+ depends on ZRAM
+
+config ZRAM_DEF_COMP_LZORLE
+ bool "lzo-rle"
+ depends on CRYPTO_LZO
+
+config ZRAM_DEF_COMP_ZSTD
+ bool "zstd"
+ depends on CRYPTO_ZSTD
+
+config ZRAM_DEF_COMP_LZ4
+ bool "lz4"
+ depends on CRYPTO_LZ4
+
+config ZRAM_DEF_COMP_LZO
+ bool "lzo"
+ depends on CRYPTO_LZO
+
+config ZRAM_DEF_COMP_LZ4HC
+ bool "lz4hc"
+ depends on CRYPTO_LZ4HC
+
+config ZRAM_DEF_COMP_842
+ bool "842"
+ depends on CRYPTO_842
+
+endchoice
+
+config ZRAM_DEF_COMP
+ string
+ default "lzo-rle" if ZRAM_DEF_COMP_LZORLE
+ default "zstd" if ZRAM_DEF_COMP_ZSTD
+ default "lz4" if ZRAM_DEF_COMP_LZ4
+ default "lzo" if ZRAM_DEF_COMP_LZO
+ default "lz4hc" if ZRAM_DEF_COMP_LZ4HC
+ default "842" if ZRAM_DEF_COMP_842
+
+config ZRAM_AUTOSEL_ALGO
+ def_bool y
+ depends on ZRAM && \
+ !(CRYPTO_LZ4=m || CRYPTO_LZ4=y || \
+ CRYPTO_LZ4HC=m || CRYPTO_LZ4HC=y || \
+ CRYPTO_842=m || CRYPTO_842=y || \
+ CRYPTO_ZSTD=m || CRYPTO_ZSTD=y)
+ select CRYPTO_LZO
+
config ZRAM_WRITEBACK
bool "Write back incompressible or idle page to backing device"
depends on ZRAM
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..052aa3f65514 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
#include "zcomp.h"
static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
#if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
#endif
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 1b697208d661..9ddccb968c68 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,7 +42,7 @@ static DEFINE_IDR(zram_index_idr);
static DEFINE_MUTEX(zram_index_mutex);
static int zram_major;
-static const char *default_compressor = "lzo-rle";
+static const char *default_compressor = CONFIG_ZRAM_DEF_COMP;
/* Module params (documentation at end) */
static unsigned int num_devices = 1;
--
2.29.2
On Sun, Nov 15, 2020 at 10:15:14AM +0000, Rui Salvaterra wrote:
> From the beginning, the zram block device always enabled CRYPTO_LZO, since
> lzo-rle is hardcoded as the fallback compression algorithm. As a consequence, on
> systems where another compression algorithm is chosen (e.g. CRYPTO_ZSTD), the
> lzo kernel module becomes unused, while still having to be built/loaded.
>
> This patch removes the hardcoded lzo-rle dependency and allows the user to
> select the default compression algorithm for zram at build time. The previous
> behaviour is kept, as the default algorithm is still lzo-rle.
>
> Suggested-by: Sergey Senozhatsky <[email protected]>
> Suggested-by: Minchan Kim <[email protected]>
> Signed-off-by: Rui Salvaterra <[email protected]>
> ---
> v5: incorporate Minchan's feedback. Allow the user to choose a default algorithm.
> v4: incorporate Sergey's feedback and fix a small typo.
> v3: fix the default selection when lzo isn't present. Rebase against 5.10-rc1.
> v2: fix the dependency on CRYPTO.
>
> I believe this is the final version, but it does deserve some comment. Given the
> choice of having more preprocessor #if/#endif directives in C files or making
> the kconfig a bit more complex, I went for the latter. However, since kconfig
> choices can only be boolean, I had to devise a way to make a string selection
> based on the boolean choice, hence the ZRAM_DEF_COMP symbol.
> I also tried to make the ZRAM_AUTOSEL_ALGO definition a bit less painful to the
> eye, while still allowing for the compression algorithms to be selected as
> modules, as per Sergey's suggestion.
>
> drivers/block/zram/Kconfig | 49 ++++++++++++++++++++++++++++++++++-
> drivers/block/zram/zcomp.c | 2 ++
> drivers/block/zram/zram_drv.c | 2 +-
> 3 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index fe7a4b7d30cf..cde089c30ffb 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -2,7 +2,6 @@
> config ZRAM
> tristate "Compressed RAM block device support"
> depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
> - select CRYPTO_LZO
> help
> Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
> Pages written to these disks are compressed and stored in memory
> @@ -14,6 +13,54 @@ config ZRAM
>
> See Documentation/admin-guide/blockdev/zram.rst for more information.
>
> +choice
> + prompt "Default zram compression algorithm"
> + depends on ZRAM
> +
> +config ZRAM_DEF_COMP_LZORLE
> + bool "lzo-rle"
> + depends on CRYPTO_LZO
> +
> +config ZRAM_DEF_COMP_ZSTD
> + bool "zstd"
> + depends on CRYPTO_ZSTD
> +
> +config ZRAM_DEF_COMP_LZ4
> + bool "lz4"
> + depends on CRYPTO_LZ4
> +
> +config ZRAM_DEF_COMP_LZO
> + bool "lzo"
> + depends on CRYPTO_LZO
> +
> +config ZRAM_DEF_COMP_LZ4HC
> + bool "lz4hc"
> + depends on CRYPTO_LZ4HC
> +
> +config ZRAM_DEF_COMP_842
> + bool "842"
> + depends on CRYPTO_842
> +
> +endchoice
> +
> +config ZRAM_DEF_COMP
> + string
> + default "lzo-rle" if ZRAM_DEF_COMP_LZORLE
> + default "zstd" if ZRAM_DEF_COMP_ZSTD
> + default "lz4" if ZRAM_DEF_COMP_LZ4
> + default "lzo" if ZRAM_DEF_COMP_LZO
> + default "lz4hc" if ZRAM_DEF_COMP_LZ4HC
> + default "842" if ZRAM_DEF_COMP_842
> +
> +config ZRAM_AUTOSEL_ALGO
> + def_bool y
> + depends on ZRAM && \
> + !(CRYPTO_LZ4=m || CRYPTO_LZ4=y || \
> + CRYPTO_LZ4HC=m || CRYPTO_LZ4HC=y || \
> + CRYPTO_842=m || CRYPTO_842=y || \
> + CRYPTO_ZSTD=m || CRYPTO_ZSTD=y)
> + select CRYPTO_LZO
> +
Hi Rui,
What's the purpose of ZRAM_AUTOSEL_ALGO?
If you and Sergey already discussed, sorry about the missing it.
Below doesn't work for your goal?
diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..7f3c50f5f87e 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,6 @@
config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
- select CRYPTO_LZO
help
Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
Pages written to these disks are compressed and stored in memory
@@ -14,6 +13,32 @@ config ZRAM
See Documentation/admin-guide/blockdev/zram.rst for more information.
+
+choice
+ prompt "zram default compressor"
+ default ZRAM_COMP_LZO_DEF
+ depends on ZRAM || CRYPTO_LZ4
+ help
+ a
+
+config ZRAM_COMP_LZO_DEF
+ bool "lzo"
+ select CRYPTO_LZO
+ help
+ b
+
+config ZRAM_COMP_LZ4_DEF
+ bool "lz4"
+ depends on CRYPTO_LZ4
+ help
+ c
+endchoice
+
+config ZRAM_DEF_COMP
+ string
+ default "lzo" if ZRAM_COMP_LZO_DEF
+ default "lz4" if ZRAM_COMP_LZ4_DEF
Hi, Minchan,
On Thu, 19 Nov 2020 at 22:26, Minchan Kim <[email protected]> wrote:
>
> What's the purpose of ZRAM_AUTOSEL_ALGO?
> If you and Sergey already discussed, sorry about the missing it.
The purpose of ZRAM_AUTOSEL_ALGO is to make sure at least one of the
required compression algorithms is enabled, either as a module or
built-in. I believe Sergey agreed with the reasoning behind it, but
he'll let us know if I misunderstood. :)
> Below doesn't work for your goal?
Unfortunately, it doesn't. :( It breaks the dependency chain, allowing
you to deselect all compression algorithms in the crypto menu, and
consequently disabling zram. This is only my opinion (and I know
select directives should be very well justified), but I believe that
it should be zram to make sure its required libraries are enabled, and
not the other way around. Having to select a compression algorithm in
the crypto menu for the zram block device to appear in the block
devices menu seems backwards to me.
Thanks,
Rui
On Fri, Nov 20, 2020 at 09:10:13AM +0000, Rui Salvaterra wrote:
> Hi, Minchan,
>
> On Thu, 19 Nov 2020 at 22:26, Minchan Kim <[email protected]> wrote:
> >
> > What's the purpose of ZRAM_AUTOSEL_ALGO?
> > If you and Sergey already discussed, sorry about the missing it.
>
> The purpose of ZRAM_AUTOSEL_ALGO is to make sure at least one of the
> required compression algorithms is enabled, either as a module or
> built-in. I believe Sergey agreed with the reasoning behind it, but
> he'll let us know if I misunderstood. :)
>
> > Below doesn't work for your goal?
>
> Unfortunately, it doesn't. :( It breaks the dependency chain, allowing
> you to deselect all compression algorithms in the crypto menu, and
Hi Rui,
I don't understand it. Please see below. ZRAM_COMP_LZO_DEF select
CRYPTO_LZO, not relying on it. If system supports other CRYPTO module
it will show on choice list. Otherwise, default lzo will be always
there and select CRYPTO_LZO.
Do I miss your point?
+
+choice
+ prompt "zram default compressor"
+ default ZRAM_COMP_LZO_DEF
+ depends on ZRAM || CRYPTO_LZ4
+ help
+ a
+
+config ZRAM_COMP_LZO_DEF
+ bool "lzo"
+ select CRYPTO_LZO
+ help
+ b
+
+config ZRAM_COMP_LZ4_DEF
+ bool "lz4"
+ depends on CRYPTO_LZ4
+ help
+ c
+endchoice
+
+config ZRAM_DEF_COMP
+ string
+ default "lzo" if ZRAM_COMP_LZO_DEF
+ default "lz4" if ZRAM_COMP_LZ4_DEF
Hi, Minchan,
On Fri, 20 Nov 2020 at 21:40, Minchan Kim <[email protected]> wrote:
>
> Do I miss your point?
Well, it's quite possible I mis{read,applied} your patch. It's late
here, I'll test it with fresher brains tomorrow to see if it's
functionally equivalent.
Thanks,
Rui
Hi, Minchan,
On Sat, 21 Nov 2020 at 00:44, Rui Salvaterra <[email protected]> wrote:
>
> Well, it's quite possible I mis{read,applied} your patch.
And I obviously did. Your kconfig suggestion does basically the same,
in a more simplified way. I guess a v6 is in order.
Thanks,
Rui