sh: rename suffix-y to suffix_y

PATCH In review Masahiro Yamada 3 participants 16 messages Jan 9, 2022 – Aug 11, 2023

Subsystems (8)

Arm (32-bit) x86 Kbuild MIPS +4 more subsystems

lore

Share & tools

lore

Masahiro Yamada

author

2022-01-09 18:16 UTC #0 lore raw

PATCH 1/5 sh: rename suffix-y to suffix_y

'export suffix-y' does not work reliably because hyphens are disallowed in shell variables.

A similar issue was fixed by commit 2bfbe7881ee0 ("kbuild: Do not use hyphen in exported variable name").

If I do similar in dash, ARCH=sh fails to build.

  $ mv linux linux~
  $ cd linux~
  $ dash
  $ make O=foo/bar ARCH=sh CROSS_COMPILE=sh4-linux-gnu- defconfig all
  make[1]: Entering directory '/home/masahiro/linux~/foo/bar'
    [ snip ]
  make[4]: *** No rule to make target 'arch/sh/boot/compressed/vmlinux.bin.', needed by 'arch/sh/boot/compressed/piggy.o'.  Stop.
  make[3]: *** [/home/masahiro/linux~/arch/sh/boot/Makefile:40: arch/sh/boot/compressed/vmlinux] Error 2
  make[2]: *** [/home/masahiro/linux~/arch/sh/Makefile:194: zImage] Error 2
  make[1]: *** [/home/masahiro/linux~/Makefile:350: __build_one_by_one] Error 2
  make[1]: Leaving directory '/home/masahiro/linux~/foo/bar'
  make: *** [Makefile:219: __sub-make] Error 2

The maintainer of GNU Make stated that there is no consistent way to export variables that do not meet the shell's naming criteria.
(https://savannah.gnu.org/bugs/?55719)

Consequently, you cannot use hyphens in exported variables.

Signed-off-by Masahiro Yamada <[email protected]>

---

 arch/sh/boot/Makefile            | 16 ++++++++--------
 arch/sh/boot/compressed/Makefile |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

arch/sh/boot/Makefile +8 -8

index 5c123f5b2797..1f5d2df3c7e0 100644
@@ -19,12 +19,12 @@ CONFIG_ZERO_PAGE_OFFSET	?= 0x00001000
 CONFIG_ENTRY_OFFSET	?= 0x00001000
 CONFIG_PHYSICAL_START	?= $(CONFIG_MEMORY_START)
 
-suffix-y := bin
-suffix-$(CONFIG_KERNEL_GZIP)	:= gz
-suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
-suffix-$(CONFIG_KERNEL_LZMA)	:= lzma
-suffix-$(CONFIG_KERNEL_XZ)	:= xz
-suffix-$(CONFIG_KERNEL_LZO)	:= lzo
+suffix_y := bin
+suffix_$(CONFIG_KERNEL_GZIP)	:= gz
+suffix_$(CONFIG_KERNEL_BZIP2)	:= bz2
+suffix_$(CONFIG_KERNEL_LZMA)	:= lzma
+suffix_$(CONFIG_KERNEL_XZ)	:= xz
+suffix_$(CONFIG_KERNEL_LZO)	:= lzo
 
 targets := zImage vmlinux.srec romImage uImage uImage.srec uImage.gz \
 	   uImage.bz2 uImage.lzma uImage.xz uImage.lzo uImage.bin \

@@ -106,10 +106,10 @@ OBJCOPYFLAGS_uImage.srec := -I binary -O srec
 $(obj)/uImage.srec: $(obj)/uImage FORCE
 	$(call if_changed,objcopy)
 
-$(obj)/uImage: $(obj)/uImage.$(suffix-y)
+$(obj)/uImage: $(obj)/uImage.$(suffix_y)
 	@ln -sf $(notdir $<) $@
 	@echo '  Image $@ is ready'
 
 export CONFIG_PAGE_OFFSET CONFIG_MEMORY_START CONFIG_BOOT_LINK_OFFSET \
        CONFIG_PHYSICAL_START CONFIG_ZERO_PAGE_OFFSET CONFIG_ENTRY_OFFSET \
-       KERNEL_MEMORY suffix-y
+       KERNEL_MEMORY suffix_y

arch/sh/boot/compressed/Makefile +1 -1

index cf3174df7859..c1eb9a62de55 100644
@@ -64,5 +64,5 @@ OBJCOPYFLAGS += -R .empty_zero_page
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(ld-bfd) -T
 
-$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE
+$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix_y) FORCE
 	$(call if_changed,ld)

Signature · 1 line

2.32.0

Masahiro Yamada

author

2022-01-09 18:16 UTC #1 lore raw

PATCH 2/5 kbuild: drop $(size_append) from cmd_zstd

The appended file size is only used by the decompressors, which some architectures support.

As the comment "zstd22 is used for kernel compression" says, cmd_zstd22 is used in arch/{mips,s390,x86}/boot/compressed/Makefile.

On the other hand, there is no good reason to append the file size to cmd_zstd since it is used for other purposes.

Actually cmd_zstd is only used in usr/Makefile, where the appended file size is rather harmful.

The initramfs with its file size appended is considered as corrupted data, so commit 65e00e04e5ae ("initramfs: refactor the initramfs build rules") added 'override size_append := :' to make it no-op.

As a conclusion, this $(size_append) should not exist here.

Signed-off-by Masahiro Yamada <[email protected]>

---

 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/Makefile.lib +1 -1

index d1f865b8c0cb..5366466ea0e4 100644
@@ -473,7 +473,7 @@ 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) -19 > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
       cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@

Signature · 1 line

2.32.0

Nicolas Schier

Reviewed-by on v1 2/5

2022-01-10 11:34 UTC #2 lore raw

Re: [PATCH 2/5] kbuild: drop $(size_append) from cmd_zstd

Quoted 12 lines · Masahiro Yamada

The appended file size is only used by the decompressors, which some architectures support.
As the comment "zstd22 is used for kernel compression" says, cmd_zstd22 is used in arch/{mips,s390,x86}/boot/compressed/Makefile.
On the other hand, there is no good reason to append the file size to cmd_zstd since it is used for other purposes.
Actually cmd_zstd is only used in usr/Makefile, where the appended file size is rather harmful.

Quoted 8 lines

The initramfs with its file size appended is considered as corrupted data, so commit 65e00e04e5ae ("initramfs: refactor the initramfs build rules") added 'override size_append := :' to make it no-op.
As a conclusion, this $(size_append) should not exist here.
Signed-off-by Masahiro Yamada <[email protected]>
---

Reviewed-by Nicolas Schier <[email protected]>

Quoted 20 lines

 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/Makefile.lib +1 -1

index d1f865b8c0cb..5366466ea0e4 100644
@@ -473,7 +473,7 @@ 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) -19 > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
       cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@

Signature · 1 line

2.32.0

Masahiro Yamada

author

2022-01-09 18:16 UTC #3 lore raw

PATCH 3/5 kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

In reply to Masahiro Yamada

GZIP-compressed files end with 4 byte data that represents the size of the original input. The decompressors (the self-extracting kernel) exploit it to know the vmlinux size beforehand. To mimic the GZIP's trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}. Unfortunately these macros are used everywhere despite the appended size data is only useful for the decompressors.

There is no guarantee that such hand-crafted trailers are safely ignored. In fact, the kernel refuses compressed initramdisks with the garbage data. That is why usr/Makefile overrides size_append to make it no-op.

To limit the use of such broken compressed files, this commit renames the existing macros as follows:

  cmd_bzip2   --> cmd_bzip2_with_size
  cmd_lzma    --> cmd_lzma_with_size
  cmd_lzo     --> cmd_lzo_with_size
  cmd_lz4     --> cmd_lz4_with_size
  cmd_xzkern  --> cmd_xzkern_with_size
  cmd_zstd22  --> cmd_zstd22_with_size

