linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] kbuild: use perl instead of shell to get file size
       [not found] <20211120165349.99908-1-alex_y_xu.ref@yahoo.ca>
@ 2021-11-20 16:53 ` Alex Xu (Hello71)
  2021-11-20 16:53   ` [PATCH 2/2] kbuild: pass --stream-size --no-content-size to zstd Alex Xu (Hello71)
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Xu (Hello71) @ 2021-11-20 16:53 UTC (permalink / raw)
  To: Michael Forney, Masahiro Yamada, Michal Marek, Nick Desaulniers,
	Nick Terrell, Ingo Molnar, Sedat Dilek, Kees Cook, linux-kernel,
	linux-kbuild
  Cc: 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH 2/2] kbuild: pass --stream-size --no-content-size to zstd
  2021-11-20 16:53 ` [PATCH 1/2] kbuild: use perl instead of shell to get file size Alex Xu (Hello71)
@ 2021-11-20 16:53   ` Alex Xu (Hello71)
  2021-11-22 19:20     ` Nick Terrell
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Xu (Hello71) @ 2021-11-20 16:53 UTC (permalink / raw)
  To: Michael Forney, Masahiro Yamada, Michal Marek, Nick Desaulniers,
	Nick Terrell, Ingo Molnar, Sedat Dilek, Kees Cook, linux-kernel,
	linux-kbuild
  Cc: 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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index ca901814986a..13d756fbcdc7 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -468,10 +468,10 @@ quiet_cmd_xzmisc = XZMISC  $@
 # be used because it would require zstd to allocate a 128 MB buffer.
 
 quiet_cmd_zstd = ZSTD    $@
-      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
+      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) --stream-size=$(total_size) --no-content-size -19; $(size_append); } > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
-      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
+      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) --stream-size=$(total_size) --no-content-size -22 --ultra; $(size_append); } > $@
 
 # ASM offsets
 # ---------------------------------------------------------------------------
-- 
2.34.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] kbuild: pass --stream-size --no-content-size to zstd
  2021-11-20 16:53   ` [PATCH 2/2] kbuild: pass --stream-size --no-content-size to zstd Alex Xu (Hello71)
@ 2021-11-22 19:20     ` Nick Terrell
  0 siblings, 0 replies; 3+ messages in thread
From: Nick Terrell @ 2021-11-22 19:20 UTC (permalink / raw)
  To: Alex Xu (Hello71)
  Cc: Michael Forney, Masahiro Yamada, Michal Marek, Nick Desaulniers,
	Ingo Molnar, Sedat Dilek, Kees Cook, linux-kernel, linux-kbuild



> On Nov 20, 2021, at 8:53 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.

I like the change in general. However, I have a question of portability.
The --stream-size option was added in zstd-1.4.4 released November
2019 [0]. That means the build will fail for people with older zstd
versions. As a reference, Ubuntu Bionic (18.04) includes zstd-1.3.3,
and Ubuntu Focal (20.04) has zstd-1.4.4. So Bionic users will have to
install zstd manually to build the kernel.

I’d prefer to err on the side of caution for portability. I’d say once
Bionic hits end of life, it’d be safe to require zstd-1.4.4 or newer.
However, I see the need to reduce memory usage, as this issue has
come up before.

Could we detect the zstd version with `zstd --version` and pass
--stream-size if we detect the version is >= v1.4.4? That add some
complexity, but I think it would be worth it. Just be sure to document
why we are detecting support for --stream-size, and why it is important
to pass to zstd.

Best,
Nick Terrell

[0] https://github.com/facebook/zstd/releases/tag/v1.4.4

> Signed-off-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>
> ---
> scripts/Makefile.lib | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index ca901814986a..13d756fbcdc7 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -468,10 +468,10 @@ quiet_cmd_xzmisc = XZMISC  $@
> # be used because it would require zstd to allocate a 128 MB buffer.
> 
> quiet_cmd_zstd = ZSTD    $@
> -      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
> +      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) --stream-size=$(total_size) --no-content-size -19; $(size_append); } > $@
> 
> quiet_cmd_zstd22 = ZSTD22  $@
> -      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> +      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) --stream-size=$(total_size) --no-content-size -22 --ultra; $(size_append); } > $@
> 
> # ASM offsets
> # ---------------------------------------------------------------------------
> -- 
> 2.34.0
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-22 19:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20211120165349.99908-1-alex_y_xu.ref@yahoo.ca>
2021-11-20 16:53 ` [PATCH 1/2] kbuild: use perl instead of shell to get file size Alex Xu (Hello71)
2021-11-20 16:53   ` [PATCH 2/2] kbuild: pass --stream-size --no-content-size to zstd Alex Xu (Hello71)
2021-11-22 19:20     ` Nick Terrell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).