[PATCH v2 1/2] kbuild: use perl instead of shell to get file size

[PATCH v2 1/2] kbuild: use perl instead of shell to get file size

[Alex Xu (Hello71)]

This makes it easier to get the size of multiple files. Perl is already
a requirement for all builds to do header checks, so this is not an
additional dependency.
---
 arch/arm/boot/deflate_xip_data.sh | 2 +-
 arch/powerpc/boot/wrapper         | 2 +-
 scripts/Makefile.lib              | 9 ++-------
 scripts/file-size.pl              | 8 ++++++++
 scripts/file-size.sh              | 4 ----
 scripts/link-vmlinux.sh           | 4 ++--
 6 files changed, 14 insertions(+), 15 deletions(-)
 create mode 100755 scripts/file-size.pl
 delete mode 100755 scripts/file-size.sh

diff --git a/arch/arm/boot/deflate_xip_data.sh b/arch/arm/boot/deflate_xip_data.sh
index 304495c3c2c5..14cfa2babb93 100755
--- a/arch/arm/boot/deflate_xip_data.sh
+++ b/arch/arm/boot/deflate_xip_data.sh
@@ -43,7 +43,7 @@ data_start=$(($__data_loc - $base_offset))
 data_end=$(($_edata_loc - $base_offset))
 
 # Make sure data occupies the last part of the file.
-file_end=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" "$XIPIMAGE")
+file_end=$(${PERL} "${srctree}/scripts/file-size.pl" "$XIPIMAGE")
 if [ "$file_end" != "$data_end" ]; then
 	printf "end of xipImage doesn't match with _edata_loc (%#x vs %#x)\n" \
 	       $(($file_end + $base_offset)) $_edata_loc 1>&2
diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index 9184eda780fd..9f9ee8613432 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -380,7 +380,7 @@ vmz="$tmpdir/`basename \"$kernel\"`.$ext"
 
 # Calculate the vmlinux.strip size
 ${CROSS}objcopy $objflags "$kernel" "$vmz.$$"
-strip_size=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" "$vmz.$$")
+strip_size=$(${PERL} "${srctree}/scripts/file-size.pl" "$vmz.$$")
 
 if [ -z "$cacheit" -o ! -f "$vmz$compression" -o "$vmz$compression" -ot "$kernel" ]; then
     # recompress the image if we need to
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index d1f865b8c0cb..ca901814986a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -379,13 +379,8 @@ dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
 
 # Bzip2 and LZMA do not include size in file... so we have to fake that;
 # append the size as a 32-bit littleendian number as gzip does.