To keep the decompressors working, I updated the following Makefiles accordingly:

arch/arm/boot/compressed/Makefile
arch/h8300/boot/compressed/Makefile
arch/mips/boot/compressed/Makefile
arch/parisc/boot/compressed/Makefile
arch/s390/boot/compressed/Makefile
arch/sh/boot/compressed/Makefile
arch/x86/boot/compressed/Makefile

I reused the current macro names for the normal usecases; they produce the compressed data in the proper format.

I did not touch the following:

arch/arc/boot/Makefile
arch/arm64/boot/Makefile
arch/csky/boot/Makefile
arch/mips/boot/Makefile
arch/riscv/boot/Makefile
arch/sh/boot/Makefile
kernel/Makefile

This means those Makefiles will stop appending the size data.

I dropped the 'override size_append' hack from usr/Makefile.

Signed-off-by Masahiro Yamada <[email protected]>

---

 arch/arm/boot/compressed/Makefile    |  8 ++++----
 arch/h8300/boot/compressed/Makefile  |  4 +++-
 arch/mips/boot/compressed/Makefile   | 12 +++++------
 arch/parisc/boot/compressed/Makefile | 10 +++++-----
 arch/s390/boot/compressed/Makefile   | 12 +++++------
 arch/sh/boot/compressed/Makefile     |  8 ++++----
 arch/x86/boot/compressed/Makefile    | 12 +++++------
 scripts/Makefile.lib                 | 30 ++++++++++++++++++++++------
 usr/Makefile                         |  5 -----
 9 files changed, 58 insertions(+), 43 deletions(-)

arch/arm/boot/compressed/Makefile +4 -4

index 91265e7ff672..adc0e318a1ea 100644
@@ -77,10 +77,10 @@ CPPFLAGS_vmlinux.lds += -DTEXT_OFFSET="$(TEXT_OFFSET)"
 CPPFLAGS_vmlinux.lds += -DMALLOC_SIZE="$(MALLOC_SIZE)"
 
 compress-$(CONFIG_KERNEL_GZIP) = gzip
-compress-$(CONFIG_KERNEL_LZO)  = lzo
-compress-$(CONFIG_KERNEL_LZMA) = lzma
-compress-$(CONFIG_KERNEL_XZ)   = xzkern
-compress-$(CONFIG_KERNEL_LZ4)  = lz4
+compress-$(CONFIG_KERNEL_LZO)  = lzo_with_size
+compress-$(CONFIG_KERNEL_LZMA) = lzma_with_size
+compress-$(CONFIG_KERNEL_XZ)   = xzkern_with_size
+compress-$(CONFIG_KERNEL_LZ4)  = lz4_with_size
 
 libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o

arch/h8300/boot/compressed/Makefile +3 -1

index 5942793f77a0..6ab2fa5ba105 100644
@@ -30,9 +30,11 @@ $(obj)/vmlinux.bin: vmlinux FORCE
 
 suffix-$(CONFIG_KERNEL_GZIP)    := gzip
 suffix-$(CONFIG_KERNEL_LZO)     := lzo
+compress-$(CONFIG_KERNEL_GZIP)  := gzip
+compress-$(CONFIG_KERNEL_LZO)   := lzo_with_size
 
 $(obj)/vmlinux.bin.$(suffix-y): $(obj)/vmlinux.bin FORCE
-	$(call if_changed,$(suffix-y))
+	$(call if_changed,$(compress-y))
 
 LDFLAGS_piggy.o := -r --format binary --oformat elf32-h8300-linux -T
 OBJCOPYFLAGS := -O binary

arch/mips/boot/compressed/Makefile +6 -6

index f27cf31b4140..832f8001d7d9 100644
@@ -64,12 +64,12 @@ $(obj)/vmlinux.bin: $(KBUILD_IMAGE) FORCE
 	$(call if_changed,objcopy)
 
 tool_$(CONFIG_KERNEL_GZIP)    = gzip
-tool_$(CONFIG_KERNEL_BZIP2)   = bzip2
-tool_$(CONFIG_KERNEL_LZ4)     = lz4
-tool_$(CONFIG_KERNEL_LZMA)    = lzma
-tool_$(CONFIG_KERNEL_LZO)     = lzo
-tool_$(CONFIG_KERNEL_XZ)      = xzkern
-tool_$(CONFIG_KERNEL_ZSTD)    = zstd22
+tool_$(CONFIG_KERNEL_BZIP2)   = bzip2_with_size
+tool_$(CONFIG_KERNEL_LZ4)     = lz4_with_size
+tool_$(CONFIG_KERNEL_LZMA)    = lzma_with_size
+tool_$(CONFIG_KERNEL_LZO)     = lzo_with_size
+tool_$(CONFIG_KERNEL_XZ)      = xzkern_with_size
+tool_$(CONFIG_KERNEL_ZSTD)    = zstd22_with_size
 
 targets += vmlinux.bin.z

arch/parisc/boot/compressed/Makefile +5 -5

index bf4f2891d0b7..2640f72d69ce 100644
@@ -70,15 +70,15 @@ suffix-$(CONFIG_KERNEL_XZ)  := xz
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T
 $(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE

arch/s390/boot/compressed/Makefile +6 -6

index 3b860061e84d..8ea880b7c3ec 100644
@@ -71,17 +71,17 @@ suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,zstd22)
+	$(call if_changed,zstd22_with_size)
 
 OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed
 $(obj)/piggy.o: $(obj)/vmlinux.bin$(suffix-y) FORCE

arch/sh/boot/compressed/Makefile +4 -4

index c1eb9a62de55..a6808a403f4b 100644
@@ -52,13 +52,13 @@ vmlinux.bin.all-y := $(obj)/vmlinux.bin
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 
 OBJCOPYFLAGS += -R .empty_zero_page

arch/x86/boot/compressed/Makefile +6 -6

index 431bf7f846c3..2825c74bcae3 100644
@@ -123,17 +123,17 @@ vmlinux.bin.all-$(CONFIG_X86_NEED_RELOCS) += $(obj)/vmlinux.relocs
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,zstd22)
+	$(call if_changed,zstd22_with_size)
 
 suffix-$(CONFIG_KERNEL_GZIP)	:= gz
 suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2

scripts/Makefile.lib +24 -6

index 5366466ea0e4..4207a72d429f 100644
@@ -395,19 +395,31 @@ printf "%08x\n" $$dec_size |						\
 )
 
 quiet_cmd_bzip2 = BZIP2   $@
-      cmd_bzip2 = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
+      cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@
+
+quiet_cmd_bzip2_with_size = BZIP2   $@
+      cmd_bzip2_with_size = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
 
 # Lzma
 # ---------------------------------------------------------------------------
 
 quiet_cmd_lzma = LZMA    $@
-      cmd_lzma = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
+      cmd_lzma = cat $(real-prereqs) | $(LZMA) -9 > $@
+
+quiet_cmd_lzma_with_size = LZMA    $@
+      cmd_lzma_with_size = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
 
 quiet_cmd_lzo = LZO     $@
-      cmd_lzo = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
+      cmd_lzo = cat $(real-prereqs) | $(KLZOP) -9 > $@
+
+quiet_cmd_lzo_with_size = LZO     $@
+      cmd_lzo_with_size = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
 
 quiet_cmd_lz4 = LZ4     $@
-      cmd_lz4 = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
+      cmd_lz4 = cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout > $@
+
+quiet_cmd_lz4_with_size = LZ4     $@
+      cmd_lz4_with_size = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
                   $(size_append); } > $@
 
 # U-Boot mkimage

@@ -450,7 +462,10 @@ quiet_cmd_uimage = UIMAGE  $@
 # big dictionary would increase the memory usage too much in the multi-call
 # decompression mode. A BCJ filter isn't used either.
 quiet_cmd_xzkern = XZKERN  $@
