| Message ID | 20130717160506.GI8731@rric.localhost (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 17, 2013 at 9:05 AM, Robert Richter <rric@kernel.org> wrote: > > I narrowed this down. The problem is that zinstall on ARCH=arm has a > dependency to vmlinux which does a prepare/prepare3 and finally does a > forced rebuild of kernel.release even if it exists already. > > Rebuilding it removes kernel.release first and then recreates it. This > might race with another parallel make job running depmod. > > So on arm and maybe other archs we need the same as for x86: > > 1648e4f8 x86, kbuild: make "make install" not depend on vmlinux > > The patch below fixes this for arm. It is build-tested in my > environment, but please test it in others too if possible. Ack. This looks like the right thing to do. That said, I think for clarity you might perhaps want to make the unrelated change of adding double quotes around $(KERNELRELEASE) a separate patch. Not a big deal, but it took me a moment to say "why did he do that" (in fact, I'm not sure KERNEL_RELEASE can actually validly have spaces in it, but quoting it is certainly not wrong either - but it might be unnecessary. Linus
On 17.07.13 09:36:57, Linus Torvalds wrote: > On Wed, Jul 17, 2013 at 9:05 AM, Robert Richter <rric@kernel.org> wrote: > > > > I narrowed this down. The problem is that zinstall on ARCH=arm has a > > dependency to vmlinux which does a prepare/prepare3 and finally does a > > forced rebuild of kernel.release even if it exists already. > > > > Rebuilding it removes kernel.release first and then recreates it. This > > might race with another parallel make job running depmod. > > > > So on arm and maybe other archs we need the same as for x86: > > > > 1648e4f8 x86, kbuild: make "make install" not depend on vmlinux > > > > The patch below fixes this for arm. It is build-tested in my > > environment, but please test it in others too if possible. > > Ack. This looks like the right thing to do. Thanks for looking at this. > That said, I think for clarity you might perhaps want to make the > unrelated change of adding double quotes around $(KERNELRELEASE) a > separate patch. Not a big deal, but it took me a moment to say "why > did he do that" (in fact, I'm not sure KERNEL_RELEASE can actually > validly have spaces in it, but quoting it is certainly not wrong > either - but it might be unnecessary. If kernel.release doen't exist KERNEL_RELEASE is empty, thus all args for install.sh shift one arg left. Putting it in quotes avoids this. Noticed this since the first verified file in install.sh ($2) should be the image file but was already System.map ($3). Will splitt the patches and resend. -Robert
On Wed, Jul 17, 2013 at 9:57 AM, Robert Richter <rric@kernel.org> wrote: > > If kernel.release doen't exist KERNEL_RELEASE is empty, thus all args > for install.sh shift one arg left. You know what? I read the patch, but I didn't read your changelog well enough. You already explained that in the commit log, and I just missed it like some blind monkey. My bad. > Will splitt the patches and resend. No need to split, I was wrong, it wasn't some unrelated cleanup, and you were right the first time. So Michal (or ARM people - whoever wants to take the patch), just take my ack. No objections. Linus
On 17.7.2013 19:03, Linus Torvalds wrote: > On Wed, Jul 17, 2013 at 9:57 AM, Robert Richter <rric@kernel.org> wrote: >> >> If kernel.release doen't exist KERNEL_RELEASE is empty, thus all args >> for install.sh shift one arg left. > > You know what? I read the patch, but I didn't read your changelog well > enough. You already explained that in the commit log, and I just > missed it like some blind monkey. My bad. > >> Will splitt the patches and resend. > > No need to split, I was wrong, it wasn't some unrelated cleanup, and > you were right the first time. > > So Michal (or ARM people - whoever wants to take the patch), just take > my ack. No objections. I can add it to the kbuild tree if needed. Otherwise you can add Acked-by: Michal Marek <mmarek@suse.cz>. Michal
On 18.07.13 11:22:24, Michal Marek wrote: > > So Michal (or ARM people - whoever wants to take the patch), just take > > my ack. No objections. > > I can add it to the kbuild tree if needed. Otherwise you can add > Acked-by: Michal Marek <mmarek@suse.cz>. This didn't make it upstream yet, can somebody at it to a tree? Thanks, -Robert
Robert, All, On 2013-09-30 10:49 +0200, Robert Richter spake thusly: > On 18.07.13 11:22:24, Michal Marek wrote: > > > So Michal (or ARM people - whoever wants to take the patch), just take > > > my ack. No objections. > > > > I can add it to the kbuild tree if needed. Otherwise you can add > > Acked-by: Michal Marek <mmarek@suse.cz>. > > This didn't make it upstream yet, can somebody at it to a tree? Since it's been acked-by Linus, I'll queue it in my tree, for Michal to pull from. Expect a pull-request soon. Regards, Yann E. MORIN.
On Mon, Sep 30, 2013 at 6:31 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > On 2013-09-30 10:49 +0200, Robert Richter spake thusly: >> On 18.07.13 11:22:24, Michal Marek wrote: >> > > So Michal (or ARM people - whoever wants to take the patch), just take >> > > my ack. No objections. >> > >> > I can add it to the kbuild tree if needed. Otherwise you can add >> > Acked-by: Michal Marek <mmarek@suse.cz>. >> >> This didn't make it upstream yet, can somebody at it to a tree? > > Since it's been acked-by Linus, I'll queue it in my tree, for Michal to > pull from. Expect a pull-request soon. Sorry for chiming in that late, but I didn't think of this when reading the original submission. Just doing "make oldconfig; make install" used to work. Removing the dependency of "make vmlinux" on vmlinux breaks this, doesn't it? I had the habit of doing the above many years ago, when I was mostly doing native builds, and before I had my own custom linux-install-kernel script that e.g. knows how to copy kernels and modules around to NFS servers. Not that I'm strongly attached to it, but there may be other users... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 9.10.2013 09:18, Geert Uytterhoeven wrote: > On Mon, Sep 30, 2013 at 6:31 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: >> On 2013-09-30 10:49 +0200, Robert Richter spake thusly: >>> On 18.07.13 11:22:24, Michal Marek wrote: >>>>> So Michal (or ARM people - whoever wants to take the patch), just take >>>>> my ack. No objections. >>>> >>>> I can add it to the kbuild tree if needed. Otherwise you can add >>>> Acked-by: Michal Marek <mmarek@suse.cz>. >>> >>> This didn't make it upstream yet, can somebody at it to a tree? >> >> Since it's been acked-by Linus, I'll queue it in my tree, for Michal to >> pull from. Expect a pull-request soon. > > Sorry for chiming in that late, but I didn't think of this when reading the > original submission. > > Just doing "make oldconfig; make install" used to work. On ARM and maybe other architectures. > Removing the dependency of "make vmlinux" on vmlinux breaks this, doesn't it? Yes. > I had the habit of doing the above many years ago, when I was mostly doing > native builds, and before I had my own custom linux-install-kernel > script that e.g. > knows how to copy kernels and modules around to NFS servers. > > Not that I'm strongly attached to it, but there may be other users... We can't eat the cake and have it :). What can be done is to make arch/arm/boot/install.sh print a friendlier error message, like the x86 version does: if [ ! -f "$1" ]; then echo "" 1>&2 echo " *** Missing file: $1" 1>&2 echo ' *** You need to run "make" before "make install".' 1>&2 echo "" 1>&2 exit 1 fi Michal
On Wed, Oct 9, 2013 at 12:28 PM, Michal Marek <mmarek@suse.cz> wrote: > On 9.10.2013 09:18, Geert Uytterhoeven wrote: >> Sorry for chiming in that late, but I didn't think of this when reading the >> original submission. >> >> Just doing "make oldconfig; make install" used to work. > > On ARM and maybe other architectures. Until 2009 it also worked on x86 ;-) >> Removing the dependency of "make vmlinux" on vmlinux breaks this, doesn't it? > >> Not that I'm strongly attached to it, but there may be other users... > > We can't eat the cake and have it :). What can be done is to make Sure. > arch/arm/boot/install.sh print a friendlier error message, like the x86 > version does: > > if [ ! -f "$1" ]; then > echo "" 1>&2 > echo " *** Missing file: $1" 1>&2 > echo ' *** You need to run "make" before "make install".' 1>&2 > echo "" 1>&2 > exit 1 > fi Definitely! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c0ac0f5..fe63986 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -295,10 +295,15 @@ archprepare: # Convert bzImage to zImage bzImage: zImage -zImage Image xipImage bootpImage uImage: vmlinux +BOOT_TARGETS = zImage Image xipImage bootpImage uImage +INSTALL_TARGETS = zinstall uinstall install + +PHONY += bzImage $(BOOT_TARGETS) $(INSTALL_TARGETS) + +$(BOOT_TARGETS): vmlinux $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@ -zinstall uinstall install: vmlinux +$(INSTALL_TARGETS): $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@ %.dtb: | scripts diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile index 84aa2ca..ec2f806 100644 --- a/arch/arm/boot/Makefile +++ b/arch/arm/boot/Makefile @@ -95,24 +95,24 @@ initrd: @test "$(INITRD)" != "" || \ (echo You must specify INITRD; exit -1) -install: $(obj)/Image - $(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \ +install: + $(CONFIG_SHELL) $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" \ $(obj)/Image System.map "$(INSTALL_PATH)" -zinstall: $(obj)/zImage - $(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \ +zinstall: + $(CONFIG_SHELL) $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" \ $(obj)/zImage System.map "$(INSTALL_PATH)" -uinstall: $(obj)/uImage - $(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \ +uinstall: + $(CONFIG_SHELL) $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" \ $(obj)/uImage System.map "$(INSTALL_PATH)" zi: - $(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \ + $(CONFIG_SHELL) $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" \ $(obj)/zImage System.map "$(INSTALL_PATH)" i: - $(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \ + $(CONFIG_SHELL) $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" \ $(obj)/Image System.map "$(INSTALL_PATH)" subdir- := bootp compressed dts diff --git a/arch/arm/boot/install.sh b/arch/arm/boot/install.sh index 06ea7d4..2a45092 100644 --- a/arch/arm/boot/install.sh +++ b/arch/arm/boot/install.sh @@ -20,6 +20,20 @@ # $4 - default install path (blank if root directory) # +verify () { + if [ ! -f "$1" ]; then + echo "" 1>&2 + echo " *** Missing file: $1" 1>&2 + echo ' *** You need to run "make" before "make install".' 1>&2 + echo "" 1>&2 + exit 1 + fi +} + +# Make sure the files actually exist +verify "$2" +verify "$3" + # User may have a custom install script if [ -x ~/bin/${INSTALLKERNEL} ]; then exec ~/bin/${INSTALLKERNEL} "$@"; fi if [ -x /sbin/${INSTALLKERNEL} ]; then exec /sbin/${INSTALLKERNEL} "$@"; fi