-size_append = printf $(shell						\
-dec_size=0;								\
-for F in $(real-prereqs); do					\
-	fsize=$$($(CONFIG_SHELL) $(srctree)/scripts/file-size.sh $$F);	\
-	dec_size=$$(expr $$dec_size + $$fsize);				\
-done;									\
-printf "%08x\n" $$dec_size |						\
+total_size = $(shell $(PERL) $(srctree)/scripts/file-size.pl $(real-prereqs))
+size_append = printf $(shell printf "%08x\n" $(total_size) |		\
 	sed 's/\(..\)/\1 /g' | {					\
 		read ch0 ch1 ch2 ch3;					\
 		for ch in $$ch3 $$ch2 $$ch1 $$ch0; do			\
diff --git a/scripts/file-size.pl b/scripts/file-size.pl
new file mode 100755
index 000000000000..170bb6d048fa
--- /dev/null
+++ b/scripts/file-size.pl
@@ -0,0 +1,8 @@
+#!/usr/bin/perl -w
+# SPDX-License-Identifier: GPL-2.0
+my $total = 0;
+foreach (@ARGV) {
+    @stat = stat $_ or die "$_: $!";
+    $total += $stat[7];
+}
+print "$total\n";
diff --git a/scripts/file-size.sh b/scripts/file-size.sh
deleted file mode 100755
index 7eb7423416b5..000000000000
--- a/scripts/file-size.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-set -- $(ls -dn "$1")
-printf '%s\n' "$5"
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 5cdd9bc5c385..c3fa38bd18ab 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -384,8 +384,8 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
 	kallsyms_step 2
 
 	# step 3
-	size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso_prev})
-	size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
+	size1=$(${PERL} "${srctree}/scripts/file-size.pl" ${kallsymso_prev})
+	size2=$(${PERL} "${srctree}/scripts/file-size.pl" ${kallsymso})
 
 	if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
 		kallsyms_step 3
-- 
2.34.0

[PATCH v2 2/2] kbuild: pass --stream-size --no-content-size to zstd

[Alex Xu (Hello71)]

Otherwise, it allocates 2 GB of memory at once. Even though the majority
of this memory is never touched, the default heuristic overcommit
refuses this request if less than 2 GB of RAM+swap is currently
available. This results in "zstd: error 11 : Allocation error : not
enough memory" and the kernel failing to build.

When the size is specified, zstd will reduce the memory request
appropriately. For typical kernel sizes of ~32 MB, the largest mmap
request will be reduced to 512 MB, which will succeed on all but the
smallest devices.

For inputs around this size, --stream-size --no-content-size may
slightly decrease the compressed size, or slightly increase it:
https://github.com/facebook/zstd/issues/2848.

Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>
---
 scripts/Makefile.lib | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index ca901814986a..c98a82ca38e6 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -466,12 +466,20 @@ quiet_cmd_xzmisc = XZMISC  $@
 # single pass, so zstd doesn't need to allocate a window buffer. When streaming
 # decompression is used, like initramfs decompression, zstd22 should likely not
 # be used because it would require zstd to allocate a 128 MB buffer.
+#
+# --stream-size to reduce zstd memory usage (otherwise zstd -22 --ultra
+# allocates, but does not use, 2 GB) and potentially improve compression.
+#
+# --no-content-size to save three bytes which we do not use (we use size_append).
+
+# zstd --stream-size is only supported since 1.4.4
+zstd_stream_size = $(shell $(ZSTD) -1c --stream-size=0 --no-content-size </dev/null >/dev/null 2>&1 && printf '%s' '--stream-size=$(total_size) --no-content-size')
 
 quiet_cmd_zstd = ZSTD    $@
-      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
+      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -19; $(size_append); } > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
-      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
+      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -22 --ultra; $(size_append); } > $@
 
 # ASM offsets
 # ---------------------------------------------------------------------------
-- 
2.34.0

Re: [PATCH v2 1/2] kbuild: use perl instead of shell to get file size

[Nick Terrell]

