If F2FS_FS is modular, enabling the compressions options
F2FS_FS_{LZ4,LZ4HZ,LZO,LZORLE,ZSTD} will make the (de)compression
algorithms {LZ4,LZ4HC,LZO,ZSTD}_{,DE}COMPRESS builtin instead of
modular, as the former depend on an intermediate boolean
F2FS_FS_COMPRESSION, which in-turn depends on tristate F2FS_FS.
Indeed, if a boolean symbol A depends directly on a tristate symbol B
and selects another tristate symbol C:
tristate B
tristate C
bool A
depends on B
select C
and B is modular, then C will also be modular.
However, if there is an intermediate boolean D in the dependency chain
between A and B:
tristate B
tristate C
bool D
depends on B
bool A
depends on D
select C
then the modular state won't propagate from B to C, and C will be
builtin instead of modular.
Fix this by making the various compression options depend directly on
F2FS_FS using a big if/endif block. Drop the now superfluous
dependencies on F2FS_FS from individual symbols.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Perhaps the propagation logic in Kconfig should be fixed instead?
Else people may reintroduce this issue when removing seemingly-unneeded
dependencies.
---
fs/f2fs/Kconfig | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index 62e638a49bbf089a..20a82ecb72b42f84 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -20,9 +20,10 @@ config F2FS_FS
If unsure, say N.
+if F2FS_FS
+
config F2FS_STAT_FS
bool "F2FS Status Information"
- depends on F2FS_FS
default y
help
/sys/kernel/debug/f2fs/ contains information about all the partitions
@@ -35,7 +36,6 @@ config F2FS_STAT_FS
config F2FS_FS_XATTR
bool "F2FS extended attributes"
- depends on F2FS_FS
default y
help
Extended attributes are name:value pairs associated with inodes by
@@ -70,7 +70,6 @@ config F2FS_FS_SECURITY
config F2FS_CHECK_FS
bool "F2FS consistency checking feature"
- depends on F2FS_FS
help
Enables BUG_ONs which check the filesystem consistency in runtime.
@@ -78,7 +77,6 @@ config F2FS_CHECK_FS
config F2FS_FAULT_INJECTION
bool "F2FS fault injection facility"
- depends on F2FS_FS
help
Test F2FS to inject faults such as ENOMEM, ENOSPC, and so on.
@@ -86,7 +84,6 @@ config F2FS_FAULT_INJECTION
config F2FS_FS_COMPRESSION
bool "F2FS compression feature"
- depends on F2FS_FS
help
Enable filesystem-level compression on f2fs regular files,
multiple back-end compression algorithms are supported.
@@ -137,3 +134,5 @@ config F2FS_FS_LZORLE
default y
help
Support LZO-RLE compress algorithm, if unsure, say Y.
+
+endif
--
2.25.1
On Mon, Feb 22, 2021 at 9:59 PM Geert Uytterhoeven <[email protected]> wrote:
>
> If F2FS_FS is modular, enabling the compressions options
> F2FS_FS_{LZ4,LZ4HZ,LZO,LZORLE,ZSTD} will make the (de)compression
> algorithms {LZ4,LZ4HC,LZO,ZSTD}_{,DE}COMPRESS builtin instead of
> modular, as the former depend on an intermediate boolean
> F2FS_FS_COMPRESSION, which in-turn depends on tristate F2FS_FS.
>
> Indeed, if a boolean symbol A depends directly on a tristate symbol B
> and selects another tristate symbol C:
>
> tristate B
>
> tristate C
>
> bool A
> depends on B
> select C
>
> and B is modular, then C will also be modular.
>
> However, if there is an intermediate boolean D in the dependency chain
> between A and B:
>
> tristate B
>
> tristate C
>
> bool D
> depends on B
>
> bool A
> depends on D
> select C
>
> then the modular state won't propagate from B to C, and C will be
> builtin instead of modular.
>
> Fix this by making the various compression options depend directly on
> F2FS_FS using a big if/endif block. Drop the now superfluous
> dependencies on F2FS_FS from individual symbols.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Perhaps the propagation logic in Kconfig should be fixed instead?
> Else people may reintroduce this issue when removing seemingly-unneeded
> dependencies.
I checked the code in menu_finalize(), and this seems to work like this.
I discussed the oddity of the select behavior before
(https://lore.kernel.org/linux-kbuild/[email protected]/),
but I was not confident about what the right direction was.
Anyway, the behavior is obscure from the current code.
If you want to make this more robust,
you can write as follows:
config F2FS_FS
tristate "F2FS filesystem support"
depends on BLOCK
select NLS
select CRYPTO
select CRYPTO_CRC32
select F2FS_FS_XATTR if FS_ENCRYPTION
select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
select LZO_COMPRESS if F2FS_FS_LZO
select LZO_DECOMPRESS if F2FS_FS_LZO
select LZ4_COMPRESS if F2FS_FS_LZ4
select LZ4_DECOMPRESS if F2FS_FS_LZ4
select LZ4HC_COMPRESS if F2FS_FS_LZ4HC
select ZSTD_COMPRESS if F2FS_FS_ZSTD
select ZSTD_DECOMPRESS if F2FS_FS_ZSTD
The code is a bit clumsy, but it is clear
that the module (F2FS_FS) is selecting the
compress/decompress libraries.
> ---
> fs/f2fs/Kconfig | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
> index 62e638a49bbf089a..20a82ecb72b42f84 100644
> --- a/fs/f2fs/Kconfig
> +++ b/fs/f2fs/Kconfig
> @@ -20,9 +20,10 @@ config F2FS_FS
>
> If unsure, say N.
>
> +if F2FS_FS
> +
> config F2FS_STAT_FS
> bool "F2FS Status Information"
> - depends on F2FS_FS
> default y
> help
> /sys/kernel/debug/f2fs/ contains information about all the partitions
> @@ -35,7 +36,6 @@ config F2FS_STAT_FS
>
> config F2FS_FS_XATTR
> bool "F2FS extended attributes"
> - depends on F2FS_FS
> default y
> help
> Extended attributes are name:value pairs associated with inodes by
> @@ -70,7 +70,6 @@ config F2FS_FS_SECURITY
>
> config F2FS_CHECK_FS
> bool "F2FS consistency checking feature"
> - depends on F2FS_FS
> help
> Enables BUG_ONs which check the filesystem consistency in runtime.
>
> @@ -78,7 +77,6 @@ config F2FS_CHECK_FS
>
> config F2FS_FAULT_INJECTION
> bool "F2FS fault injection facility"
> - depends on F2FS_FS
> help
> Test F2FS to inject faults such as ENOMEM, ENOSPC, and so on.
>
> @@ -86,7 +84,6 @@ config F2FS_FAULT_INJECTION
>
> config F2FS_FS_COMPRESSION
> bool "F2FS compression feature"
> - depends on F2FS_FS
> help
> Enable filesystem-level compression on f2fs regular files,
> multiple back-end compression algorithms are supported.
> @@ -137,3 +134,5 @@ config F2FS_FS_LZORLE
> default y
> help
> Support LZO-RLE compress algorithm, if unsure, say Y.
> +
> +endif
> --
> 2.25.1
>
--
Best Regards
Masahiro Yamada
Hi Yamada-san,
On Tue, Feb 23, 2021 at 7:31 AM Masahiro Yamada <[email protected]> wrote:
> On Mon, Feb 22, 2021 at 9:59 PM Geert Uytterhoeven <[email protected]> wrote:
> > If F2FS_FS is modular, enabling the compressions options
> > F2FS_FS_{LZ4,LZ4HZ,LZO,LZORLE,ZSTD} will make the (de)compression
> > algorithms {LZ4,LZ4HC,LZO,ZSTD}_{,DE}COMPRESS builtin instead of
> > modular, as the former depend on an intermediate boolean
> > F2FS_FS_COMPRESSION, which in-turn depends on tristate F2FS_FS.
> >
> > Indeed, if a boolean symbol A depends directly on a tristate symbol B
> > and selects another tristate symbol C:
> >
> > tristate B
> >
> > tristate C
> >
> > bool A
> > depends on B
> > select C
> >
> > and B is modular, then C will also be modular.
> >
> > However, if there is an intermediate boolean D in the dependency chain
> > between A and B:
> >
> > tristate B
> >
> > tristate C
> >
> > bool D
> > depends on B
> >
> > bool A
> > depends on D
> > select C
> >
> > then the modular state won't propagate from B to C, and C will be
> > builtin instead of modular.
> >
> > Fix this by making the various compression options depend directly on
> > F2FS_FS using a big if/endif block. Drop the now superfluous
> > dependencies on F2FS_FS from individual symbols.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > Perhaps the propagation logic in Kconfig should be fixed instead?
> > Else people may reintroduce this issue when removing seemingly-unneeded
> > dependencies.
>
> I checked the code in menu_finalize(), and this seems to work like this.
>
> I discussed the oddity of the select behavior before
> (https://lore.kernel.org/linux-kbuild/[email protected]/),
> but I was not confident about what the right direction was.
>
>
> Anyway, the behavior is obscure from the current code.
>
> If you want to make this more robust,
> you can write as follows:
>
> config F2FS_FS
> tristate "F2FS filesystem support"
> depends on BLOCK
> select NLS
> select CRYPTO
> select CRYPTO_CRC32
> select F2FS_FS_XATTR if FS_ENCRYPTION
> select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
> select LZO_COMPRESS if F2FS_FS_LZO
> select LZO_DECOMPRESS if F2FS_FS_LZO
> select LZ4_COMPRESS if F2FS_FS_LZ4
> select LZ4_DECOMPRESS if F2FS_FS_LZ4
> select LZ4HC_COMPRESS if F2FS_FS_LZ4HC
> select ZSTD_COMPRESS if F2FS_FS_ZSTD
> select ZSTD_DECOMPRESS if F2FS_FS_ZSTD
>
> The code is a bit clumsy, but it is clear
> that the module (F2FS_FS) is selecting the
> compress/decompress libraries.
Actually the above is what I tried first ;-) Works fine.
Then I started to look for similar cases in other file systems (e.g.
EROFS_FS_ZIP), and discovered the issue doesn't happen there, which
sparked my investigation. So I settled on the direct dependency,
because it keeps all compression-related logic together.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On 2021/2/23 15:42, Geert Uytterhoeven wrote:
>> I checked the code in menu_finalize(), and this seems to work like this.
>>
>> I discussed the oddity of the select behavior before
>> (https://lore.kernel.org/linux-kbuild/[email protected]/),
>> but I was not confident about what the right direction was.
>>
>>
>> Anyway, the behavior is obscure from the current code.
>>
>> If you want to make this more robust,
>> you can write as follows:
>>
>> config F2FS_FS
>> tristate "F2FS filesystem support"
>> depends on BLOCK
>> select NLS
>> select CRYPTO
>> select CRYPTO_CRC32
>> select F2FS_FS_XATTR if FS_ENCRYPTION
>> select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
>> select LZO_COMPRESS if F2FS_FS_LZO
>> select LZO_DECOMPRESS if F2FS_FS_LZO
>> select LZ4_COMPRESS if F2FS_FS_LZ4
>> select LZ4_DECOMPRESS if F2FS_FS_LZ4
>> select LZ4HC_COMPRESS if F2FS_FS_LZ4HC
>> select ZSTD_COMPRESS if F2FS_FS_ZSTD
>> select ZSTD_DECOMPRESS if F2FS_FS_ZSTD
>>
>> The code is a bit clumsy, but it is clear
>> that the module (F2FS_FS) is selecting the
>> compress/decompress libraries.
> Actually the above is what I tried first ;-) Works fine.
>
> Then I started to look for similar cases in other file systems (e.g.
> EROFS_FS_ZIP), and discovered the issue doesn't happen there, which
> sparked my investigation. So I settled on the direct dependency,
> because it keeps all compression-related logic together.
It looks above way is more explicit, how about using your previous implementation?
Thank,
>
> Gr{oetje,eeting}s,