Re: [PATCH v6 4/8] init: add support for zstd compressed kernel

[Nick Terrell]

> On Jul 7, 2020, at 3:19 AM, Norbert Lange <nolange79@xxxxxxxxx> wrote:
> 
> Thanks for the respin.
> 
> Am Di., 7. Juli 2020 um 05:51 Uhr schrieb Nick Terrell <nickrterrell@xxxxxxxxx>:
>> 
>> From: Nick Terrell <terrelln@xxxxxx>
>> 
>> * Adds the zstd cmd to scripts/Makefile.lib
>> * Adds the HAVE_KERNEL_ZSTD and KERNEL_ZSTD options
>> 
>> Architecture specific support is still needed for decompression.
>> 
>> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> Tested-by: Sedat Dilek <sedat.dilek@xxxxxxxxx>
>> Signed-off-by: Nick Terrell <terrelln@xxxxxx>
>> ---
>> init/Kconfig         | 15 ++++++++++++++-
>> scripts/Makefile.lib | 15 +++++++++++++++
>> 2 files changed, 29 insertions(+), 1 deletion(-)
>> 
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 0498af567f70..8d99f0c5e240 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -191,13 +191,16 @@ config HAVE_KERNEL_LZO
>> config HAVE_KERNEL_LZ4
>>        bool
>> 
>> +config HAVE_KERNEL_ZSTD
>> +       bool
>> +
>> config HAVE_KERNEL_UNCOMPRESSED
>>        bool
>> 
>> choice
>>        prompt "Kernel compression mode"
>>        default KERNEL_GZIP
>> -       depends on HAVE_KERNEL_GZIP || HAVE_KERNEL_BZIP2 || HAVE_KERNEL_LZMA || HAVE_KERNEL_XZ || HAVE_KERNEL_LZO || HAVE_KERNEL_LZ4 || HAVE_KERNEL_UNCOMPRESSED
>> +       depends on HAVE_KERNEL_GZIP || HAVE_KERNEL_BZIP2 || HAVE_KERNEL_LZMA || HAVE_KERNEL_XZ || HAVE_KERNEL_LZO || HAVE_KERNEL_LZ4 || HAVE_KERNEL_ZSTD || HAVE_KERNEL_UNCOMPRESSED
>>        help
>>          The linux kernel is a kind of self-extracting executable.
>>          Several compression algorithms are available, which differ
>> @@ -276,6 +279,16 @@ config KERNEL_LZ4
>>          is about 8% bigger than LZO. But the decompression speed is
>>          faster than LZO.
>> 
>> +config KERNEL_ZSTD
>> +       bool "ZSTD"
>> +       depends on HAVE_KERNEL_ZSTD
>> +       help
>> +         ZSTD is a compression algorithm targeting intermediate compression
>> +         with fast decompression speed. It will compress better than GZIP and
>> +         decompress around the same speed as LZO, but slower than LZ4. You
>> +         will need at least 192 KB RAM or more for booting. The zstd command
>> +         line tools is required for compression.
>> +
>> config KERNEL_UNCOMPRESSED
>>        bool "None"
>>        depends on HAVE_KERNEL_UNCOMPRESSED
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 916b2f7f7098..d960f8815f87 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -413,6 +413,21 @@ quiet_cmd_xzkern = XZKERN  $@
>> quiet_cmd_xzmisc = XZMISC  $@
>>       cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@
>> 
>> +# ZSTD
>> +# ---------------------------------------------------------------------------
>> +# Appends the uncompressed size of the data using size_append. The .zst
>> +# format has the size information available at the beginning of the file too,
>> +# but it's in a more complex format and it's good to avoid changing the part
>> +# of the boot code that reads the uncompressed size.
>> +# Note that the bytes added by size_append will make the zstd tool think that
>> +# the file is corrupt. This is expected.
>> +
>> +quiet_cmd_zstd = ZSTD    $@
>> +cmd_zstd = (cat $(filter-out FORCE,$^) | \
>> +       zstd -19 && \
>> +        $(call size_append, $(filter-out FORCE,$^))) > $@ || \
>> +       (rm -f $@ ; false)
> 
> Is there any reason not to use '--ultra -22' ?
> As far as I understand the other patches, the decompression should be
> able to handle it,
> and in terms of time required for a kernel build the difference is
> insignificant.

For kernel compression there isnât a strong reason not to use `--ultra -22`.
It may slow down decompression a small amount, because the difference
is that it has a 128 MB window size instead of a 8 MB window size.

However, that probably isnât want you want for initramfs compression,
which can optionally now use this same command. We could go like xz
and have both cmd_zstdmisc and cmd_zstdkern, and only use `-22` for the
kernel version.

It also looks like there were a few minor changes made to the other
compress cmds in this file while these patches have been out, so I will apply
them to zstd as well.

Iâll submit a new version with these changes today.

> And would it be better to run zstd on a prepared file instead of
> stream enconding?
> windowsize would be adjusted to min(windowsize, filesize) for one.

Yeah, that would be helpful for initramfs compression when the file is
smaller than the window size, since it would limit the memory necessary
For decompression. But, it wouldnât help kernel compression.

For that weâd have to create a temporary file, because it looks like these
commands potentially accept more than one input file. Do you know the
standard practice for temporary files in the build system?

Thanks for the review,
Nick