> On Nov 24, 2021, at 7:31 AM, Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
> 
> This makes it easier to get the size of multiple files. Perl is already
> a requirement for all builds to do header checks, so this is not an
> additional dependency.
> ---
> arch/arm/boot/deflate_xip_data.sh | 2 +-
> arch/powerpc/boot/wrapper         | 2 +-
> scripts/Makefile.lib              | 9 ++-------
> scripts/file-size.pl              | 8 ++++++++
> scripts/file-size.sh              | 4 ----
> scripts/link-vmlinux.sh           | 4 ++--
> 6 files changed, 14 insertions(+), 15 deletions(-)
> create mode 100755 scripts/file-size.pl
> delete mode 100755 scripts/file-size.sh
> 
> diff --git a/arch/arm/boot/deflate_xip_data.sh b/arch/arm/boot/deflate_xip_data.sh
> index 304495c3c2c5..14cfa2babb93 100755
> --- a/arch/arm/boot/deflate_xip_data.sh
> +++ b/arch/arm/boot/deflate_xip_data.sh
> @@ -43,7 +43,7 @@ data_start=$(($__data_loc - $base_offset))
> data_end=$(($_edata_loc - $base_offset))
> 
> # Make sure data occupies the last part of the file.
> -file_end=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" "$XIPIMAGE")
> +file_end=$(${PERL} "${srctree}/scripts/file-size.pl" "$XIPIMAGE")
> if [ "$file_end" != "$data_end" ]; then
> 	printf "end of xipImage doesn't match with _edata_loc (%#x vs %#x)\n" \
> 	       $(($file_end + $base_offset)) $_edata_loc 1>&2
> diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
> index 9184eda780fd..9f9ee8613432 100755
> --- a/arch/powerpc/boot/wrapper
> +++ b/arch/powerpc/boot/wrapper
> @@ -380,7 +380,7 @@ vmz="$tmpdir/`basename \"$kernel\"`.$ext"
> 
> # Calculate the vmlinux.strip size
> ${CROSS}objcopy $objflags "$kernel" "$vmz.$$"
> -strip_size=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" "$vmz.$$")
> +strip_size=$(${PERL} "${srctree}/scripts/file-size.pl" "$vmz.$$")
> 
> if [ -z "$cacheit" -o ! -f "$vmz$compression" -o "$vmz$compression" -ot "$kernel" ]; then
>     # recompress the image if we need to
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index d1f865b8c0cb..ca901814986a 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -379,13 +379,8 @@ dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
> 
> # Bzip2 and LZMA do not include size in file... so we have to fake that;
> # append the size as a 32-bit littleendian number as gzip does.
> -size_append = printf $(shell						\
> -dec_size=0;								\
> -for F in $(real-prereqs); do					\
> -	fsize=$$($(CONFIG_SHELL) $(srctree)/scripts/file-size.sh $$F);	\
> -	dec_size=$$(expr $$dec_size + $$fsize);				\
> -done;									\
> -printf "%08x\n" $$dec_size |						\
> +total_size = $(shell $(PERL) $(srctree)/scripts/file-size.pl $(real-prereqs))
> +size_append = printf $(shell printf "%08x\n" $(total_size) |		\
> 	sed 's/\(..\)/\1 /g' | {					\
> 		read ch0 ch1 ch2 ch3;					\
> 		for ch in $$ch3 $$ch2 $$ch1 $$ch0; do			\
> diff --git a/scripts/file-size.pl b/scripts/file-size.pl
> new file mode 100755
> index 000000000000..170bb6d048fa
> --- /dev/null
> +++ b/scripts/file-size.pl
> @@ -0,0 +1,8 @@
> +#!/usr/bin/perl -w
> +# SPDX-License-Identifier: GPL-2.0
> +my $total = 0;
> +foreach (@ARGV) {
> +    @stat = stat $_ or die "$_: $!";
> +    $total += $stat[7];
> +}
> +print "$total\n";
> diff --git a/scripts/file-size.sh b/scripts/file-size.sh
> deleted file mode 100755
> index 7eb7423416b5..000000000000
> --- a/scripts/file-size.sh
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -set -- $(ls -dn "$1")
> -printf '%s\n' "$5"
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 5cdd9bc5c385..c3fa38bd18ab 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -384,8 +384,8 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
> 	kallsyms_step 2
> 
> 	# step 3
> -	size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso_prev})
> -	size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
> +	size1=$(${PERL} "${srctree}/scripts/file-size.pl" ${kallsymso_prev})
> +	size2=$(${PERL} "${srctree}/scripts/file-size.pl" ${kallsymso})
> 
> 	if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
> 		kallsyms_step 3
> -- 
> 2.34.0
> 

You can add:

Tested-by: Nick Terrell <terrelln@fb.com>

Re: [PATCH v2 2/2] kbuild: pass --stream-size --no-content-size to zstd

[Nick Terrell]

> On Nov 24, 2021, at 7:31 AM, Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
> 
> Otherwise, it allocates 2 GB of memory at once. Even though the majority
> of this memory is never touched, the default heuristic overcommit
> refuses this request if less than 2 GB of RAM+swap is currently
> available. This results in "zstd: error 11 : Allocation error : not
> enough memory" and the kernel failing to build.
> 
> When the size is specified, zstd will reduce the memory request
> appropriately. For typical kernel sizes of ~32 MB, the largest mmap
> request will be reduced to 512 MB, which will succeed on all but the
> smallest devices.
> 
> For inputs around this size, --stream-size --no-content-size may
> slightly decrease the compressed size, or slightly increase it:
> https://github.com/facebook/zstd/issues/2848.
> 
> Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>
> ---
> scripts/Makefile.lib | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index ca901814986a..c98a82ca38e6 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -466,12 +466,20 @@ quiet_cmd_xzmisc = XZMISC  $@
> # single pass, so zstd doesn't need to allocate a window buffer. When streaming
> # decompression is used, like initramfs decompression, zstd22 should likely not
> # be used because it would require zstd to allocate a 128 MB buffer.
> +#
> +# --stream-size to reduce zstd memory usage (otherwise zstd -22 --ultra
> +# allocates, but does not use, 2 GB) and potentially improve compression.
> +#
> +# --no-content-size to save three bytes which we do not use (we use size_append).
> +
> +# zstd --stream-size is only supported since 1.4.4
> +zstd_stream_size = $(shell $(ZSTD) -1c --stream-size=0 --no-content-size </dev/null >/dev/null 2>&1 && printf '%s' '--stream-size=$(total_size) --no-content-size')
> 
> quiet_cmd_zstd = ZSTD    $@
> -      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
> +      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -19; $(size_append); } > $@
> 
> quiet_cmd_zstd22 = ZSTD22  $@
> -      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> +      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -22 --ultra; $(size_append); } > $@
> 
> # ASM offsets
> # ---------------------------------------------------------------------------
> -- 
> 2.34.0
> 

