Cc-ing Andrew
message-id: [email protected]
On (20/10/28 11:59), Rui Salvaterra wrote:
> There's nothing special about zram and lzo. It works just fine without it, so
> as long as at least one of the other supported compression algorithms is
> selected.
>
> Suggested-by: Sergey Senozhatsky <[email protected]>
> Signed-off-by: Rui Salvaterra <[email protected]>
Minchan, I'm fine with the change.
Two things from my side:
1) The commit message, probably, can be a bit more informative. Something
like this?
ZRAM always enables CRYPTO_LZO because lzo-rle is the hardcoded
fallback compression algorithm, which means that on systems where
ZRAM always use, for instance, CRYPTO_ZSTD lzo kernel module
becomes unneeded. This patch removes the hardcoded lzo-lre
dependency, instead ZRAM picks the first supported CRYPTO
compression algorithm, should it be ZSTD or LZ4, etc; and only
forcibly enables CRYPTO_LZO (previous behaviour) if none of the
alternative algorithms were selected.
2) The ZRAM_AUTOSEL_ALGO allows to deselect CRYPTO_LZO only if
CRYPTO_LZ4/CRYPTO_LZ4HC/CRYPTO_842/CRYPTO_ZSTD are compiled in (=y).
If any of the algorithms is selected as a module (=m) then CRYPTO_LZO
is selected as the default algorithm. Apparently depends on !(CONFIG_FOO)
means depends on !(CONFIG_FOO=y).
It appears that the below change fixes it, but it looks a bit ugly.
---
diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 141ce0ebad06..f2fd34de9200 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -15,7 +15,7 @@ config ZRAM
config ZRAM_AUTOSEL_ALGO
def_bool y
- depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || CRYPTO_ZSTD)
+ depends on ZRAM && !(CRYPTO_LZ4=m || CRYPTO_LZ4HC=m || CRYPTO_842=m || CRYPTO_ZSTD=m || CRYPTO_LZ4=y || CRYPTO_LZ4HC=y || CRYPTO_842=y || CRYPTO_ZSTD=y)
select CRYPTO_LZO
config ZRAM_WRITEBACK
---
-ss
Hi Sergey and Rui,
On Thu, Oct 29, 2020 at 03:59:27AM +0900, Sergey Senozhatsky wrote:
> Cc-ing Andrew
>
> message-id: [email protected]
>
> On (20/10/28 11:59), Rui Salvaterra wrote:
> > There's nothing special about zram and lzo. It works just fine without it, so
> > as long as at least one of the other supported compression algorithms is
> > selected.
> >
> > Suggested-by: Sergey Senozhatsky <[email protected]>
> > Signed-off-by: Rui Salvaterra <[email protected]>
Sorry for the late. I am still looking for the time to look into this
patch.
I totally agree with the motivation of Rui. Before that, just a
dumb question.
Can't we just provide choice/endchoice in Kconfig to select default
comp algorithm from admin?
>
> Minchan, I'm fine with the change.
>
> Two things from my side:
>
> 1) The commit message, probably, can be a bit more informative. Something
> like this?
>
> ZRAM always enables CRYPTO_LZO because lzo-rle is the hardcoded
> fallback compression algorithm, which means that on systems where
> ZRAM always use, for instance, CRYPTO_ZSTD lzo kernel module
> becomes unneeded. This patch removes the hardcoded lzo-lre
> dependency, instead ZRAM picks the first supported CRYPTO
> compression algorithm, should it be ZSTD or LZ4, etc; and only
> forcibly enables CRYPTO_LZO (previous behaviour) if none of the
> alternative algorithms were selected.
>
>
> 2) The ZRAM_AUTOSEL_ALGO allows to deselect CRYPTO_LZO only if
> CRYPTO_LZ4/CRYPTO_LZ4HC/CRYPTO_842/CRYPTO_ZSTD are compiled in (=y).
> If any of the algorithms is selected as a module (=m) then CRYPTO_LZO
> is selected as the default algorithm. Apparently depends on !(CONFIG_FOO)
> means depends on !(CONFIG_FOO=y).
>
> It appears that the below change fixes it, but it looks a bit ugly.
>
> ---
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index 141ce0ebad06..f2fd34de9200 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -15,7 +15,7 @@ config ZRAM
>
> config ZRAM_AUTOSEL_ALGO
> def_bool y
> - depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || CRYPTO_ZSTD)
> + depends on ZRAM && !(CRYPTO_LZ4=m || CRYPTO_LZ4HC=m || CRYPTO_842=m || CRYPTO_ZSTD=m || CRYPTO_LZ4=y || CRYPTO_LZ4HC=y || CRYPTO_842=y || CRYPTO_ZSTD=y)
> select CRYPTO_LZO
>
> config ZRAM_WRITEBACK
> ---
>
> -ss
Hi, Minchan,
On Tue, 3 Nov 2020 at 21:29, Minchan Kim <[email protected]> wrote:
>
> Can't we just provide choice/endchoice in Kconfig to select default
> comp algorithm from admin?
I'm fine with whatever you guys decide, just let me know what works
best for everyone.
Thanks,
Rui
On Wed, Nov 04, 2020 at 02:12:35PM +0000, Rui Salvaterra wrote:
> Hi, Minchan,
>
> On Tue, 3 Nov 2020 at 21:29, Minchan Kim <[email protected]> wrote:
> >
> > Can't we just provide choice/endchoice in Kconfig to select default
> > comp algorithm from admin?
>
> I'm fine with whatever you guys decide, just let me know what works
> best for everyone.
Thanks. Sergey, if you don't mined, I'd like to approach more explict
way to select default compressor algorithm in Kconfig.
Do you have any suggestion?
Thank you.
On (20/11/04 12:41), Minchan Kim wrote:
> On Wed, Nov 04, 2020 at 02:12:35PM +0000, Rui Salvaterra wrote:
> > Hi, Minchan,
> >
> > On Tue, 3 Nov 2020 at 21:29, Minchan Kim <[email protected]> wrote:
> > >
> > > Can't we just provide choice/endchoice in Kconfig to select default
> > > comp algorithm from admin?
> >
> > I'm fine with whatever you guys decide, just let me know what works
> > best for everyone.
>
> Thanks. Sergey, if you don't mined, I'd like to approach more explict
> way to select default compressor algorithm in Kconfig.
>
> Do you have any suggestion?
No objections. Some people tend to dislike new Kconfig options,
but we probably can live with that.
-ss