-      cmd_xzkern = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
+      cmd_xzkern = cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh > $@
+
+quiet_cmd_xzkern_with_size = XZKERN  $@
+      cmd_xzkern_with_size = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
                      $(size_append); } > $@
 
 quiet_cmd_xzmisc = XZMISC  $@

@@ -476,7 +491,10 @@ quiet_cmd_zstd = ZSTD    $@
       cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
-      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
+      cmd_zstd22 = cat $(real-prereqs) | $(ZSTD) -22 --ultra > $@
+
+quiet_cmd_zstd22_with_size = ZSTD22  $@
+      cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
 
 # ASM offsets
 # ---------------------------------------------------------------------------

usr/Makefile +0 -5

index b1a81a40eab1..7b89c0175a3a 100644
@@ -3,11 +3,6 @@
 # kbuild file for usr/ - including initramfs image
 #
 
-# cmd_bzip2, cmd_lzma, cmd_lzo, cmd_lz4 from scripts/Makefile.lib appends the
-# size at the end of the compressed file, which unfortunately does not work
-# with unpack_to_rootfs(). Make size_append no-op.
-override size_append := :
-
 compress-y					:= shipped
 compress-$(CONFIG_INITRAMFS_COMPRESSION_GZIP)	:= gzip
 compress-$(CONFIG_INITRAMFS_COMPRESSION_BZIP2)	:= bzip2

Signature · 1 line

2.32.0

Nicolas Schier

Reviewed-by on v1 3/5

2022-01-10 11:34 UTC #4 lore raw

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

Quoted 45 lines · Masahiro Yamada

GZIP-compressed files end with 4 byte data that represents the size of the original input. The decompressors (the self-extracting kernel) exploit it to know the vmlinux size beforehand. To mimic the GZIP's trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}. Unfortunately these macros are used everywhere despite the appended size data is only useful for the decompressors.
There is no guarantee that such hand-crafted trailers are safely ignored. In fact, the kernel refuses compressed initramdisks with the garbage data. That is why usr/Makefile overrides size_append to make it no-op.
To limit the use of such broken compressed files, this commit renames the existing macros as follows:
  cmd_bzip2   --> cmd_bzip2_with_size
  cmd_lzma    --> cmd_lzma_with_size
  cmd_lzo     --> cmd_lzo_with_size
  cmd_lz4     --> cmd_lz4_with_size
  cmd_xzkern  --> cmd_xzkern_with_size
  cmd_zstd22  --> cmd_zstd22_with_size
To keep the decompressors working, I updated the following Makefiles accordingly:
arch/arm/boot/compressed/Makefile
arch/h8300/boot/compressed/Makefile
arch/mips/boot/compressed/Makefile
arch/parisc/boot/compressed/Makefile
arch/s390/boot/compressed/Makefile
arch/sh/boot/compressed/Makefile
arch/x86/boot/compressed/Makefile
I reused the current macro names for the normal usecases; they produce the compressed data in the proper format.
I did not touch the following:
arch/arc/boot/Makefile
arch/arm64/boot/Makefile
arch/csky/boot/Makefile
arch/mips/boot/Makefile
arch/riscv/boot/Makefile
arch/sh/boot/Makefile
kernel/Makefile

Quoted 6 lines

This means those Makefiles will stop appending the size data.
I dropped the 'override size_append' hack from usr/Makefile.
Signed-off-by Masahiro Yamada <[email protected]>
---

Reviewed-by Nicolas Schier <[email protected]>

Quoted 257 lines

 arch/arm/boot/compressed/Makefile    |  8 ++++----
 arch/h8300/boot/compressed/Makefile  |  4 +++-
 arch/mips/boot/compressed/Makefile   | 12 +++++------
 arch/parisc/boot/compressed/Makefile | 10 +++++-----
 arch/s390/boot/compressed/Makefile   | 12 +++++------
 arch/sh/boot/compressed/Makefile     |  8 ++++----
 arch/x86/boot/compressed/Makefile    | 12 +++++------
 scripts/Makefile.lib                 | 30 ++++++++++++++++++++++------
 usr/Makefile                         |  5 -----
 9 files changed, 58 insertions(+), 43 deletions(-)

arch/arm/boot/compressed/Makefile +4 -4

index 91265e7ff672..adc0e318a1ea 100644
@@ -77,10 +77,10 @@ CPPFLAGS_vmlinux.lds += -DTEXT_OFFSET="$(TEXT_OFFSET)"
 CPPFLAGS_vmlinux.lds += -DMALLOC_SIZE="$(MALLOC_SIZE)"
 
 compress-$(CONFIG_KERNEL_GZIP) = gzip
-compress-$(CONFIG_KERNEL_LZO)  = lzo
-compress-$(CONFIG_KERNEL_LZMA) = lzma
-compress-$(CONFIG_KERNEL_XZ)   = xzkern
-compress-$(CONFIG_KERNEL_LZ4)  = lz4
+compress-$(CONFIG_KERNEL_LZO)  = lzo_with_size
+compress-$(CONFIG_KERNEL_LZMA) = lzma_with_size
+compress-$(CONFIG_KERNEL_XZ)   = xzkern_with_size
+compress-$(CONFIG_KERNEL_LZ4)  = lz4_with_size
 
 libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o

arch/h8300/boot/compressed/Makefile +3 -1

index 5942793f77a0..6ab2fa5ba105 100644
@@ -30,9 +30,11 @@ $(obj)/vmlinux.bin: vmlinux FORCE
 
 suffix-$(CONFIG_KERNEL_GZIP)    := gzip
 suffix-$(CONFIG_KERNEL_LZO)     := lzo
+compress-$(CONFIG_KERNEL_GZIP)  := gzip
+compress-$(CONFIG_KERNEL_LZO)   := lzo_with_size
 
 $(obj)/vmlinux.bin.$(suffix-y): $(obj)/vmlinux.bin FORCE
-	$(call if_changed,$(suffix-y))
+	$(call if_changed,$(compress-y))
 
 LDFLAGS_piggy.o := -r --format binary --oformat elf32-h8300-linux -T
 OBJCOPYFLAGS := -O binary

arch/mips/boot/compressed/Makefile +6 -6

index f27cf31b4140..832f8001d7d9 100644
@@ -64,12 +64,12 @@ $(obj)/vmlinux.bin: $(KBUILD_IMAGE) FORCE
 	$(call if_changed,objcopy)
 
 tool_$(CONFIG_KERNEL_GZIP)    = gzip
-tool_$(CONFIG_KERNEL_BZIP2)   = bzip2
-tool_$(CONFIG_KERNEL_LZ4)     = lz4
-tool_$(CONFIG_KERNEL_LZMA)    = lzma
-tool_$(CONFIG_KERNEL_LZO)     = lzo
-tool_$(CONFIG_KERNEL_XZ)      = xzkern
-tool_$(CONFIG_KERNEL_ZSTD)    = zstd22
+tool_$(CONFIG_KERNEL_BZIP2)   = bzip2_with_size
+tool_$(CONFIG_KERNEL_LZ4)     = lz4_with_size
+tool_$(CONFIG_KERNEL_LZMA)    = lzma_with_size
+tool_$(CONFIG_KERNEL_LZO)     = lzo_with_size
+tool_$(CONFIG_KERNEL_XZ)      = xzkern_with_size
+tool_$(CONFIG_KERNEL_ZSTD)    = zstd22_with_size
 
 targets += vmlinux.bin.z

arch/parisc/boot/compressed/Makefile +5 -5

index bf4f2891d0b7..2640f72d69ce 100644
@@ -70,15 +70,15 @@ suffix-$(CONFIG_KERNEL_XZ)  := xz
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T
 $(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE

arch/s390/boot/compressed/Makefile +6 -6

index 3b860061e84d..8ea880b7c3ec 100644
@@ -71,17 +71,17 @@ suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,zstd22)
+	$(call if_changed,zstd22_with_size)
 
 OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed
 $(obj)/piggy.o: $(obj)/vmlinux.bin$(suffix-y) FORCE

