2023-09-15 14:31:06

by Martin Nybo Andersen

Subject: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

Kmod is now using the kernel decompressor which doesn't handle CRC64
and dictionaries larger than 1MiB.

Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
Signed-off-by: Martin Nybo Andersen <[email protected]>
---
scripts/Makefile.modinst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index c59cc57286ba..ffbafbd3aeea 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -144,7 +144,7 @@ endif
quiet_cmd_gzip = GZIP $@
cmd_gzip = $(KGZIP) -n -f $<
quiet_cmd_xz = XZ $@
- cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
+ cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
quiet_cmd_zstd = ZSTD $@
cmd_zstd = $(ZSTD) -T0 --rm -f -q $<

--
2.40.1

2023-09-22 07:50:40

by Martin Nybo Andersen

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

Hello?
Anyone?

Best regards,
- Martin

On Fri, 15 Sep 2023, Martin Nybo Andersen wrote:

> Kmod is now using the kernel decompressor which doesn't handle CRC64
> and dictionaries larger than 1MiB.
>
> Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> Signed-off-by: Martin Nybo Andersen <[email protected]>
> ---
> scripts/Makefile.modinst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index c59cc57286ba..ffbafbd3aeea 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -144,7 +144,7 @@ endif
> quiet_cmd_gzip = GZIP $@
> cmd_gzip = $(KGZIP) -n -f $<
> quiet_cmd_xz = XZ $@
> - cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
> + cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
> quiet_cmd_zstd = ZSTD $@
> cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
>
> --
> 2.40.1
>
>

2023-09-22 08:40:41

by Tor Vic

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

>
> Hello?

Hello,

> Anyone?
>
> Best regards,
> - Martin
>
> On Fri, 15 Sep 2023, Martin Nybo Andersen wrote:
>
>> Kmod is now using the kernel decompressor which doesn't handle CRC64
>> and dictionaries larger than 1MiB.
>>
>> Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
>> Signed-off-by: Martin Nybo Andersen <[email protected]>
>> ---
>> scripts/Makefile.modinst | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
>> index c59cc57286ba..ffbafbd3aeea 100644
>> --- a/scripts/Makefile.modinst
>> +++ b/scripts/Makefile.modinst
>> @@ -144,7 +144,7 @@ endif
>> quiet_cmd_gzip = GZIP    $@
>>       cmd_gzip = $(KGZIP) -n -f $<
>> quiet_cmd_xz = XZ      $@
>> -      cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
>> +      cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<

I wonder whether it should be guarded with
ifdef CONFIG_MODULE_DECOMPRESS

But on the other hand, the difference between 1M and 2M is likely very
small in terms of compression ratio.

Cheers,
Tor Vic

>> quiet_cmd_zstd = ZSTD    $@
>>       cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
>>
>> --
>> 2.40.1
>>
>>
>

2023-09-22 09:06:44

by Martin Nybo Andersen

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

On Fri, 22 Sep 2023, Tor Vic wrote:

>>
>> Hello?
>
> Hello,

:)

>> Anyone?
>>
>> Best regards,
>> - Martin
>>
>> On Fri, 15 Sep 2023, Martin Nybo Andersen wrote:
>>
>>> Kmod is now using the kernel decompressor which doesn't handle CRC64
>>> and dictionaries larger than 1MiB.
>>>
>>> Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
>>> Signed-off-by: Martin Nybo Andersen <[email protected]>
>>> ---
>>> scripts/Makefile.modinst | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
>>> index c59cc57286ba..ffbafbd3aeea 100644
>>> --- a/scripts/Makefile.modinst
>>> +++ b/scripts/Makefile.modinst
>>> @@ -144,7 +144,7 @@ endif
>>> quiet_cmd_gzip = GZIP    $@
>>>       cmd_gzip = $(KGZIP) -n -f $<
>>> quiet_cmd_xz = XZ      $@
>>> -      cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
>>> +      cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
>
> I wonder whether it should be guarded with
> ifdef CONFIG_MODULE_DECOMPRESS
>
> But on the other hand, the difference between 1M and 2M is likely very small
> in terms of compression ratio.

That would work, since kmod decompress the modules in userspace if
CONFIG_MODULE_DECOMPRESS if off.

What I'm aiming for is just that the kernel should be able to decompress
its own modules, when it advertises it.

Cheers,
Martin

> Cheers,
> Tor Vic
>
>>> quiet_cmd_zstd = ZSTD    $@
>>>       cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
>>>
>>> --
>>> 2.40.1
>>>
>>>
>>
>
>

2023-09-22 10:48:39

