From: Masahiro Yamada <masahiroy@kernel.org> To: linux-kbuild@vger.kernel.org Cc: Masahiro Yamada <masahiroy@kernel.org>, Albert Ou <aou@eecs.berkeley.edu>, Nathan Chancellor <nathan@kernel.org>, Nicolas Schier <nicolas@fjasle.eu>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: [PATCH] kbuild: buildtar: add comments about inconsistent package generation Date: Mon, 15 Apr 2024 02:41:39 +0900 [thread overview] Message-ID: <20240414174139.3001175-1-masahiroy@kernel.org> (raw) scripts/package/buildtar checks some kernel packages, and copies the first image found. This may potentially produce an inconsistent (and possibly wrong) package. For instance, the for-loop for arm64 checks Image.{bz2,gz,lz4,lzma,lzo}, and vmlinuz.efi, then copies the first image found, which might be a stale image. When CONFIG_EFI_ZBOOT is enabled in the pristine source tree, 'make ARCH=arm64 tar-pkg' will build and copy vmlinuz.efi. This is the expected behavior. If you build the kernel with CONFIG_EFI_ZBOOT, Image.gz will be created, which will remain in the build directory unless you clean it. Even if CONFIG_EFI_ZBOOT is turned on later, 'make ARCH=arm64 tar-pkg' will copy stale Image.gz instead of the latest vmlinuz.efi, as Image.gz takes precedence over vmlinuz.efi. In summary, the code "[ -f ... ] && cp" does not consistently produce the desired outcome. The other package scripts are deterministic; scripts/package/mkdebian, for example, chooses a copied kernel image based on CONFIG options. I removed [ -f ... ] checks from x86, alpha, parisc, and the default because they have a single kernel image to copy. If it is missing, it should be an error. I did not modify the code for mips, arm64, riscv. Instead, I left some comments. Eventually, someone may fix the code, or at the very least, it may discourage the copy-pasting of incorrect code. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/package/buildtar | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/scripts/package/buildtar b/scripts/package/buildtar index 72c91a1b832f..ed8d9b496305 100755 --- a/scripts/package/buildtar +++ b/scripts/package/buildtar @@ -53,18 +53,24 @@ cp -v -- "${objtree}/vmlinux" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" # # Install arch-specific kernel image(s) # +# Note: +# mips, arm64, and riscv copy the first image found. This may not produce +# the desired outcome because it may pick up a stale file remaining in the +# build tree. +# case "${ARCH}" in x86|i386|x86_64) - [ -f "${objtree}/arch/x86/boot/bzImage" ] && cp -v -- "${objtree}/arch/x86/boot/bzImage" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" + cp -v -- "${objtree}/arch/x86/boot/bzImage" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" ;; alpha) - [ -f "${objtree}/arch/alpha/boot/vmlinux.gz" ] && cp -v -- "${objtree}/arch/alpha/boot/vmlinux.gz" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" + cp -v -- "${objtree}/arch/alpha/boot/vmlinux.gz" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" ;; parisc*) - [ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" + cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" [ -f "${objtree}/lifimage" ] && cp -v -- "${objtree}/lifimage" "${tmpdir}/boot/lifimage-${KERNELRELEASE}" ;; mips) + # Please note the following code may copy a stale file. if [ -f "${objtree}/arch/mips/boot/compressed/vmlinux.bin" ]; then cp -v -- "${objtree}/arch/mips/boot/compressed/vmlinux.bin" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" elif [ -f "${objtree}/arch/mips/boot/compressed/vmlinux.ecoff" ]; then @@ -86,6 +92,7 @@ case "${ARCH}" in fi ;; arm64) + # Please note the following code may copy a stale file. for i in Image.bz2 Image.gz Image.lz4 Image.lzma Image.lzo vmlinuz.efi ; do if [ -f "${objtree}/arch/arm64/boot/${i}" ] ; then cp -v -- "${objtree}/arch/arm64/boot/${i}" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" @@ -94,6 +101,7 @@ case "${ARCH}" in done ;; riscv) + # Please note the following code may copy a stale file. for i in Image.bz2 Image.gz Image; do if [ -f "${objtree}/arch/riscv/boot/${i}" ] ; then cp -v -- "${objtree}/arch/riscv/boot/${i}" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" @@ -102,7 +110,7 @@ case "${ARCH}" in done ;; *) - [ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}" + cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}" echo "" >&2 echo '** ** ** WARNING ** ** **' >&2 echo "" >&2 -- 2.40.1
WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <masahiroy@kernel.org> To: linux-kbuild@vger.kernel.org Cc: Masahiro Yamada <masahiroy@kernel.org>, Albert Ou <aou@eecs.berkeley.edu>, Nathan Chancellor <nathan@kernel.org>, Nicolas Schier <nicolas@fjasle.eu>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: [PATCH] kbuild: buildtar: add comments about inconsistent package generation Date: Mon, 15 Apr 2024 02:41:39 +0900 [thread overview] Message-ID: <20240414174139.3001175-1-masahiroy@kernel.org> (raw) scripts/package/buildtar checks some kernel packages, and copies the first image found. This may potentially produce an inconsistent (and possibly wrong) package. For instance, the for-loop for arm64 checks Image.{bz2,gz,lz4,lzma,lzo}, and vmlinuz.efi, then copies the first image found, which might be a stale image. When CONFIG_EFI_ZBOOT is enabled in the pristine source tree, 'make ARCH=arm64 tar-pkg' will build and copy vmlinuz.efi. This is the expected behavior. If you build the kernel with CONFIG_EFI_ZBOOT, Image.gz will be created, which will remain in the build directory unless you clean it. Even if CONFIG_EFI_ZBOOT is turned on later, 'make ARCH=arm64 tar-pkg' will copy stale Image.gz instead of the latest vmlinuz.efi, as Image.gz takes precedence over vmlinuz.efi. In summary, the code "[ -f ... ] && cp" does not consistently produce the desired outcome. The other package scripts are deterministic; scripts/package/mkdebian, for example, chooses a copied kernel image based on CONFIG options. I removed [ -f ... ] checks from x86, alpha, parisc, and the default because they have a single kernel image to copy. If it is missing, it should be an error. I did not modify the code for mips, arm64, riscv. Instead, I left some comments. Eventually, someone may fix the code, or at the very least, it may discourage the copy-pasting of incorrect code. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/package/buildtar | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/scripts/package/buildtar b/scripts/package/buildtar index 72c91a1b832f..ed8d9b496305 100755 --- a/scripts/package/buildtar +++ b/scripts/package/buildtar @@ -53,18 +53,24 @@ cp -v -- "${objtree}/vmlinux" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" # # Install arch-specific kernel image(s) # +# Note: +# mips, arm64, and riscv copy the first image found. This may not produce +# the desired outcome because it may pick up a stale file remaining in the +# build tree. +# case "${ARCH}" in x86|i386|x86_64) - [ -f "${objtree}/arch/x86/boot/bzImage" ] && cp -v -- "${objtree}/arch/x86/boot/bzImage" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" + cp -v -- "${objtree}/arch/x86/boot/bzImage" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" ;; alpha) - [ -f "${objtree}/arch/alpha/boot/vmlinux.gz" ] && cp -v -- "${objtree}/arch/alpha/boot/vmlinux.gz" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" + cp -v -- "${objtree}/arch/alpha/boot/vmlinux.gz" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" ;; parisc*) - [ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" + cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" [ -f "${objtree}/lifimage" ] && cp -v -- "${objtree}/lifimage" "${tmpdir}/boot/lifimage-${KERNELRELEASE}" ;; mips) + # Please note the following code may copy a stale file. if [ -f "${objtree}/arch/mips/boot/compressed/vmlinux.bin" ]; then cp -v -- "${objtree}/arch/mips/boot/compressed/vmlinux.bin" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" elif [ -f "${objtree}/arch/mips/boot/compressed/vmlinux.ecoff" ]; then @@ -86,6 +92,7 @@ case "${ARCH}" in fi ;; arm64) + # Please note the following code may copy a stale file. for i in Image.bz2 Image.gz Image.lz4 Image.lzma Image.lzo vmlinuz.efi ; do if [ -f "${objtree}/arch/arm64/boot/${i}" ] ; then cp -v -- "${objtree}/arch/arm64/boot/${i}" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}" @@ -94,6 +101,7 @@ case "${ARCH}" in done ;; riscv) + # Please note the following code may copy a stale file. for i in Image.bz2 Image.gz Image; do if [ -f "${objtree}/arch/riscv/boot/${i}" ] ; then cp -v -- "${objtree}/arch/riscv/boot/${i}" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}" @@ -102,7 +110,7 @@ case "${ARCH}" in done ;; *) - [ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}" + cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}" echo "" >&2 echo '** ** ** WARNING ** ** **' >&2 echo "" >&2 -- 2.40.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next reply other threads:[~2024-04-14 17:42 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-04-14 17:41 Masahiro Yamada [this message] 2024-04-14 17:41 ` [PATCH] kbuild: buildtar: add comments about inconsistent package generation Masahiro Yamada 2024-04-15 6:59 ` Emil Renner Berthing 2024-04-15 6:59 ` Emil Renner Berthing 2024-04-16 2:07 ` Masahiro Yamada 2024-04-16 2:07 ` Masahiro Yamada 2024-04-17 19:37 ` Nicolas Schier 2024-04-17 19:37 ` Nicolas Schier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20240414174139.3001175-1-masahiroy@kernel.org \ --to=masahiroy@kernel.org \ --cc=aou@eecs.berkeley.edu \ --cc=linux-kbuild@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=nathan@kernel.org \ --cc=nicolas@fjasle.eu \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.