arch/sh/boot/compressed/Makefile +4 -4

index c1eb9a62de55..a6808a403f4b 100644
@@ -52,13 +52,13 @@ vmlinux.bin.all-y := $(obj)/vmlinux.bin
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 
 OBJCOPYFLAGS += -R .empty_zero_page

arch/x86/boot/compressed/Makefile +6 -6

index 431bf7f846c3..2825c74bcae3 100644
@@ -123,17 +123,17 @@ vmlinux.bin.all-$(CONFIG_X86_NEED_RELOCS) += $(obj)/vmlinux.relocs
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,zstd22)
+	$(call if_changed,zstd22_with_size)
 
 suffix-$(CONFIG_KERNEL_GZIP)	:= gz
 suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2

scripts/Makefile.lib +24 -6

index 5366466ea0e4..4207a72d429f 100644
@@ -395,19 +395,31 @@ printf "%08x\n" $$dec_size |						\
 )
 
 quiet_cmd_bzip2 = BZIP2   $@
-      cmd_bzip2 = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
+      cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@
+
+quiet_cmd_bzip2_with_size = BZIP2   $@
+      cmd_bzip2_with_size = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
 
 # Lzma
 # ---------------------------------------------------------------------------
 
 quiet_cmd_lzma = LZMA    $@
-      cmd_lzma = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
+      cmd_lzma = cat $(real-prereqs) | $(LZMA) -9 > $@
+
+quiet_cmd_lzma_with_size = LZMA    $@
+      cmd_lzma_with_size = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
 
 quiet_cmd_lzo = LZO     $@
-      cmd_lzo = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
+      cmd_lzo = cat $(real-prereqs) | $(KLZOP) -9 > $@
+
+quiet_cmd_lzo_with_size = LZO     $@
+      cmd_lzo_with_size = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
 
 quiet_cmd_lz4 = LZ4     $@
-      cmd_lz4 = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
+      cmd_lz4 = cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout > $@
+
+quiet_cmd_lz4_with_size = LZ4     $@
+      cmd_lz4_with_size = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
                   $(size_append); } > $@
 
 # U-Boot mkimage

@@ -450,7 +462,10 @@ quiet_cmd_uimage = UIMAGE  $@
 # big dictionary would increase the memory usage too much in the multi-call
 # decompression mode. A BCJ filter isn't used either.
 quiet_cmd_xzkern = XZKERN  $@
-      cmd_xzkern = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
+      cmd_xzkern = cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh > $@
+
+quiet_cmd_xzkern_with_size = XZKERN  $@
+      cmd_xzkern_with_size = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
                      $(size_append); } > $@
 
 quiet_cmd_xzmisc = XZMISC  $@

@@ -476,7 +491,10 @@ quiet_cmd_zstd = ZSTD    $@
       cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
-      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
+      cmd_zstd22 = cat $(real-prereqs) | $(ZSTD) -22 --ultra > $@
+
+quiet_cmd_zstd22_with_size = ZSTD22  $@
+      cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
 
 # ASM offsets
 # ---------------------------------------------------------------------------

usr/Makefile +0 -5

index b1a81a40eab1..7b89c0175a3a 100644
@@ -3,11 +3,6 @@
 # kbuild file for usr/ - including initramfs image
 #
 
-# cmd_bzip2, cmd_lzma, cmd_lzo, cmd_lz4 from scripts/Makefile.lib appends the
-# size at the end of the compressed file, which unfortunately does not work
-# with unpack_to_rootfs(). Make size_append no-op.
-override size_append := :
-
 compress-y					:= shipped
 compress-$(CONFIG_INITRAMFS_COMPRESSION_GZIP)	:= gzip
 compress-$(CONFIG_INITRAMFS_COMPRESSION_BZIP2)	:= bzip2

Signature · 1 line

2.32.0

Eugeniu Rosca

2023-06-23 14:54 UTC #5 lore raw

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

Hello Yamada-san,
Hello Nicolas,
Cc: SzuWei Lin (committer of the patch in AOSP [1])
Cc: Kbuild

Quoted 46 lines · Nicolas Schier

Quoted 45 lines · Masahiro Yamada
GZIP-compressed files end with 4 byte data that represents the size of the original input. The decompressors (the self-extracting kernel) exploit it to know the vmlinux size beforehand. To mimic the GZIP's trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}. Unfortunately these macros are used everywhere despite the appended size data is only useful for the decompressors.
There is no guarantee that such hand-crafted trailers are safely ignored. In fact, the kernel refuses compressed initramdisks with the garbage data. That is why usr/Makefile overrides size_append to make it no-op.
To limit the use of such broken compressed files, this commit renames the existing macros as follows:
  cmd_bzip2   --> cmd_bzip2_with_size
  cmd_lzma    --> cmd_lzma_with_size
  cmd_lzo     --> cmd_lzo_with_size
  cmd_lz4     --> cmd_lz4_with_size
  cmd_xzkern  --> cmd_xzkern_with_size
  cmd_zstd22  --> cmd_zstd22_with_size
To keep the decompressors working, I updated the following Makefiles accordingly:
arch/arm/boot/compressed/Makefile
arch/h8300/boot/compressed/Makefile
arch/mips/boot/compressed/Makefile
arch/parisc/boot/compressed/Makefile
arch/s390/boot/compressed/Makefile
arch/sh/boot/compressed/Makefile
arch/x86/boot/compressed/Makefile
I reused the current macro names for the normal usecases; they produce the compressed data in the proper format.
I did not touch the following:
arch/arc/boot/Makefile
arch/arm64/boot/Makefile
arch/csky/boot/Makefile
arch/mips/boot/Makefile
arch/riscv/boot/Makefile
arch/sh/boot/Makefile
kernel/Makefile

Quoted 8 lines

Quoted 6 lines
This means those Makefiles will stop appending the size data.
I dropped the 'override size_append' hack from usr/Makefile.
Signed-off-by Masahiro Yamada <[email protected]>
---
Reviewed-by Nicolas Schier <[email protected]>

If you don't mind, I would like to report another instance of "/bin/sh: Argument list too long" while building some out-of-tree *ko in a number of downstream v5.15.78+ kernels containing [1].

For some time now, we've been living with ugly hacks to overcome it.

Fortunately, recent git bisecting efforts apparently reveal that current v5.17-rc1 commit (and its backports in downstream) look to act as the culprit (confirmed on several host machines). So, I started to have some hopes of a long-term solution and hence sharing the findings as a first step.

I am not entirely clear how to properly trace this behavior, since no amount of "make V=1/V=2" uncovers more details. Purely by accident, I looked into the top/htop output (while running the repro) and noticed several processes doing:

/bin/sh -c dec_size=0; for F in <humongous list of filenames>; do \
fsize=$(sh /abs/path/to/scripts/file-size.sh $F); \
dec_size=$(expr $dec_size + $fsize); done; printf "%08x\n" $dec_size \
| sed 's/$..$/\1 /g' | { read ch0 ch1 ch2 ch3; for ch in \
$ch3 $ch2 $ch1 $ch0; do printf '%s%03o' '\\' $((0x$ch)); done; }

As it was the case in the recent report [2], the above command seems to require/assume generous amount of space for the shell arguments.

I still haven't compared the exact traces before and after this commit, to quantify by how much the shell argument list is increased (TODO).

Another aspect is that current commit seems to introduce the regression in a multi-threaded make only. The issue is apparently masked by 'make -j1' (TBC), which adds another level of complexity.

Unfortunately, the build use-case is highly tailored to downstream and is not repeatable against vanilla out of the box.