You can add:

Tested-by: Nick Terrell <terrelln@fb.com>
Reviewed-by: Nick Terrell <terrelln@fb.com>

Best,
Nick Terrell

Re: [PATCH v2 2/2] kbuild: pass --stream-size --no-content-size to zstd

[Masahiro Yamada]

On Thu, Nov 25, 2021 at 12:30 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>
> Otherwise, it allocates 2 GB of memory at once. Even though the majority
> of this memory is never touched, the default heuristic overcommit
> refuses this request if less than 2 GB of RAM+swap is currently
> available. This results in "zstd: error 11 : Allocation error : not
> enough memory" and the kernel failing to build.
>
> When the size is specified, zstd will reduce the memory request
> appropriately. For typical kernel sizes of ~32 MB, the largest mmap
> request will be reduced to 512 MB, which will succeed on all but the
> smallest devices.
>
> For inputs around this size, --stream-size --no-content-size may
> slightly decrease the compressed size, or slightly increase it:
> https://github.com/facebook/zstd/issues/2848.
>
> Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>




The reason why we need this workaround is just because we do
"cat and compress".  zstd must allocate a huge memory beforehand
since it cannot predict how long the stream it will receive.

If zstd is given with a file name, it can fstat it to know its file size
and allocate the minimal amount of memory.


This is my test.
I used 'ulimit' to set the upper limit of the memory the zstd can use.


[test steps]

  # Create a 1kB file
  $ truncate --size=1k dummy

  # Set the memory size limit to 10MB
  $ ulimit -S -v 10240

  # Pass the file as a argument; success
  $ zstd -19 -o dummy.zst dummy
  dummy                :  2.15%   (  1024 =>     22 bytes, dummy.zst)

  # cat and zstd; fail
  $ cat dummy | zstd -19 > dummy.zst
  zstd: error 11 : Allocation error : not enough memory

  # cat and zstd --stream-size; success
  $ cat dummy | zstd -19 --stream-size=1024 > dummy.zst




scripts/Makefile.modinst was written in such a way
that zstd can know the file size by itself.

      cmd_zstd = $(ZSTD) -T0 --rm -f -q $<


We cannot rewrite scripts/Makefile.lib in that way because
arch/x86/boot/compress/Makefile concatenates two files before
compression. And this is the only use-case of this feature.

So, I am seriously considering to revert this commit:

commit d3dd3b5a29bb9582957451531fed461628dfc834
Author: H. Peter Anvin <hpa@zytor.com>
Date:   Tue May 5 21:17:15 2009 -0700

    kbuild: allow compressors (gzip, bzip2, lzma) to take multiple inputs




With that commit reverted, zstd will take a single input file,
and we can do "zstd -o <output> <input>".


So, I will take some time to investigate that approach.




> ---
>  scripts/Makefile.lib | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index ca901814986a..c98a82ca38e6 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -466,12 +466,20 @@ quiet_cmd_xzmisc = XZMISC  $@
>  # single pass, so zstd doesn't need to allocate a window buffer. When streaming
>  # decompression is used, like initramfs decompression, zstd22 should likely not
>  # be used because it would require zstd to allocate a 128 MB buffer.
> +#
> +# --stream-size to reduce zstd memory usage (otherwise zstd -22 --ultra
> +# allocates, but does not use, 2 GB) and potentially improve compression.
> +#
> +# --no-content-size to save three bytes which we do not use (we use size_append).
> +
> +# zstd --stream-size is only supported since 1.4.4
> +zstd_stream_size = $(shell $(ZSTD) -1c --stream-size=0 --no-content-size </dev/null >/dev/null 2>&1 && printf '%s' '--stream-size=$(total_size) --no-content-size')
>
>  quiet_cmd_zstd = ZSTD    $@
> -      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
> +      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -19; $(size_append); } > $@
>
>  quiet_cmd_zstd22 = ZSTD22  $@
> -      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> +      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -22 --ultra; $(size_append); } > $@
>
>  # ASM offsets
>  # ---------------------------------------------------------------------------
> --
> 2.34.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 2/2] kbuild: pass --stream-size --no-content-size to zstd

