| Message ID | 20240605081623.8765-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,kvm-unit-tests] build: fix .aux.o target building | expand |
On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote: > Here's another oddity I ran into with the build system. Try run make > twice. With arm64 and ppc64, the first time it removes some intermediate > files and the second causes another rebuild of several files. After > that it's fine. s390x seems to follow a similar pattern but does not > suffer from the problem. Also, the .PRECIOUS directive is not preventing > them from being deleted inthe first place. So... that probably means I > haven't understood it properly and the fix may not be correct, but it > does appear to DTRT... Anybody with some good Makefile knowledge might > have a better idea. > $ make clean -j &>/dev/null && make -d … Successfully remade target file 'all'. Removing intermediate files... rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean). .PRECIOUS: %.o %.aux.o Fixed the issue (I’ve tested on ppc64 only). > >
On Wed, Jun 05, 2024 at 12:42:30PM GMT, Marc Hartmayer wrote: > On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote: > > Here's another oddity I ran into with the build system. Try run make > > twice. With arm64 and ppc64, the first time it removes some intermediate > > files and the second causes another rebuild of several files. After > > that it's fine. s390x seems to follow a similar pattern but does not > > suffer from the problem. Also, the .PRECIOUS directive is not preventing > > them from being deleted inthe first place. So... that probably means I > > haven't understood it properly and the fix may not be correct, but it > > does appear to DTRT... Anybody with some good Makefile knowledge might > > have a better idea. > > > > $ make clean -j &>/dev/null && make -d > … > Successfully remade target file 'all'. > Removing intermediate files... > rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o > > So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean). > > .PRECIOUS: %.o %.aux.o Just using .PRECIOUS is fine by me. Thanks, drew > > Fixed the issue (I’ve tested on ppc64 only). > > > > > > -- > Kind regards / Beste Grüße > Marc Hartmayer > > IBM Deutschland Research & Development GmbH > Vorsitzender des Aufsichtsrats: Wolfgang Wendt > Geschäftsführung: David Faller > Sitz der Gesellschaft: Böblingen > Registergericht: Amtsgericht Stuttgart, HRB 243294
On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote: > On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote: > > Here's another oddity I ran into with the build system. Try run make > > twice. With arm64 and ppc64, the first time it removes some intermediate > > files and the second causes another rebuild of several files. After > > that it's fine. s390x seems to follow a similar pattern but does not > > suffer from the problem. Also, the .PRECIOUS directive is not preventing > > them from being deleted inthe first place. So... that probably means I > > haven't understood it properly and the fix may not be correct, but it > > does appear to DTRT... Anybody with some good Makefile knowledge might > > have a better idea. > > > > $ make clean -j &>/dev/null && make -d > … > Successfully remade target file 'all'. > Removing intermediate files... > rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o > > So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean). > > .PRECIOUS: %.o %.aux.o Ah, so %.o does not match %.aux.o. That answers that. Did you see why s390x is immune? Maybe it defines the target explicitly somewhere. Is it better to define explicit targets if we want to keep them, or add to .PRECIOUS? Your patch would be simpler. Thanks, Nick
On Wed, Jun 05, 2024 at 08:53 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote: > On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote: >> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote: >> > Here's another oddity I ran into with the build system. Try run make >> > twice. With arm64 and ppc64, the first time it removes some intermediate >> > files and the second causes another rebuild of several files. After >> > that it's fine. s390x seems to follow a similar pattern but does not >> > suffer from the problem. Also, the .PRECIOUS directive is not preventing >> > them from being deleted inthe first place. So... that probably means I >> > haven't understood it properly and the fix may not be correct, but it >> > does appear to DTRT... Anybody with some good Makefile knowledge might >> > have a better idea. >> > >> >> $ make clean -j &>/dev/null && make -d >> … >> Successfully remade target file 'all'. >> Removing intermediate files... >> rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o >> >> So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean). >> >> .PRECIOUS: %.o %.aux.o > > Ah, so %.o does not match %.aux.o. That answers that. Did you see > why s390x is immune? Maybe it defines the target explicitly somewhere. Not yet :/ But what was also interesting is that if I’m using multiple jobs I don’t see the issue. make clean -j; make -j; make -j # <- the last make has nothing to do if I’m using: make clean -j; make; make -j # <- the last make has something to do… that something that irritates me > > Is it better to define explicit targets if we want to keep them, or > add to .PRECIOUS? Your patch would be simpler. Normally, I would say without .PRECIOUS it’s cleaner, but there is already a .PRECIOUS for %.so… So as Andrew has already written .PRECIOUS: %.so %.aux.o should also be fine. > > Thanks, > Nick >
On Thu Jun 6, 2024 at 1:07 AM AEST, Marc Hartmayer wrote: > On Wed, Jun 05, 2024 at 08:53 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote: > > On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote: > >> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote: > >> > Here's another oddity I ran into with the build system. Try run make > >> > twice. With arm64 and ppc64, the first time it removes some intermediate > >> > files and the second causes another rebuild of several files. After > >> > that it's fine. s390x seems to follow a similar pattern but does not > >> > suffer from the problem. Also, the .PRECIOUS directive is not preventing > >> > them from being deleted inthe first place. So... that probably means I > >> > haven't understood it properly and the fix may not be correct, but it > >> > does appear to DTRT... Anybody with some good Makefile knowledge might > >> > have a better idea. > >> > > >> > >> $ make clean -j &>/dev/null && make -d > >> … > >> Successfully remade target file 'all'. > >> Removing intermediate files... > >> rm powerpc/emulator.aux.o powerpc/tm.aux.o powerpc/spapr_hcall.aux.o powerpc/interrupts.aux.o powerpc/selftest.aux.o powerpc/smp.aux.o powerpc/selftest-migration.aux.o powerpc/spapr_vpa.aux.o powerpc/sprs.aux.o powerpc/rtas.aux.o powerpc/memory-verify.aux.o > >> > >> So an easier fix would be to add %.aux.o to .PRECIOUS (but that’s probably still not clean). > >> > >> .PRECIOUS: %.o %.aux.o > > > > Ah, so %.o does not match %.aux.o. That answers that. Did you see > > why s390x is immune? Maybe it defines the target explicitly somewhere. > > Not yet :/ Strange. Both unpatched powerpc and s390x have these lines. Looking for a rule with intermediate file 'powerpc/emulator.aux.o'. I think that's why it's considered intermediate at least for powerpc, but I can't see why s390x is different... oh, taking out the .SECONDARY lines makes s390x delete the aux intermediates. I guess those targets transitively depend on the aux files which prevents the aux from being deleted. Is that fragile? I'm not sure. The patch that introduced them was not solving this problem (that came before your %.aux.o target patch). s390x does not need any .PRECIOUS targets at all at the moment. I guess that mostly explains things. > But what was also interesting is that if I’m using multiple > jobs I don’t see the issue. > > make clean -j; make -j; make -j # <- the last make has nothing to do > > if I’m using: > > make clean -j; make; make -j # <- the last make has something to do… > that something that irritates me This is with s390x? Maybe with parallel make, the target is getting rebuilt via a different prerequisite that is not a .SECONDARY target? Adding %.aux.o in PRECIOUS there should help in that case. > > > > Is it better to define explicit targets if we want to keep them, or > > add to .PRECIOUS? Your patch would be simpler. > > Normally, I would say without .PRECIOUS it’s cleaner, but there is > already a .PRECIOUS for %.so… So as Andrew has already written > > .PRECIOUS: %.so %.aux.o > > should also be fine. Okay, for a minimal fix I will do that. Thanks, Nick
On Thu, Jun 06, 2024 at 01:49 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote: > On Thu Jun 6, 2024 at 1:07 AM AEST, Marc Hartmayer wrote: >> On Wed, Jun 05, 2024 at 08:53 PM +1000, "Nicholas Piggin" <npiggin@gmail.com> wrote: >> > On Wed Jun 5, 2024 at 8:42 PM AEST, Marc Hartmayer wrote: >> >> On Wed, Jun 05, 2024 at 06:16 PM +1000, Nicholas Piggin <npiggin@gmail.com> wrote: […snip…] > > >> But what was also interesting is that if I’m using multiple >> jobs I don’t see the issue. >> >> make clean -j; make -j; make -j # <- the last make has nothing to do >> >> if I’m using: >> >> make clean -j; make; make -j # <- the last make has something to do… >> that something that irritates me > > This is with s390x? Maybe with parallel make, the target is getting No, it’s a ppc64 cross-build on x86. > rebuilt via a different prerequisite that is not a .SECONDARY target? > Adding %.aux.o in PRECIOUS there should help in that case. Yes, it helps - that was the reason for my fix :) > >> > >> > Is it better to define explicit targets if we want to keep them, or >> > add to .PRECIOUS? Your patch would be simpler. >> >> Normally, I would say without .PRECIOUS it’s cleaner, but there is >> already a .PRECIOUS for %.so… So as Andrew has already written >> >> .PRECIOUS: %.so %.aux.o >> >> should also be fine. > > Okay, for a minimal fix I will do that. > > Thanks, > Nick >
diff --git a/arm/Makefile.common b/arm/Makefile.common index f828dbe01..3ebbcc13e 100644 --- a/arm/Makefile.common +++ b/arm/Makefile.common @@ -72,7 +72,7 @@ eabiobjs = lib/arm/eabi_compat.o FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi) ifeq ($(CONFIG_EFI),y) -%.aux.o: $(SRCDIR)/lib/auxinfo.c +$(tests-all:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c $(CC) $(CFLAGS) -c -o $@ $< \ -DPROGNAME=\"$(@:.aux.o=.efi)\" -DAUXFLAGS=$(AUXFLAGS) @@ -92,6 +92,7 @@ ifeq ($(CONFIG_EFI),y) -j .reloc \ -O binary $^ $@ else +$(tests-all:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c %.aux.o: $(SRCDIR)/lib/auxinfo.c $(CC) $(CFLAGS) -c -o $@ $< \ -DPROGNAME=\"$(@:.aux.o=.flat)\" -DAUXFLAGS=$(AUXFLAGS) diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common index b98f71c2f..3b219eee0 100644 --- a/powerpc/Makefile.common +++ b/powerpc/Makefile.common @@ -52,7 +52,7 @@ cflatobjs += lib/powerpc/smp.o OBJDIRS += lib/powerpc -%.aux.o: $(SRCDIR)/lib/auxinfo.c +$(tests-all:.elf=.aux.o): $(SRCDIR)/lib/auxinfo.c $(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\" FLATLIBS = $(libcflat) $(LIBFDT_archive) diff --git a/riscv/Makefile b/riscv/Makefile index 919a3ebb5..4610cb4d4 100644 --- a/riscv/Makefile +++ b/riscv/Makefile @@ -81,7 +81,7 @@ CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt asm-offsets = lib/riscv/asm-offsets.h include $(SRCDIR)/scripts/asm-offsets.mak -%.aux.o: $(SRCDIR)/lib/auxinfo.c +$(tests:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c $(CC) $(CFLAGS) -c -o $@ $< \ -DPROGNAME=\"$(notdir $(@:.aux.o=.$(exe)))\" -DAUXFLAGS=$(AUXFLAGS) diff --git a/s390x/Makefile b/s390x/Makefile index 23342bd64..4c0c8085c 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -178,7 +178,7 @@ lds-autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -MT $@ %.lds: %.lds.S $(asm-offsets) $(CPP) $(lds-autodepend-flags) $(CPPFLAGS) -P -C -o $@ $< -%.aux.o: $(SRCDIR)/lib/auxinfo.c +$(tests:.elf=.aux.o): $(SRCDIR)/lib/auxinfo.c $(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\" .SECONDEXPANSION: