linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile
@ 2020-11-21 19:36 Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 1/6] ARC: build: remove non-existing bootpImage from KBUILD_IMAGE Masahiro Yamada
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-11-21 19:36 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Masahiro Yamada, linux-kernel




Masahiro Yamada (6):
  ARC: build: remove non-existing bootpImage from KBUILD_IMAGE
  ARC: build: add uImage.lzma to the top-level target
  ARC: build: add boot_targets to PHONY
  ARC: build: move symlink creation to arch/arc/Makefile to avoid race
  ARC: build: remove unneeded extra-y
  ARC: build: use $(READELF) instead of hard-coded readelf

 arch/arc/Makefile      | 20 +++++++++++++-------
 arch/arc/boot/Makefile | 18 ++++--------------
 2 files changed, 17 insertions(+), 21 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] ARC: build: remove non-existing bootpImage from KBUILD_IMAGE
  2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
@ 2020-11-21 19:36 ` Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 2/6] ARC: build: add uImage.lzma to the top-level target Masahiro Yamada
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-11-21 19:36 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Masahiro Yamada, linux-kernel

The deb-pkg builds for ARCH=arc fail.

  $ export CROSS_COMPILE=<your-arc-compiler-prefix>
  $ make -s ARCH=arc defconfig
  $ make ARCH=arc bindeb-pkg
  SORTTAB vmlinux
  SYSMAP  System.map
  MODPOST Module.symvers
  make KERNELRELEASE=5.10.0-rc4 ARCH=arc KBUILD_BUILD_VERSION=2 -f ./Makefile intdeb-pkg
  sh ./scripts/package/builddeb
  cp: cannot stat 'arch/arc/boot/bootpImage': No such file or directory
  make[4]: *** [scripts/Makefile.package:87: intdeb-pkg] Error 1
  make[3]: *** [Makefile:1527: intdeb-pkg] Error 2
  make[2]: *** [debian/rules:13: binary-arch] Error 2
  dpkg-buildpackage: error: debian/rules binary subprocess returned exit status 2
  make[1]: *** [scripts/Makefile.package:83: bindeb-pkg] Error 2
  make: *** [Makefile:1527: bindeb-pkg] Error 2

The reason is obvious; arch/arc/Makefile sets $(boot)/bootpImage as
the default image, but there is no rule to build it.

Remove the meaningless KBUILD_IMAGE assignment so it will fallback
to the default vmlinux. With this change, you can build the deb package.

I removed the 'bootpImage' target as well. At best, it provides
'make bootpImage' as an alias of 'make vmlinux', but I do not see
much sense in doing so.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arc/Makefile | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index 0c6bf0d1df7a..acf99420e161 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -102,12 +102,6 @@ libs-y		+= arch/arc/lib/ $(LIBGCC)
 
 boot		:= arch/arc/boot
 
-#default target for make without any arguments.
-KBUILD_IMAGE	:= $(boot)/bootpImage
-
-all:	bootpImage
-bootpImage: vmlinux
-
 boot_targets += uImage uImage.bin uImage.gz
 
 $(boot_targets): vmlinux
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/6] ARC: build: add uImage.lzma to the top-level target
  2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 1/6] ARC: build: remove non-existing bootpImage from KBUILD_IMAGE Masahiro Yamada
@ 2020-11-21 19:36 ` Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 3/6] ARC: build: add boot_targets to PHONY Masahiro Yamada
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-11-21 19:36 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Masahiro Yamada, linux-kernel

arch/arc/boot/Makefile supports uImage.lzma, but you cannot do
'make uImage.lzma' because the corresponding target is missing
in arch/arc/Makefile. Add it.

I also changed the assignment operator '+=' to ':=' since this is the
only place where we expect this variable to be set.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index acf99420e161..61a41123ad4c 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -102,7 +102,7 @@ libs-y		+= arch/arc/lib/ $(LIBGCC)
 
 boot		:= arch/arc/boot
 
-boot_targets += uImage uImage.bin uImage.gz
+boot_targets := uImage uImage.bin uImage.gz uImage.lzma
 
 $(boot_targets): vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/6] ARC: build: add boot_targets to PHONY
  2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 1/6] ARC: build: remove non-existing bootpImage from KBUILD_IMAGE Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 2/6] ARC: build: add uImage.lzma to the top-level target Masahiro Yamada