[Nick Terrell]

> On Dec 5, 2021, at 2:52 PM, Masahiro Yamada <masahiroy@kernel.org> wrote:
> 
> On Thu, Nov 25, 2021 at 12:30 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>> 
>> Otherwise, it allocates 2 GB of memory at once. Even though the majority
>> of this memory is never touched, the default heuristic overcommit
>> refuses this request if less than 2 GB of RAM+swap is currently
>> available. This results in "zstd: error 11 : Allocation error : not
>> enough memory" and the kernel failing to build.
>> 
>> When the size is specified, zstd will reduce the memory request
>> appropriately. For typical kernel sizes of ~32 MB, the largest mmap
>> request will be reduced to 512 MB, which will succeed on all but the
>> smallest devices.
>> 
>> For inputs around this size, --stream-size --no-content-size may
>> slightly decrease the compressed size, or slightly increase it:
>> https://github.com/facebook/zstd/issues/2848.
>> 
>> Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>
> 
> 
> 
> 
> The reason why we need this workaround is just because we do
> "cat and compress".  zstd must allocate a huge memory beforehand
> since it cannot predict how long the stream it will receive.
> 
> If zstd is given with a file name, it can fstat it to know its file size
> and allocate the minimal amount of memory.
> 
> 
> This is my test.
> I used 'ulimit' to set the upper limit of the memory the zstd can use.
> 
> 
> [test steps]
> 
>  # Create a 1kB file
>  $ truncate --size=1k dummy
> 
>  # Set the memory size limit to 10MB
>  $ ulimit -S -v 10240
> 
>  # Pass the file as a argument; success
>  $ zstd -19 -o dummy.zst dummy
>  dummy                :  2.15%   (  1024 =>     22 bytes, dummy.zst)
> 
>  # cat and zstd; fail
>  $ cat dummy | zstd -19 > dummy.zst
>  zstd: error 11 : Allocation error : not enough memory
> 
>  # cat and zstd --stream-size; success
>  $ cat dummy | zstd -19 --stream-size=1024 > dummy.zst
> 
> 
> 
> 
> scripts/Makefile.modinst was written in such a way
> that zstd can know the file size by itself.
> 
>      cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
> 
> 
> We cannot rewrite scripts/Makefile.lib in that way because
> arch/x86/boot/compress/Makefile concatenates two files before
> compression. And this is the only use-case of this feature.
> 
> So, I am seriously considering to revert this commit:
> 
> commit d3dd3b5a29bb9582957451531fed461628dfc834
> Author: H. Peter Anvin <hpa@zytor.com>
> Date:   Tue May 5 21:17:15 2009 -0700
> 
>    kbuild: allow compressors (gzip, bzip2, lzma) to take multiple inputs
> 
> 
> 
> 
> With that commit reverted, zstd will take a single input file,
> and we can do "zstd -o <output> <input>".
> 
> 
> So, I will take some time to investigate that approach.

This will definitely work from a zstd perspective. All versions
of zstd will downsize their memory usage to match the file size.

Best,
Nick Terrell

>> ---
>> scripts/Makefile.lib | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>> 
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index ca901814986a..c98a82ca38e6 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -466,12 +466,20 @@ quiet_cmd_xzmisc = XZMISC  $@
>> # single pass, so zstd doesn't need to allocate a window buffer. When streaming
>> # decompression is used, like initramfs decompression, zstd22 should likely not
>> # be used because it would require zstd to allocate a 128 MB buffer.
>> +#
>> +# --stream-size to reduce zstd memory usage (otherwise zstd -22 --ultra
>> +# allocates, but does not use, 2 GB) and potentially improve compression.
>> +#
>> +# --no-content-size to save three bytes which we do not use (we use size_append).
>> +
>> +# zstd --stream-size is only supported since 1.4.4
>> +zstd_stream_size = $(shell $(ZSTD) -1c --stream-size=0 --no-content-size </dev/null >/dev/null 2>&1 && printf '%s' '--stream-size=$(total_size) --no-content-size')
>> 
>> quiet_cmd_zstd = ZSTD    $@
>> -      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
>> +      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -19; $(size_append); } > $@
>> 
>> quiet_cmd_zstd22 = ZSTD22  $@
>> -      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
>> +      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -22 --ultra; $(size_append); } > $@
>> 
>> # ASM offsets
>> # ---------------------------------------------------------------------------
>> --
>> 2.34.0
>> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

Re: [PATCH v2 2/2] kbuild: pass --stream-size --no-content-size to zstd

[Sedat Dilek]

On Wed, Nov 24, 2021 at 4:30 PM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>
> Otherwise, it allocates 2 GB of memory at once. Even though the majority
> of this memory is never touched, the default heuristic overcommit
> refuses this request if less than 2 GB of RAM+swap is currently
> available. This results in "zstd: error 11 : Allocation error : not
> enough memory" and the kernel failing to build.
>
> When the size is specified, zstd will reduce the memory request
> appropriately. For typical kernel sizes of ~32 MB, the largest mmap
> request will be reduced to 512 MB, which will succeed on all but the
> smallest devices.
>
> For inputs around this size, --stream-size --no-content-size may
> slightly decrease the compressed size, or slightly increase it:
> https://github.com/facebook/zstd/issues/2848.
>

Hi Alex and Nick T.,

some questions:

Can I apply this patch as a single patch - without patch 1/2?

Is there an impact also on the kernel's ZRAM/ZSWAP support plus using
ZSTD as (de)comp-algo?

Here I have:

$ grep -i zstd /boot/config-5.15.7-1-amd64-clang13-lto | egrep -i 'zram|zswap'
CONFIG_ZSWAP_COMPRESSOR_DEFAULT_ZSTD=y
CONFIG_ZSWAP_COMPRESSOR_DEFAULT="zstd"
CONFIG_ZRAM_DEF_COMP_ZSTD=y
CONFIG_ZRAM_DEF_COMP="zstd"

Thanks.

Regards,
- Sedat -

> Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>
> ---
>  scripts/Makefile.lib | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index ca901814986a..c98a82ca38e6 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -466,12 +466,20 @@ quiet_cmd_xzmisc = XZMISC  $@
>  # single pass, so zstd doesn't need to allocate a window buffer. When streaming
>  # decompression is used, like initramfs decompression, zstd22 should likely not
>  # be used because it would require zstd to allocate a 128 MB buffer.
> +#
> +# --stream-size to reduce zstd memory usage (otherwise zstd -22 --ultra
> +# allocates, but does not use, 2 GB) and potentially improve compression.
> +#
> +# --no-content-size to save three bytes which we do not use (we use size_append).
> +
> +# zstd --stream-size is only supported since 1.4.4
> +zstd_stream_size = $(shell $(ZSTD) -1c --stream-size=0 --no-content-size </dev/null >/dev/null 2>&1 && printf '%s' '--stream-size=$(total_size) --no-content-size')
>
>  quiet_cmd_zstd = ZSTD    $@
> -      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
> +      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -19; $(size_append); } > $@
>
>  quiet_cmd_zstd22 = ZSTD22  $@
> -      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> +      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -22 --ultra; $(size_append); } > $@
>
>  # ASM offsets
>  # ---------------------------------------------------------------------------
> --
> 2.34.0
>

Re: [PATCH v2 2/2] kbuild: pass --stream-size --no-content-size to zstd

[Sedat Dilek]

On Fri, Dec 17, 2021 at 9:51 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Wed, Nov 24, 2021 at 4:30 PM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
> >
> > Otherwise, it allocates 2 GB of memory at once. Even though the majority
> > of this memory is never touched, the default heuristic overcommit
> > refuses this request if less than 2 GB of RAM+swap is currently
> > available. This results in "zstd: error 11 : Allocation error : not
> > enough memory" and the kernel failing to build.
> >
> > When the size is specified, zstd will reduce the memory request
> > appropriately. For typical kernel sizes of ~32 MB, the largest mmap
> > request will be reduced to 512 MB, which will succeed on all but the
> > smallest devices.
> >
> > For inputs around this size, --stream-size --no-content-size may
> > slightly decrease the compressed size, or slightly increase it:
> > https://github.com/facebook/zstd/issues/2848.
> >
>
> Hi Alex and Nick T.,
>
> some questions:
>
> Can I apply this patch as a single patch - without patch 1/2?
>
> Is there an impact also on the kernel's ZRAM/ZSWAP support plus using
> ZSTD as (de)comp-algo?
>
> Here I have:
>
> $ grep -i zstd /boot/config-5.15.7-1-amd64-clang13-lto | egrep -i 'zram|zswap'
> CONFIG_ZSWAP_COMPRESSOR_DEFAULT_ZSTD=y
> CONFIG_ZSWAP_COMPRESSOR_DEFAULT="zstd"
> CONFIG_ZRAM_DEF_COMP_ZSTD=y
> CONFIG_ZRAM_DEF_COMP="zstd"
>