by Masahiro Yamada

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

On Fri, Sep 22, 2023 at 5:52 PM Martin Nybo Andersen <[email protected]> wrote:
>
> On Fri, 22 Sep 2023, Tor Vic wrote:
>
> >>
> >> Hello?
> >
> > Hello,
>
> :)
>
> >> Anyone?
> >>
> >> Best regards,
> >> - Martin
> >>
> >> On Fri, 15 Sep 2023, Martin Nybo Andersen wrote:
> >>
> >>> Kmod is now using the kernel decompressor which doesn't handle CRC64
> >>> and dictionaries larger than 1MiB.
> >>>
> >>> Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> >>> Signed-off-by: Martin Nybo Andersen <[email protected]>
> >>> ---
> >>> scripts/Makefile.modinst | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> >>> index c59cc57286ba..ffbafbd3aeea 100644
> >>> --- a/scripts/Makefile.modinst
> >>> +++ b/scripts/Makefile.modinst
> >>> @@ -144,7 +144,7 @@ endif
> >>> quiet_cmd_gzip = GZIP $@
> >>> cmd_gzip = $(KGZIP) -n -f $<
> >>> quiet_cmd_xz = XZ $@
> >>> - cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
> >>> + cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
> >
> > I wonder whether it should be guarded with
> > ifdef CONFIG_MODULE_DECOMPRESS
> >
> > But on the other hand, the difference between 1M and 2M is likely very small
> > in terms of compression ratio.
>
> That would work, since kmod decompress the modules in userspace if
> CONFIG_MODULE_DECOMPRESS if off.
>
> What I'm aiming for is just that the kernel should be able to decompress
> its own modules, when it advertises it.


Hello, sorry for the delay.

I am OK with picking the patch to my tree.


I think we can use --check=crc32 --lzma2=dict=1MiB
unconditionally.

(But, if the in-kernel decompressor is improved
to understand CRC64 in the future, we can loosen
this restriction again.)



Just small change requests.

Please do not use the Fixes tag to point at the Debian bug tracker.


Instead, you can do

Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582

or

Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582



Also, mention that this change happened in

kmod commit 09c9f8c5df04 ("libkmod: Use kernel decompression when available")

in the commit description might be helpful.




I added Lukas to CC.
He is the author of the change on kmod side.






> Cheers,
> Martin
>
> > Cheers,
> > Tor Vic
> >
> >>> quiet_cmd_zstd = ZSTD $@
> >>> cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
> >>>
> >>> --
> >>> 2.40.1
> >>>
> >>>
> >>
> >
> >



--
Best Regards
Masahiro Yamada

2023-09-22 11:26:35

by Martin Nybo Andersen

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

On Fri, 22 Sep 2023, Masahiro Yamada wrote:

[...]

> Hello, sorry for the delay.
>
> I am OK with picking the patch to my tree.

Hi,

That sound great :)

> I think we can use --check=crc32 --lzma2=dict=1MiB
> unconditionally.
>
> (But, if the in-kernel decompressor is improved
> to understand CRC64 in the future, we can loosen
> this restriction again.)

Of course.

> Just small change requests.
>
> Please do not use the Fixes tag to point at the Debian bug tracker.
>
>
> Instead, you can do
>
> Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
>
> or
>
> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
>
>
>
> Also, mention that this change happened in
>
> kmod commit 09c9f8c5df04 ("libkmod: Use kernel decompression when available")
>
> in the commit description might be helpful.
>
>
>
>
> I added Lukas to CC.
> He is the author of the change on kmod side.

Is this ok?