@ 2020-11-21 19:36 ` Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 4/6] ARC: build: move symlink creation to arch/arc/Makefile to avoid race Masahiro Yamada
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-11-21 19:36 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Masahiro Yamada, linux-kernel

The top-level boot_targets (uImage and uImage.*) should be phony
targets. They just let Kbuild descend into arch/arc/boot/ and create
files there.

If a file exists in the top directory with the same name, the boot
image will not be created.

You can confirm it by the following steps:

  $ export CROSS_COMPILE=<your-arc-compiler-prefix>
  $ make -s ARCH=arc defconfig all   # vmlinux will be built
  $ touch uImage.gz
  $ make ARCH=arc uImage.gz
  CALL    scripts/atomic/check-atomics.sh
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  # arch/arc/boot/uImage.gz is not created

Specify the targets as PHONY to fix this.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arc/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index 61a41123ad4c..cf9da9aea12a 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -104,6 +104,7 @@ boot		:= arch/arc/boot
 
 boot_targets := uImage uImage.bin uImage.gz uImage.lzma
 
+PHONY += $(boot_targets)
 $(boot_targets): vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/6] ARC: build: move symlink creation to arch/arc/Makefile to avoid race
  2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
                   ` (2 preceding siblings ...)
  2020-11-21 19:36 ` [PATCH 3/6] ARC: build: add boot_targets to PHONY Masahiro Yamada
@ 2020-11-21 19:36 ` Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 5/6] ARC: build: remove unneeded extra-y Masahiro Yamada
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-11-21 19:36 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Masahiro Yamada, linux-kernel

If you run 'make uImage uImage.gz' with the parallel option, uImage.gz
will be created by two threads simultaneously.

This is because arch/arc/Makefile does not specify the dependency
between uImage and uImage.gz. Hence, GNU Make assumes they can be
built in parallel. One thread descends into arch/arc/boot/ to create
uImage, and another to create uImage.gz.

Please notice the same log is displayed twice in the following steps:

  $ export CROSS_COMPILE=<your-arc-compiler-prefix>
  $ make -s ARCH=arc defconfig
  $ make -j$(nproc) ARCH=arc uImage uImage.gz
  [ snip ]
    LD      vmlinux
    SORTTAB vmlinux
    SYSMAP  System.map
    OBJCOPY arch/arc/boot/vmlinux.bin
    OBJCOPY arch/arc/boot/vmlinux.bin
    GZIP    arch/arc/boot/vmlinux.bin.gz
    GZIP    arch/arc/boot/vmlinux.bin.gz
    UIMAGE  arch/arc/boot/uImage.gz
    UIMAGE  arch/arc/boot/uImage.gz
  Image Name:   Linux-5.10.0-rc4-00003-g62f23044
  Created:      Sun Nov 22 02:52:26 2020
  Image Type:   ARC Linux Kernel Image (gzip compressed)
  Data Size:    2109376 Bytes = 2059.94 KiB = 2.01 MiB
  Load Address: 80000000
  Entry Point:  80004000
    Image arch/arc/boot/uImage is ready
  Image Name:   Linux-5.10.0-rc4-00003-g62f23044
  Created:      Sun Nov 22 02:52:26 2020
  Image Type:   ARC Linux Kernel Image (gzip compressed)
  Data Size:    2815455 Bytes = 2749.47 KiB = 2.69 MiB
  Load Address: 80000000
  Entry Point:  80004000

This is a race between the two threads trying to write to the same file
arch/arc/boot/uImage.gz. This is a potential problem that can generate
a broken file.