$ egrep 'stream-size' build-log_5.15.9-1-amd64-clang13-lto.txt
49360:  { cat arch/x86/boot/compressed/vmlinux.bin
arch/x86/boot/compressed/vmlinux.relocs | zstd --stream-size=53340760
--no-content-size -22 --ultra; printf \130\352
\055\003; } > arch/x86/boot/compressed/vmlinux.bin.zst

Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

- Sedat -

> Thanks.
>
> Regards,
> - Sedat -
>
> > Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>
> > ---
> >  scripts/Makefile.lib | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index ca901814986a..c98a82ca38e6 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -466,12 +466,20 @@ quiet_cmd_xzmisc = XZMISC  $@
> >  # single pass, so zstd doesn't need to allocate a window buffer. When streaming
> >  # decompression is used, like initramfs decompression, zstd22 should likely not
> >  # be used because it would require zstd to allocate a 128 MB buffer.
> > +#
> > +# --stream-size to reduce zstd memory usage (otherwise zstd -22 --ultra
> > +# allocates, but does not use, 2 GB) and potentially improve compression.
> > +#
> > +# --no-content-size to save three bytes which we do not use (we use size_append).
> > +
> > +# zstd --stream-size is only supported since 1.4.4
> > +zstd_stream_size = $(shell $(ZSTD) -1c --stream-size=0 --no-content-size </dev/null >/dev/null 2>&1 && printf '%s' '--stream-size=$(total_size) --no-content-size')
> >
> >  quiet_cmd_zstd = ZSTD    $@
> > -      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
> > +      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -19; $(size_append); } > $@
> >
> >  quiet_cmd_zstd22 = ZSTD22  $@
> > -      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> > +      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) $(zstd_stream_size) -22 --ultra; $(size_append); } > $@
> >
> >  # ASM offsets
> >  # ---------------------------------------------------------------------------
> > --
> > 2.34.0
> >

Re: [PATCH v2 1/2] kbuild: use perl instead of shell to get file size

[Sedat Dilek]

On Wed, Nov 24, 2021 at 4:30 PM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>
> This makes it easier to get the size of multiple files. Perl is already
> a requirement for all builds to do header checks, so this is not an
> additional dependency.

$ egrep -B1 -A4 'file-size.pl' build-log_5.15.9-1-amd64-clang13-lto.txt
49219-+ clang -nostdinc -I./arch/x86/include
-I./arch/x86/include/generated -I./include -I./arch/x86/include/uapi
-I./arch/x86/include/generated/uapi -I./include/uapi
-I./include/generated/uapi -include ./include/linux/compiler-version.h
-include ./include/linux/kconfig.h -D__KERNEL__ -Qunused-arguments
-fmacro-prefix-map=./= -D__AS
SEMBLY__ -fno-PIE --target=x86_64-linux-gnu -fintegrated-as
-Werror=unknown-warning-option -Werror=ignored-optimization-argument
-m64 -DCC_USING_NOP_MCOUNT -DCC_USING_
FENTRY -fno-lto -c -o .tmp_vmlinux.kallsyms2.o .tmp_vmlinux.kallsyms2.S
49220:+ perl ./scripts/file-size.pl .tmp_vmlinux.kallsyms1.o
49221-+ size1=1932072
49222:+ perl ./scripts/file-size.pl .tmp_vmlinux.kallsyms2.o
49223-+ size2=1932072
49224-+ [ 1932072 -ne 1932072 ]
49225-+ [ -n  ]
49226-+ vmlinux_link vmlinux .tmp_vmlinux.kallsyms2.o .btf.vmlinux.bin.o

Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

- Sedat -