I will continue to increase my understanding behind what's happening. In case there are already any suggestions, would appreciate those.

[1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512

    ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")

[2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/

Signature · 2 lines

Best regards,
Eugeniu Rosca

Eugeniu Rosca

2023-07-19 19:09 UTC #6 lore raw

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

Hello Yamada-san,

Quoted 94 lines · Eugeniu Rosca

Hello Yamada-san,
Hello Nicolas,
Cc: SzuWei Lin (committer of the patch in AOSP [1])
Cc: Kbuild
Quoted 46 lines · Nicolas Schier
Quoted 45 lines · Masahiro Yamada
GZIP-compressed files end with 4 byte data that represents the size of the original input. The decompressors (the self-extracting kernel) exploit it to know the vmlinux size beforehand. To mimic the GZIP's trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}. Unfortunately these macros are used everywhere despite the appended size data is only useful for the decompressors.
There is no guarantee that such hand-crafted trailers are safely ignored. In fact, the kernel refuses compressed initramdisks with the garbage data. That is why usr/Makefile overrides size_append to make it no-op.
To limit the use of such broken compressed files, this commit renames the existing macros as follows:
  cmd_bzip2   --> cmd_bzip2_with_size
  cmd_lzma    --> cmd_lzma_with_size
  cmd_lzo     --> cmd_lzo_with_size
  cmd_lz4     --> cmd_lz4_with_size
  cmd_xzkern  --> cmd_xzkern_with_size
  cmd_zstd22  --> cmd_zstd22_with_size
To keep the decompressors working, I updated the following Makefiles accordingly:
arch/arm/boot/compressed/Makefile
arch/h8300/boot/compressed/Makefile
arch/mips/boot/compressed/Makefile
arch/parisc/boot/compressed/Makefile
arch/s390/boot/compressed/Makefile
arch/sh/boot/compressed/Makefile
arch/x86/boot/compressed/Makefile
I reused the current macro names for the normal usecases; they produce the compressed data in the proper format.
I did not touch the following:
arch/arc/boot/Makefile
arch/arm64/boot/Makefile
arch/csky/boot/Makefile
arch/mips/boot/Makefile
arch/riscv/boot/Makefile
arch/sh/boot/Makefile
kernel/Makefile
Quoted 8 lines
Quoted 6 lines
This means those Makefiles will stop appending the size data.
I dropped the 'override size_append' hack from usr/Makefile.
Signed-off-by Masahiro Yamada <[email protected]>
---
Reviewed-by Nicolas Schier <[email protected]>
If you don't mind, I would like to report another instance of "/bin/sh: Argument list too long" while building some out-of-tree *ko in a number of downstream v5.15.78+ kernels containing [1].
For some time now, we've been living with ugly hacks to overcome it.
Fortunately, recent git bisecting efforts apparently reveal that current v5.17-rc1 commit (and its backports in downstream) look to act as the culprit (confirmed on several host machines). So, I started to have some hopes of a long-term solution and hence sharing the findings as a first step.
I am not entirely clear how to properly trace this behavior, since no amount of "make V=1/V=2" uncovers more details. Purely by accident, I looked into the top/htop output (while running the repro) and noticed several processes doing:
/bin/sh -c dec_size=0; for F in <humongous list of filenames>; do \
fsize=$(sh /abs/path/to/scripts/file-size.sh $F); \
dec_size=$(expr $dec_size + $fsize); done; printf "%08x\n" $dec_size \
| sed 's/$..$/\1 /g' | { read ch0 ch1 ch2 ch3; for ch in \
$ch3 $ch2 $ch1 $ch0; do printf '%s%03o' '\\' $((0x$ch)); done; }
As it was the case in the recent report [2], the above command seems to require/assume generous amount of space for the shell arguments.
I still haven't compared the exact traces before and after this commit, to quantify by how much the shell argument list is increased (TODO).
Another aspect is that current commit seems to introduce the regression in a multi-threaded make only. The issue is apparently masked by 'make -j1' (TBC), which adds another level of complexity.

Quoted 5 lines

Unfortunately, the build use-case is highly tailored to downstream and is not repeatable against vanilla out of the box.
I will continue to increase my understanding behind what's happening. In case there are already any suggestions, would appreciate those.

JFYI, we've got confirmation from Qualcomm Customer Support interface that reverting [1] heals the issue on QC end as well. However, it looks like none of us has clear understanding how to properly troubleshoot/trace/compare the behavior before and after the commit.

I would happily follow any suggestions.

Quoted 4 lines

[1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
    ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
[2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/

Signature · 2 lines

Best regards,
Eugeniu Rosca

Masahiro Yamada

author

2023-07-22 16:09 UTC #7 lore raw

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

Quoted 111 lines · Eugeniu Rosca

Hello Yamada-san,
Quoted 94 lines · Eugeniu Rosca
Hello Yamada-san,
Hello Nicolas,
Cc: SzuWei Lin (committer of the patch in AOSP [1])
Cc: Kbuild
Quoted 46 lines · Nicolas Schier
Quoted 45 lines · Masahiro Yamada
GZIP-compressed files end with 4 byte data that represents the size of the original input. The decompressors (the self-extracting kernel) exploit it to know the vmlinux size beforehand. To mimic the GZIP's trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}. Unfortunately these macros are used everywhere despite the appended size data is only useful for the decompressors.
There is no guarantee that such hand-crafted trailers are safely ignored. In fact, the kernel refuses compressed initramdisks with the garbage data. That is why usr/Makefile overrides size_append to make it no-op.
To limit the use of such broken compressed files, this commit renames the existing macros as follows:
  cmd_bzip2   --> cmd_bzip2_with_size
  cmd_lzma    --> cmd_lzma_with_size
  cmd_lzo     --> cmd_lzo_with_size
  cmd_lz4     --> cmd_lz4_with_size
  cmd_xzkern  --> cmd_xzkern_with_size
  cmd_zstd22  --> cmd_zstd22_with_size
To keep the decompressors working, I updated the following Makefiles accordingly:
arch/arm/boot/compressed/Makefile
arch/h8300/boot/compressed/Makefile
arch/mips/boot/compressed/Makefile
arch/parisc/boot/compressed/Makefile
arch/s390/boot/compressed/Makefile
arch/sh/boot/compressed/Makefile
arch/x86/boot/compressed/Makefile
I reused the current macro names for the normal usecases; they produce the compressed data in the proper format.
I did not touch the following:
arch/arc/boot/Makefile
arch/arm64/boot/Makefile
arch/csky/boot/Makefile
arch/mips/boot/Makefile
arch/riscv/boot/Makefile
arch/sh/boot/Makefile
kernel/Makefile
Quoted 8 lines
Quoted 6 lines
This means those Makefiles will stop appending the size data.
I dropped the 'override size_append' hack from usr/Makefile.
Signed-off-by Masahiro Yamada <[email protected]>
---
Reviewed-by Nicolas Schier <[email protected]>
If you don't mind, I would like to report another instance of "/bin/sh: Argument list too long" while building some out-of-tree *ko in a number of downstream v5.15.78+ kernels containing [1].
For some time now, we've been living with ugly hacks to overcome it.
Fortunately, recent git bisecting efforts apparently reveal that current v5.17-rc1 commit (and its backports in downstream) look to act as the culprit (confirmed on several host machines). So, I started to have some hopes of a long-term solution and hence sharing the findings as a first step.
I am not entirely clear how to properly trace this behavior, since no amount of "make V=1/V=2" uncovers more details. Purely by accident, I looked into the top/htop output (while running the repro) and noticed several processes doing:
/bin/sh -c dec_size=0; for F in <humongous list of filenames>; do \
fsize=$(sh /abs/path/to/scripts/file-size.sh $F); \
dec_size=$(expr $dec_size + $fsize); done; printf "%08x\n" $dec_size \
| sed 's/$..$/\1 /g' | { read ch0 ch1 ch2 ch3; for ch in \
$ch3 $ch2 $ch1 $ch0; do printf '%s%03o' '\\' $((0x$ch)); done; }
As it was the case in the recent report [2], the above command seems to require/assume generous amount of space for the shell arguments.
I still haven't compared the exact traces before and after this commit, to quantify by how much the shell argument list is increased (TODO).
Another aspect is that current commit seems to introduce the regression in a multi-threaded make only. The issue is apparently masked by 'make -j1' (TBC), which adds another level of complexity.
Quoted 5 lines
Unfortunately, the build use-case is highly tailored to downstream and is not repeatable against vanilla out of the box.
I will continue to increase my understanding behind what's happening. In case there are already any suggestions, would appreciate those.
JFYI, we've got confirmation from Qualcomm Customer Support interface that reverting [1] heals the issue on QC end as well. However, it looks like none of us has clear understanding how to properly troubleshoot/trace/compare the behavior before and after the commit.
I would happily follow any suggestions.

Quoted 8 lines

Quoted 4 lines
[1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
    ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
[2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/
--
Best regards,
Eugeniu Rosca

The only suspicious code I found in the Android common kernel is the following line in scripts/Makefile.lib

quiet_cmd_zstd = ZSTD $@

      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@

If you see the corresponding line in the mainline kernel, it looks as follows:

quiet_cmd_zstd = ZSTD $@

      cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@

7ce7e984ab2b218d6e92d5165629022fe2daf9ee depends on 64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c

But, Android common kernel back-ported only
7ce7e984ab2b218d6e92d5165629022fe2daf9ee

Please backport 64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c and see if the problem goes away.

Signature · 2 lines

Best Regards
Masahiro Yamada

Eugeniu Rosca

2023-07-25 09:24 UTC #8 lore raw

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

Hello Yamada-san,

Appreciate your willingness to support. Some findings below.

Quoted 2 lines · Masahiro Yamada

Quoted 1 line · Eugeniu Rosca
On Fri, Jun 23, 2023 at 04:45:44PM +0200, Eugeniu Rosca wrote:

[..]

Quoted 14 lines

Quoted 14 lines
Quoted 2 lines
I will continue to increase my understanding behind what's happening. In case there are already any suggestions, would appreciate those.
JFYI, we've got confirmation from Qualcomm Customer Support interface that reverting [1] heals the issue on QC end as well. However, it looks like none of us has clear understanding how to properly troubleshoot/trace/compare the behavior before and after the commit.
I would happily follow any suggestions.
Quoted 4 lines
[1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
    ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
[2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/

[..]

Quoted 2 lines

Please backport 64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c and see if the problem goes away.

Unfortunately, the problem remains after backporting the above commit.

After some more bisecting and some more trial-and-error, I finally came up with a reproduction scenario against vanilla. It also shows that after reverting 7ce7e984ab2b21 ("kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}"), the problem goes away.

It takes <30 seconds to reproduce the issue on my machine (on 2nd run).

In order to make the test self-sufficient, it also clones the Linux sources (only during 1st run, with --depth 1, for minimal footprint), hence ~1.8 GB free space is required in /tmp .

The repro.sh script:
=> https://gist.github.com/erosca/1372fdc24126dc98031444613450c494

Output against vanilla on 1st run (always OK, matches real-life case):
=> https://gist.github.com/erosca/0f5b8e0a00a256d80f0c8a6364d81568

Output against vanilla on 2nd/Nth run (NOK: Argument list too long):
=> https://gist.github.com/erosca/e5c2c6479cc32244cc38d308deea4cf5

Output against vanilla + revert_of_7ce7e984ab2b2 on Nth run (always OK):
=> https://gist.github.com/erosca/57e114f92ea20132e19fc7f5a46e7c65

Would it be possible to get your thoughts on the above?

Signature · 2 lines

Best regards,
Eugeniu Rosca

Masahiro Yamada

author

2023-08-02 09:21 UTC #9 lore raw

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

Quoted 53 lines · Eugeniu Rosca

Hello Yamada-san,
Appreciate your willingness to support. Some findings below.
Quoted 2 lines · Masahiro Yamada
Quoted 1 line · Eugeniu Rosca
On Fri, Jun 23, 2023 at 04:45:44PM +0200, Eugeniu Rosca wrote:
[..]
Quoted 14 lines
Quoted 14 lines
Quoted 2 lines
I will continue to increase my understanding behind what's happening. In case there are already any suggestions, would appreciate those.
JFYI, we've got confirmation from Qualcomm Customer Support interface that reverting [1] heals the issue on QC end as well. However, it looks like none of us has clear understanding how to properly troubleshoot/trace/compare the behavior before and after the commit.
I would happily follow any suggestions.
Quoted 4 lines
[1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
    ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
[2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/
[..]
Quoted 2 lines
Please backport 64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c and see if the problem goes away.
Unfortunately, the problem remains after backporting the above commit.
After some more bisecting and some more trial-and-error, I finally came up with a reproduction scenario against vanilla. It also shows that after reverting 7ce7e984ab2b21 ("kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}"), the problem goes away.
It takes <30 seconds to reproduce the issue on my machine (on 2nd run).
In order to make the test self-sufficient, it also clones the Linux sources (only during 1st run, with --depth 1, for minimal footprint), hence ~1.8 GB free space is required in /tmp .
The repro.sh script:
=> https://gist.github.com/erosca/1372fdc24126dc98031444613450c494
Output against vanilla on 1st run (always OK, matches real-life case):
=> https://gist.github.com/erosca/0f5b8e0a00a256d80f0c8a6364d81568
Output against vanilla on 2nd/Nth run (NOK: Argument list too long):
=> https://gist.github.com/erosca/e5c2c6479cc32244cc38d308deea4cf5

Quoted 8 lines

Output against vanilla + revert_of_7ce7e984ab2b2 on Nth run (always OK):
=> https://gist.github.com/erosca/57e114f92ea20132e19fc7f5a46e7c65
Would it be possible to get your thoughts on the above?
--
Best regards,
Eugeniu Rosca

Indeed, reverting 7ce7e984ab2b218d6e92d5165629022fe2daf9ee makes qcom's external module build successfully (but rebuilding is super slow).

Interestingly, revert 7ce7e984ab2b218d6e92d5165629022fe2daf9ee then apply the attached patch, then
'Argument list too long' will come back.

So, this is unrelated to the actual build commands.

I suspect bare 'export', which expands all variables while apparently most of them are not meant exported.

Insert the following in your reproducer, then it will work.

# qcom's audio-kernel sprinkles 'export' everywhere. # Remove bare use of 'export'
find "$ABS_KMOD" -name Kbuild | xargs sed -i '/export$/d'

--
Best Regards
Masahiro Yamada

From a1d641238851dbde00a37b7b203b110e786edd1d Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <[email protected]>
Date: Wed, 2 Aug 2023 18:11:32 +0900
Subject: [PATCH] Add dummy commands to make qcom's external module fail

Revert 7ce7e984ab2b218d6e92d5165629022fe2daf9ee and apply this instead.

I see

/bin/sh: Argument list too long

Signed-off-by Masahiro Yamada <[email protected]>

---
 scripts/Makefile.lib | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

scripts/Makefile.lib +33 -0

index 2270ed819a29..c481018d1b5f 100644
@@ -559,3 +559,36 @@ define filechk_offsets
 	 echo ""; \
 	 echo "#endif"
 endef
+
+
+# These dummy commands are not used anywhere, but
+# make qcom's external module fail to build.
+# qcom's Makefile use bare 'export'.
+# Having more and more shell commands may flood variable expansions.
+
+quiet_cmd_dummy1 = FOO    $@
+      cmd_dummy1 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy2 = FOO    $@
+      cmd_dummy2 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy3 = FOO    $@
+      cmd_dummy3 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy4 = FOO    $@
+      cmd_dummy4 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy5 = FOO    $@
+      cmd_dummy5 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy6 = FOO    $@
+      cmd_dummy6 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy7 = FOO    $@
+      cmd_dummy7 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy8 = FOO    $@
+      cmd_dummy8 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy9 = FOO    $@
+      cmd_dummy9 = cat $(real-prereqs) > $@

Signature · 1 line

2.39.2

Eugeniu Rosca

2023-08-11 11:25 UTC #10 lore raw

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

Hello Yamada-san,

Appreciate your generous support, which allowed finding the root-cause.

On Wed, Aug 02, 2023 at 06:21:14PM +0900, Masahiro Yamada wrote:

[..]

Quoted 3 lines

Indeed, reverting 7ce7e984ab2b218d6e92d5165629022fe2daf9ee makes qcom's external module build successfully (but rebuilding is super slow).

Same observation.

Quoted 9 lines

Interestingly, revert 7ce7e984ab2b218d6e92d5165629022fe2daf9ee then apply the attached patch, then
'Argument list too long' will come back.
So, this is unrelated to the actual build commands.
I suspect bare 'export', which expands all variables while apparently most of them are not meant exported.

Indeed, that seems to be the case and more evidence supports this:

* Several pre-existing commits point out to the same root-cause:

https://git.codelinaro.org/clo/la/platform/vendor/qcom/opensource/audio-kernel-ar/-/commit/4025a25a2479fc34
("makefile: kona: remove make export <all variables> instances")

https://github.com/sonyxperiadev/kernel-techpack-audio/commit/02f00754120df2
("audio-kernel: Fix build time issue")

* The more recent GNU Make 4.4.1 (called with -ddd) reveals more details just before hitting 'Argument list too long':

scripts/Makefile.lib:431: not recursively expanding size_append to export to shell function scripts/Makefile.lib:447: not recursively expanding cmd_file_size to export to shell function scripts/Makefile.lib:453: not recursively expanding cmd_bzip2_with_size to export to shell function scripts/Makefile.lib:462: not recursively expanding cmd_lzma_with_size to export to shell function scripts/Makefile.lib:468: not recursively expanding cmd_lzo_with_size to export to shell function scripts/Makefile.lib:474: not recursively expanding cmd_lz4_with_size to export to shell function scripts/Makefile.lib:520: not recursively expanding cmd_xzkern_with_size to export to shell function scripts/Makefile.lib:549: not recursively expanding cmd_zstd22_with_size to export to shell function

The "not recursively expanding" messages above come from GNU make commit https://gnu.googlesource.com/make/+/7d484017077089a
("[SV 63016] Don't fail exporting to $(shell ...)"), which appears to be saying that our issue is caused by exporting a makefile variable which contains the 'shell' directive in its definition.

I think it would be too much of a burden for Kbuild not to make use of any variables using the 'shell' keyword, so I tend to agree that defending against bare 'export' statements in Kbuild is rather not feasible and it should be fixed in the makefiles of out-of-tree *ko.

Quoted 1 line

Insert the following in your reproducer, then it will work.

Thanks for the demo code snippet.

Signature · 2 lines

Best regards,
Eugeniu Rosca

Masahiro Yamada

author

2022-01-09 18:16 UTC #11 lore raw

PATCH 4/5 arch: decompressor: remove useless vmlinux.bin.all-y

In reply to Masahiro Yamada

Presumably, arch/{parisc,s390,sh}/boot/compressed/Makefile copied arch/x86/boot/compressed/Makefile, but vmlinux.bin.all-y is useless here because it is the same as $(obj)/vmlinux.bin.

Signed-off-by Masahiro Yamada <[email protected]>

---

 arch/parisc/boot/compressed/Makefile | 14 ++++++--------
 arch/s390/boot/compressed/Makefile   | 16 +++++++---------
 arch/sh/boot/compressed/Makefile     | 12 +++++-------
 3 files changed, 18 insertions(+), 24 deletions(-)

arch/parisc/boot/compressed/Makefile +6 -8

index 2640f72d69ce..877a7099b5e1 100644
@@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -R .comment -R .note -S
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
 suffix-$(CONFIG_KERNEL_GZIP)  := gz
 suffix-$(CONFIG_KERNEL_BZIP2) := bz2
 suffix-$(CONFIG_KERNEL_LZ4)  := lz4

@@ -67,17 +65,17 @@ suffix-$(CONFIG_KERNEL_LZMA)  := lzma
 suffix-$(CONFIG_KERNEL_LZO)  := lzo
 suffix-$(CONFIG_KERNEL_XZ)  := xz
 
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lz4_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T

arch/s390/boot/compressed/Makefile +7 -9

index 8ea880b7c3ec..d04e0e7de0b3 100644
@@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -O binary --remove-section=.comment --remove-section
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
 suffix-$(CONFIG_KERNEL_GZIP)  := .gz
 suffix-$(CONFIG_KERNEL_BZIP2) := .bz2
 suffix-$(CONFIG_KERNEL_LZ4)  := .lz4

@@ -68,19 +66,19 @@ suffix-$(CONFIG_KERNEL_LZO)  := .lzo
 suffix-$(CONFIG_KERNEL_XZ)  := .xz
 suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
 
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lz4_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
-$(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.zst: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,zstd22_with_size)
 
 OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed

arch/sh/boot/compressed/Makefile +5 -7

index a6808a403f4b..591125c42d49 100644
@@ -47,17 +47,15 @@ $(obj)/vmlinux: $(addprefix $(obj)/, $(OBJECTS)) FORCE
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
 
 OBJCOPYFLAGS += -R .empty_zero_page

Signature · 1 line

2.32.0

Nicolas Schier

Reviewed-by on v1 4/5

2022-01-10 11:34 UTC #12 lore raw

Re: [PATCH 4/5] arch: decompressor: remove useless vmlinux.bin.all-y

Quoted 6 lines · Masahiro Yamada

Presumably, arch/{parisc,s390,sh}/boot/compressed/Makefile copied arch/x86/boot/compressed/Makefile, but vmlinux.bin.all-y is useless here because it is the same as $(obj)/vmlinux.bin.
Signed-off-by Masahiro Yamada <[email protected]>
---

Reviewed-by Nicolas Schier <[email protected]>

Quoted 113 lines

 arch/parisc/boot/compressed/Makefile | 14 ++++++--------
 arch/s390/boot/compressed/Makefile   | 16 +++++++---------
 arch/sh/boot/compressed/Makefile     | 12 +++++-------
 3 files changed, 18 insertions(+), 24 deletions(-)

arch/parisc/boot/compressed/Makefile +6 -8

index 2640f72d69ce..877a7099b5e1 100644
@@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -R .comment -R .note -S
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
 suffix-$(CONFIG_KERNEL_GZIP)  := gz
 suffix-$(CONFIG_KERNEL_BZIP2) := bz2
 suffix-$(CONFIG_KERNEL_LZ4)  := lz4

@@ -67,17 +65,17 @@ suffix-$(CONFIG_KERNEL_LZMA)  := lzma
 suffix-$(CONFIG_KERNEL_LZO)  := lzo
 suffix-$(CONFIG_KERNEL_XZ)  := xz
 
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lz4_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T

arch/s390/boot/compressed/Makefile +7 -9

index 8ea880b7c3ec..d04e0e7de0b3 100644
@@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -O binary --remove-section=.comment --remove-section
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
 suffix-$(CONFIG_KERNEL_GZIP)  := .gz
 suffix-$(CONFIG_KERNEL_BZIP2) := .bz2
 suffix-$(CONFIG_KERNEL_LZ4)  := .lz4

@@ -68,19 +66,19 @@ suffix-$(CONFIG_KERNEL_LZO)  := .lzo
 suffix-$(CONFIG_KERNEL_XZ)  := .xz
 suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
 
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lz4_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
-$(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.zst: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,zstd22_with_size)
 
 OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed

arch/sh/boot/compressed/Makefile +5 -7

index a6808a403f4b..591125c42d49 100644
@@ -47,17 +47,15 @@ $(obj)/vmlinux: $(addprefix $(obj)/, $(OBJECTS)) FORCE
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
 
 OBJCOPYFLAGS += -R .empty_zero_page

Signature · 1 line

2.32.0

Masahiro Yamada

author

2022-01-09 18:16 UTC #13 lore raw

PATCH 5/5 kbuild: add cmd_file_size

In reply to Masahiro Yamada

Some architectures support self-extracting kernel, which embeds the compressed vmlinux.

It has 4 byte data at the end so the decompressor can know the vmlinux size beforehand.

GZIP natively has it in the trailer, but for the other compression algorithms, the hand-crafted trailer is added.

It is unneeded to generate such _corrupted_ compressed files because it is possible to pass the size data separately.

For example, the assembly code:

.incbin "compressed-vmlinux-with-size-data-appended"

can be transformed to:

.incbin "compressed-vmlinux"
.incbin "size-data"

My hope is, after some reworks of the decompressors, the macros cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22} will go away.

This new macro, cmd_file_size, will be useful to generate a separate size-data file.

Signed-off-by Masahiro Yamada <[email protected]>

---

 scripts/Makefile.lib | 3 +++
 1 file changed, 3 insertions(+)

scripts/Makefile.lib +3 -0

index 4207a72d429f..05ca77706f6b 100644
@@ -394,6 +394,9 @@ printf "%08x\n" $$dec_size |						\
 	}								\
 )
 
+quiet_cmd_file_size = GEN     $@
+      cmd_file_size = $(size_append) > $@
+
 quiet_cmd_bzip2 = BZIP2   $@
       cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@

Signature · 1 line

2.32.0

Nicolas Schier

Reviewed-by on v1 5/5

2022-01-10 11:35 UTC #14 lore raw

Re: [PATCH 5/5] kbuild: add cmd_file_size

Quoted 21 lines · Masahiro Yamada

Some architectures support self-extracting kernel, which embeds the compressed vmlinux.
It has 4 byte data at the end so the decompressor can know the vmlinux size beforehand.
GZIP natively has it in the trailer, but for the other compression algorithms, the hand-crafted trailer is added.
It is unneeded to generate such _corrupted_ compressed files because it is possible to pass the size data separately.
For example, the assembly code:
.incbin "compressed-vmlinux-with-size-data-appended"
can be transformed to:
.incbin "compressed-vmlinux"
.incbin "size-data"

Quoted 8 lines

My hope is, after some reworks of the decompressors, the macros cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22} will go away.
This new macro, cmd_file_size, will be useful to generate a separate size-data file.
Signed-off-by Masahiro Yamada <[email protected]>
---

Reviewed-by Nicolas Schier <[email protected]>

Quoted 21 lines

 scripts/Makefile.lib | 3 +++
 1 file changed, 3 insertions(+)

scripts/Makefile.lib +3 -0

index 4207a72d429f..05ca77706f6b 100644
@@ -394,6 +394,9 @@ printf "%08x\n" $$dec_size |						\
 	}								\
 )
 
+quiet_cmd_file_size = GEN     $@
+      cmd_file_size = $(size_append) > $@
+
 quiet_cmd_bzip2 = BZIP2   $@
       cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@

Signature · 1 line

2.32.0

Nicolas Schier

Reviewed-by on v1 1/5

2022-01-10 11:32 UTC #15 lore raw

In reply to Masahiro Yamada

Quoted 21 lines · Masahiro Yamada

'export suffix-y' does not work reliably because hyphens are disallowed in shell variables.

A similar issue was fixed by commit 2bfbe7881ee0 ("kbuild: Do not use hyphen in exported variable name").

If I do similar in dash, ARCH=sh fails to build.

  $ mv linux linux~
  $ cd linux~
  $ dash
  $ make O=foo/bar ARCH=sh CROSS_COMPILE=sh4-linux-gnu- defconfig all
  make[1]: Entering directory '/home/masahiro/linux~/foo/bar'
    [ snip ]
  make[4]: *** No rule to make target 'arch/sh/boot/compressed/vmlinux.bin.', needed by 'arch/sh/boot/compressed/piggy.o'.  Stop.
  make[3]: *** [/home/masahiro/linux~/arch/sh/boot/Makefile:40: arch/sh/boot/compressed/vmlinux] Error 2
  make[2]: *** [/home/masahiro/linux~/arch/sh/Makefile:194: zImage] Error 2
  make[1]: *** [/home/masahiro/linux~/Makefile:350: __build_one_by_one] Error 2
  make[1]: Leaving directory '/home/masahiro/linux~/foo/bar'
  make: *** [Makefile:219: __sub-make] Error 2

Quoted 8 lines

The maintainer of GNU Make stated that there is no consistent way to export variables that do not meet the shell's naming criteria.
(https://savannah.gnu.org/bugs/?55719)
Consequently, you cannot use hyphens in exported variables.
Signed-off-by Masahiro Yamada <[email protected]>
---

Reviewed-by Nicolas Schier <[email protected]>

Quoted 55 lines

 arch/sh/boot/Makefile            | 16 ++++++++--------
 arch/sh/boot/compressed/Makefile |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

arch/sh/boot/Makefile +8 -8

index 5c123f5b2797..1f5d2df3c7e0 100644
@@ -19,12 +19,12 @@ CONFIG_ZERO_PAGE_OFFSET	?= 0x00001000
 CONFIG_ENTRY_OFFSET	?= 0x00001000
 CONFIG_PHYSICAL_START	?= $(CONFIG_MEMORY_START)
 
-suffix-y := bin
-suffix-$(CONFIG_KERNEL_GZIP)	:= gz
-suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
-suffix-$(CONFIG_KERNEL_LZMA)	:= lzma
-suffix-$(CONFIG_KERNEL_XZ)	:= xz
-suffix-$(CONFIG_KERNEL_LZO)	:= lzo
+suffix_y := bin
+suffix_$(CONFIG_KERNEL_GZIP)	:= gz
+suffix_$(CONFIG_KERNEL_BZIP2)	:= bz2
+suffix_$(CONFIG_KERNEL_LZMA)	:= lzma
+suffix_$(CONFIG_KERNEL_XZ)	:= xz
+suffix_$(CONFIG_KERNEL_LZO)	:= lzo
 
 targets := zImage vmlinux.srec romImage uImage uImage.srec uImage.gz \
 	   uImage.bz2 uImage.lzma uImage.xz uImage.lzo uImage.bin \

@@ -106,10 +106,10 @@ OBJCOPYFLAGS_uImage.srec := -I binary -O srec
 $(obj)/uImage.srec: $(obj)/uImage FORCE
 	$(call if_changed,objcopy)
 
-$(obj)/uImage: $(obj)/uImage.$(suffix-y)
+$(obj)/uImage: $(obj)/uImage.$(suffix_y)
 	@ln -sf $(notdir $<) $@
 	@echo '  Image $@ is ready'
 
 export CONFIG_PAGE_OFFSET CONFIG_MEMORY_START CONFIG_BOOT_LINK_OFFSET \
        CONFIG_PHYSICAL_START CONFIG_ZERO_PAGE_OFFSET CONFIG_ENTRY_OFFSET \
-       KERNEL_MEMORY suffix-y
+       KERNEL_MEMORY suffix_y

arch/sh/boot/compressed/Makefile +1 -1

index cf3174df7859..c1eb9a62de55 100644
@@ -64,5 +64,5 @@ OBJCOPYFLAGS += -R .empty_zero_page
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(ld-bfd) -T
 
-$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE
+$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix_y) FORCE
 	$(call if_changed,ld)

Signature · 1 line

2.32.0