Kmod is now (since kmod commit 09c9f8c5df04 ("libkmod: Use kernel
decompression when available")) using the kernel decompressor, when
loading compressed modules.

However, the kernel XZ decompressor is XZ Embedded, which doesn't
handle CRC64 and dictionaries larger than 1MiB.

Use CRC32 and 1MiB dictionary when XZ compressing and installing
kernel modules.

Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
Signed-off-by: Martin Nybo Andersen <[email protected]>
---
scripts/Makefile.modinst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index c59cc57286ba..ffbafbd3aeea 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -144,7 +144,7 @@ endif
quiet_cmd_gzip = GZIP $@
cmd_gzip = $(KGZIP) -n -f $<
quiet_cmd_xz = XZ $@
- cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
+ cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
quiet_cmd_zstd = ZSTD $@
cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
--
2.40.1

2023-09-22 13:45:57

by Masahiro Yamada

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

On Fri, Sep 22, 2023 at 8:04 PM Martin Nybo Andersen <[email protected]> wrote:
>
>
> On Fri, 22 Sep 2023, Masahiro Yamada wrote:
>
> [...]
>
> > Hello, sorry for the delay.
> >
> > I am OK with picking the patch to my tree.
>
> Hi,
>
> That sound great :)
>
> > I think we can use --check=crc32 --lzma2=dict=1MiB
> > unconditionally.
> >
> > (But, if the in-kernel decompressor is improved
> > to understand CRC64 in the future, we can loosen
> > this restriction again.)
>
> Of course.
>
> > Just small change requests.
> >
> > Please do not use the Fixes tag to point at the Debian bug tracker.
> >
> >
> > Instead, you can do
> >
> > Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> >
> > or
> >
> > Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> >
> >
> >
> > Also, mention that this change happened in
> >
> > kmod commit 09c9f8c5df04 ("libkmod: Use kernel decompression when available")
> >
> > in the commit description might be helpful.
> >
> >
> >
> >
> > I added Lukas to CC.
> > He is the author of the change on kmod side.
>
> Is this ok?


Yes, perfect!

Thanks.


>
> Kmod is now (since kmod commit 09c9f8c5df04 ("libkmod: Use kernel
> decompression when available")) using the kernel decompressor, when
> loading compressed modules.
>
> However, the kernel XZ decompressor is XZ Embedded, which doesn't
> handle CRC64 and dictionaries larger than 1MiB.
>
> Use CRC32 and 1MiB dictionary when XZ compressing and installing
> kernel modules.
>
> Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> Signed-off-by: Martin Nybo Andersen <[email protected]>
> ---
> scripts/Makefile.modinst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index c59cc57286ba..ffbafbd3aeea 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -144,7 +144,7 @@ endif
> quiet_cmd_gzip = GZIP $@
> cmd_gzip = $(KGZIP) -n -f $<
> quiet_cmd_xz = XZ $@
> - cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
> + cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
> quiet_cmd_zstd = ZSTD $@
> cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
> --
> 2.40.1



--
Best Regards
Masahiro Yamada

2023-09-22 16:56:16

by Nicolas Schier

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

In [email protected] probably some more experts can comment on this:

On Fri, Sep 15, 2023 at 12:15:39PM +0200, Martin Nybo Andersen wrote:
> Kmod is now using the kernel decompressor which doesn't handle CRC64
> and dictionaries larger than 1MiB.
>
> Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> Signed-off-by: Martin Nybo Andersen <[email protected]>
> ---
> scripts/Makefile.modinst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index c59cc57286ba..ffbafbd3aeea 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -144,7 +144,7 @@ endif
> quiet_cmd_gzip = GZIP $@
> cmd_gzip = $(KGZIP) -n -f $<
> quiet_cmd_xz = XZ $@
> - cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
> + cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
> quiet_cmd_zstd = ZSTD $@
> cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
>
> --
> 2.40.1

2023-09-25 07:22:22

by Masahiro Yamada

Subject: Re: [PATCH] Use CRC32 and a 1MiB dictionary for XZ compressed modules

On Fri, Sep 22, 2023 at 10:34 PM Masahiro Yamada <[email protected]> wrote:
>
> On Fri, Sep 22, 2023 at 8:04 PM Martin Nybo Andersen <[email protected]> wrote:
> >
> >
> > On Fri, 22 Sep 2023, Masahiro Yamada wrote:
> >
> > [...]
> >
> > > Hello, sorry for the delay.
> > >
> > > I am OK with picking the patch to my tree.
> >
> > Hi,
> >
> > That sound great :)
> >
> > > I think we can use --check=crc32 --lzma2=dict=1MiB
> > > unconditionally.
> > >
> > > (But, if the in-kernel decompressor is improved
> > > to understand CRC64 in the future, we can loosen
> > > this restriction again.)
> >
> > Of course.
> >
> > > Just small change requests.
> > >
> > > Please do not use the Fixes tag to point at the Debian bug tracker.
> > >
> > >
> > > Instead, you can do
> > >
> > > Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> > >
> > > or
> > >
> > > Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050582
> > >
> > >
> > >
> > > Also, mention that this change happened in
> > >
> > > kmod commit 09c9f8c5df04 ("libkmod: Use kernel decompression when available")
> > >
> > > in the commit description might be helpful.
> > >
> > >
> > >
> > >
> > > I added Lukas to CC.
> > > He is the author of the change on kmod side.
> >
> > Is this ok?
>
>
> Yes, perfect!
>
> Thanks.
>


Applied to linux-kbuild. Thanks.




--
Best Regards
Masahiro Yamada