> ---
>  arch/arm/boot/deflate_xip_data.sh | 2 +-
>  arch/powerpc/boot/wrapper         | 2 +-
>  scripts/Makefile.lib              | 9 ++-------
>  scripts/file-size.pl              | 8 ++++++++
>  scripts/file-size.sh              | 4 ----
>  scripts/link-vmlinux.sh           | 4 ++--
>  6 files changed, 14 insertions(+), 15 deletions(-)
>  create mode 100755 scripts/file-size.pl
>  delete mode 100755 scripts/file-size.sh
>
> diff --git a/arch/arm/boot/deflate_xip_data.sh b/arch/arm/boot/deflate_xip_data.sh
> index 304495c3c2c5..14cfa2babb93 100755
> --- a/arch/arm/boot/deflate_xip_data.sh
> +++ b/arch/arm/boot/deflate_xip_data.sh
> @@ -43,7 +43,7 @@ data_start=$(($__data_loc - $base_offset))
>  data_end=$(($_edata_loc - $base_offset))
>
>  # Make sure data occupies the last part of the file.
> -file_end=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" "$XIPIMAGE")
> +file_end=$(${PERL} "${srctree}/scripts/file-size.pl" "$XIPIMAGE")
>  if [ "$file_end" != "$data_end" ]; then
>         printf "end of xipImage doesn't match with _edata_loc (%#x vs %#x)\n" \
>                $(($file_end + $base_offset)) $_edata_loc 1>&2
> diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
> index 9184eda780fd..9f9ee8613432 100755
> --- a/arch/powerpc/boot/wrapper
> +++ b/arch/powerpc/boot/wrapper
> @@ -380,7 +380,7 @@ vmz="$tmpdir/`basename \"$kernel\"`.$ext"
>
>  # Calculate the vmlinux.strip size
>  ${CROSS}objcopy $objflags "$kernel" "$vmz.$$"
> -strip_size=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" "$vmz.$$")
> +strip_size=$(${PERL} "${srctree}/scripts/file-size.pl" "$vmz.$$")
>
>  if [ -z "$cacheit" -o ! -f "$vmz$compression" -o "$vmz$compression" -ot "$kernel" ]; then
>      # recompress the image if we need to
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index d1f865b8c0cb..ca901814986a 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -379,13 +379,8 @@ dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
>
>  # Bzip2 and LZMA do not include size in file... so we have to fake that;
>  # append the size as a 32-bit littleendian number as gzip does.
> -size_append = printf $(shell                                           \
> -dec_size=0;                                                            \
> -for F in $(real-prereqs); do                                   \
> -       fsize=$$($(CONFIG_SHELL) $(srctree)/scripts/file-size.sh $$F);  \
> -       dec_size=$$(expr $$dec_size + $$fsize);                         \
> -done;                                                                  \
> -printf "%08x\n" $$dec_size |                                           \
> +total_size = $(shell $(PERL) $(srctree)/scripts/file-size.pl $(real-prereqs))
> +size_append = printf $(shell printf "%08x\n" $(total_size) |           \
>         sed 's/\(..\)/\1 /g' | {                                        \
>                 read ch0 ch1 ch2 ch3;                                   \
>                 for ch in $$ch3 $$ch2 $$ch1 $$ch0; do                   \
> diff --git a/scripts/file-size.pl b/scripts/file-size.pl
> new file mode 100755
> index 000000000000..170bb6d048fa
> --- /dev/null
> +++ b/scripts/file-size.pl
> @@ -0,0 +1,8 @@
> +#!/usr/bin/perl -w
> +# SPDX-License-Identifier: GPL-2.0
> +my $total = 0;
> +foreach (@ARGV) {
> +    @stat = stat $_ or die "$_: $!";
> +    $total += $stat[7];
> +}
> +print "$total\n";
> diff --git a/scripts/file-size.sh b/scripts/file-size.sh
> deleted file mode 100755
> index 7eb7423416b5..000000000000
> --- a/scripts/file-size.sh
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -set -- $(ls -dn "$1")
> -printf '%s\n' "$5"
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 5cdd9bc5c385..c3fa38bd18ab 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -384,8 +384,8 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
>         kallsyms_step 2
>
>         # step 3
> -       size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso_prev})
> -       size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
> +       size1=$(${PERL} "${srctree}/scripts/file-size.pl" ${kallsymso_prev})
> +       size2=$(${PERL} "${srctree}/scripts/file-size.pl" ${kallsymso})
>
>         if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
>                 kallsyms_step 3
> --
> 2.34.0
>