I fixed a similar problem for ARM by commit 3939f3345050 ("ARM: 8418/1:
add boot image dependencies to not generate invalid images").

I highly recommend to avoid such build rules that cause a race condition.

Move the uImage rule to arch/arc/Makefile.

Another strangeness is that arch/arc/boot/Makefile compares the
timestamps between $(obj)/uImage and $(obj)/uImage.*:

  $(obj)/uImage: $(obj)/uImage.$(suffix-y)
          @ln -sf $(notdir $<) $@
          @echo '  Image $@ is ready'

This does not work as expected since $(obj)/uImage is a symlink.
The symlink should be created in a phony target rule.

I used $(kecho) instead of echo to suppress the message
'Image arch/arc/boot/uImage is ready' when the -s option is given.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arc/Makefile      | 13 ++++++++++++-
 arch/arc/boot/Makefile | 11 +----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index cf9da9aea12a..578bdbbb0fa7 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -102,11 +102,22 @@ libs-y		+= arch/arc/lib/ $(LIBGCC)
 
 boot		:= arch/arc/boot
 
-boot_targets := uImage uImage.bin uImage.gz uImage.lzma
+boot_targets := uImage.bin uImage.gz uImage.lzma
 
 PHONY += $(boot_targets)
 $(boot_targets): vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 
+uimage-default-y			:= uImage.bin
+uimage-default-$(CONFIG_KERNEL_GZIP)	:= uImage.gz
+uimage-default-$(CONFIG_KERNEL_LZMA)	:= uImage.lzma
+
+PHONY += uImage
+uImage: $(uimage-default-y)
+	@ln -sf $< $(boot)/uImage
+	@$(kecho) '  Image $(boot)/uImage is ready'
+
+CLEAN_FILES += $(boot)/uImage
+
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
diff --git a/arch/arc/boot/Makefile b/arch/arc/boot/Makefile
index 538b92f4dd25..3b1f8a69a89e 100644
--- a/arch/arc/boot/Makefile
+++ b/arch/arc/boot/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-targets := vmlinux.bin vmlinux.bin.gz uImage
+targets := vmlinux.bin vmlinux.bin.gz
 
 # uImage build relies on mkimage being availble on your host for ARC target
 # You will need to build u-boot for ARC, rename mkimage to arc-elf32-mkimage
@@ -13,11 +13,6 @@ LINUX_START_TEXT = $$(readelf -h vmlinux | \
 UIMAGE_LOADADDR    = $(CONFIG_LINUX_LINK_BASE)
 UIMAGE_ENTRYADDR   = $(LINUX_START_TEXT)
 
-suffix-y := bin
-suffix-$(CONFIG_KERNEL_GZIP)	:= gz
-suffix-$(CONFIG_KERNEL_LZMA)	:= lzma
-
-targets += uImage
 targets += uImage.bin
 targets += uImage.gz
 targets += uImage.lzma
@@ -42,7 +37,3 @@ $(obj)/uImage.gz: $(obj)/vmlinux.bin.gz FORCE
 
 $(obj)/uImage.lzma: $(obj)/vmlinux.bin.lzma FORCE
 	$(call if_changed,uimage,lzma)
-
-$(obj)/uImage: $(obj)/uImage.$(suffix-y)
-	@ln -sf $(notdir $<) $@
-	@echo '  Image $@ is ready'
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/6] ARC: build: remove unneeded extra-y
  2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
                   ` (3 preceding siblings ...)
  2020-11-21 19:36 ` [PATCH 4/6] ARC: build: move symlink creation to arch/arc/Makefile to avoid race Masahiro Yamada
@ 2020-11-21 19:36 ` Masahiro Yamada
  2020-11-21 19:36 ` [PATCH 6/6] ARC: build: use $(READELF) instead of hard-coded readelf Masahiro Yamada
  2020-11-25  4:40 ` [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Vineet Gupta
  6 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-11-21 19:36 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Masahiro Yamada, linux-kernel

Adding vmlinux.* to extra-y has no point because we expect they are
built on demand while building uImage.*

Add them to 'targets' is enough to include the corresponding .cmd file.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arc/boot/Makefile | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arc/boot/Makefile b/arch/arc/boot/Makefile
index 3b1f8a69a89e..b3870cc100bf 100644
--- a/arch/arc/boot/Makefile
+++ b/arch/arc/boot/Makefile
@@ -1,5 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-targets := vmlinux.bin vmlinux.bin.gz
 
 # uImage build relies on mkimage being availble on your host for ARC target
 # You will need to build u-boot for ARC, rename mkimage to arc-elf32-mkimage
@@ -13,12 +12,12 @@ LINUX_START_TEXT = $$(readelf -h vmlinux | \
 UIMAGE_LOADADDR    = $(CONFIG_LINUX_LINK_BASE)
 UIMAGE_ENTRYADDR   = $(LINUX_START_TEXT)
 
+targets += vmlinux.bin
+targets += vmlinux.bin.gz
+targets += vmlinux.bin.lzma
 targets += uImage.bin
 targets += uImage.gz
 targets += uImage.lzma
-extra-y += vmlinux.bin
-extra-y += vmlinux.bin.gz
-extra-y += vmlinux.bin.lzma
 
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/6] ARC: build: use $(READELF) instead of hard-coded readelf
  2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
                   ` (4 preceding siblings ...)
  2020-11-21 19:36 ` [PATCH 5/6] ARC: build: remove unneeded extra-y Masahiro Yamada
@ 2020-11-21 19:36 ` Masahiro Yamada
  2020-11-25  4:40 ` [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Vineet Gupta
  6 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-11-21 19:36 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Masahiro Yamada, linux-kernel

The top Makefile defines READELF as the readelf in the cross-toolchains.
Use it rather than the host readelf.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arc/boot/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/boot/Makefile b/arch/arc/boot/Makefile
index b3870cc100bf..5648748c285f 100644
--- a/arch/arc/boot/Makefile
+++ b/arch/arc/boot/Makefile
@@ -6,7 +6,7 @@
 
 OBJCOPYFLAGS= -O binary -R .note -R .note.gnu.build-id -R .comment -S
 
-LINUX_START_TEXT = $$(readelf -h vmlinux | \
+LINUX_START_TEXT = $$($(READELF) -h vmlinux | \
 			grep "Entry point address" | grep -o 0x.*)
 
 UIMAGE_LOADADDR    = $(CONFIG_LINUX_LINK_BASE)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile
  2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
                   ` (5 preceding siblings ...)
  2020-11-21 19:36 ` [PATCH 6/6] ARC: build: use $(READELF) instead of hard-coded readelf Masahiro Yamada
@ 2020-11-25  4:40 ` Vineet Gupta
  2020-12-01 12:27   ` Masahiro Yamada
  6 siblings, 1 reply; 9+ messages in thread
From: Vineet Gupta @ 2020-11-25  4:40 UTC (permalink / raw)
  To: Masahiro Yamada, linux-snps-arc; +Cc: linux-kernel

Hi Masahiro San,

On 11/21/20 11:36 AM, Masahiro Yamada wrote:
>
>
> Masahiro Yamada (6):
>    ARC: build: remove non-existing bootpImage from KBUILD_IMAGE
>    ARC: build: add uImage.lzma to the top-level target
>    ARC: build: add boot_targets to PHONY
>    ARC: build: move symlink creation to arch/arc/Makefile to avoid race
>    ARC: build: remove unneeded extra-y
>    ARC: build: use $(READELF) instead of hard-coded readelf
>
>   arch/arc/Makefile      | 20 +++++++++++++-------
>   arch/arc/boot/Makefile | 18 ++++--------------
>   2 files changed, 17 insertions(+), 21 deletions(-)

This LGTM. Do you want me to pick up these via ARC tree ?

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile
  2020-11-25  4:40 ` [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Vineet Gupta
@ 2020-12-01 12:27   ` Masahiro Yamada
  0 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2020-12-01 12:27 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: linux-snps-arc, linux-kernel

On Wed, Nov 25, 2020 at 1:40 PM Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
>
> Hi Masahiro San,
>
> On 11/21/20 11:36 AM, Masahiro Yamada wrote:
> >
> >
> > Masahiro Yamada (6):
> >    ARC: build: remove non-existing bootpImage from KBUILD_IMAGE
> >    ARC: build: add uImage.lzma to the top-level target
> >    ARC: build: add boot_targets to PHONY
> >    ARC: build: move symlink creation to arch/arc/Makefile to avoid race
> >    ARC: build: remove unneeded extra-y
> >    ARC: build: use $(READELF) instead of hard-coded readelf
> >
> >   arch/arc/Makefile      | 20 +++++++++++++-------
> >   arch/arc/boot/Makefile | 18 ++++--------------
> >   2 files changed, 17 insertions(+), 21 deletions(-)
>
> This LGTM. Do you want me to pick up these via ARC tree ?
>
> Thx,
> -Vineet

Yes, please. Thanks.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-12-01 12:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-21 19:36 [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Masahiro Yamada
2020-11-21 19:36 ` [PATCH 1/6] ARC: build: remove non-existing bootpImage from KBUILD_IMAGE Masahiro Yamada
2020-11-21 19:36 ` [PATCH 2/6] ARC: build: add uImage.lzma to the top-level target Masahiro Yamada
2020-11-21 19:36 ` [PATCH 3/6] ARC: build: add boot_targets to PHONY Masahiro Yamada
2020-11-21 19:36 ` [PATCH 4/6] ARC: build: move symlink creation to arch/arc/Makefile to avoid race Masahiro Yamada
2020-11-21 19:36 ` [PATCH 5/6] ARC: build: remove unneeded extra-y Masahiro Yamada
2020-11-21 19:36 ` [PATCH 6/6] ARC: build: use $(READELF) instead of hard-coded readelf Masahiro Yamada
2020-11-25  4:40 ` [PATCH 0/6] ARC: build: fix various issues in arc boot Makefile Vineet Gupta
2020-12-01 12:27   ` Masahiro Yamada

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).