[Qemu-devel] [PATCH 0/2] Build fixes

[Qemu-devel] [PATCH 0/2] Build fixes

[Juan Quintela]

Hi

This series fixes the parallel build of qemu.  I tried to get the best of
Daniel and Stefan ideas with mine.
Once there fixed an obvious fix.

Daniel, can you check that parallel build also works for you?  It works for me (TM).
Stefan, what do you think?

It avoids the recursive call for serializing dependencies.
We still need to put direct dependencies for qemu-{img,nbd,io}.o of the headers
I don't understand why the ones in rules.make are not enough.

Later, Juan.

Juan Quintela (2):
  fix parallel build
  fix type in CFLAGS name

 Makefile        |    9 ++++++---
 Makefile.target |    6 +++---
 rules.mak       |    4 ++--
 3 files changed, 11 insertions(+), 8 deletions(-)

[Qemu-devel] [PATCH 1/2] fix parallel build

[Juan Quintela]

Based on a ideas of Daniel Jacobowitz + Stefan Weil

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 Makefile        |    9 ++++++---
 Makefile.target |    6 +++---
 rules.mak       |    2 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index c783aa4..ce165b2 100644
--- a/Makefile
+++ b/Makefile
@@ -40,8 +40,9 @@ config-all-devices.mak: $(SUBDIR_DEVICES_MAK)

 -include config-all-devices.mak

-build-all: config-host.h config-all-devices.h $(DOCS) $(TOOLS)
-	$(call quiet-command, $(MAKE) $(SUBDIR_MAKEFLAGS) recurse-all,)
+GENERATED_HEADERS = config-host.h config-all-devices.h
+
+build-all: $(DOCS) $(TOOLS) recurse-all

 config-host.h: config-host.h-timestamp
 config-host.h-timestamp: config-host.mak
@@ -67,7 +68,7 @@ romsubdir-%:

 ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))

-recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
+recurse-all: $(GENERATED_HEADERS) $(SUBDIR_RULES) $(ROMSUBDIR_RULES)

 #######################################################################
 # block-obj-y is code used by both qemu system emulation and qemu-img
@@ -211,6 +212,8 @@ libqemu_common.a: $(obj-y)

 qemu-img.o: qemu-img-cmds.h

+qemu-img.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS)
+
 qemu-img$(EXESUF): qemu-img.o qemu-tool.o $(block-obj-y)

 qemu-nbd$(EXESUF):  qemu-nbd.o qemu-tool.o $(block-obj-y)
diff --git a/Makefile.target b/Makefile.target
index fefd7ac..0673c55 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -22,6 +22,7 @@ endif
 endif

 PROGS=$(QEMU_PROG)
+GENERATED_HEADERS = config-target.h config-devices.h

 LIBS+=-lm

@@ -33,8 +34,7 @@ config-target.h-timestamp: config-target.mak
 config-devices.h: config-devices.h-timestamp
 config-devices.h-timestamp: config-devices.mak

-all: config-target.h config-devices.h
-	$(call quiet-command, $(MAKE) $(PROGS),)
+all: $(PROGS)

 # Dummy command so that make thinks it has done something
 	@true
@@ -308,7 +308,7 @@ endif # CONFIG_SOFTMMU

 obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o

-$(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) $(ARLIBS)
+$(QEMU_PROG): $(GENERATED_HEADERS) $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) $(ARLIBS)
 	$(call LINK,$(obj-y) $(obj-$(TARGET_BASE_ARCH)-y))


diff --git a/rules.mak b/rules.mak
index 5d7e8bb..4eb1f90 100644
--- a/rules.mak
+++ b/rules.mak
@@ -13,7 +13,7 @@ MAKEFLAGS += -rR

 QEMU_CFLAGS += -MMD -MP -MT $@

-%.o: %.c
+%.o: %.c $(GENERATED_HEADERS)
 	$(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")

 %.o: %.S
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/2] fix type in CFLAGS name

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 rules.mak |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/rules.mak b/rules.mak
index 4eb1f90..f8e5b04 100644
--- a/rules.mak
+++ b/rules.mak
@@ -17,7 +17,7 @@ QEMU_CFLAGS += -MMD -MP -MT $@
 	$(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")

 %.o: %.S
-	$(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CLAGS) -c -o $@ $<,"  AS    $(TARGET_DIR)$@")
+	$(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<,"  AS    $(TARGET_DIR)$@")

 %.o: %.m
 	$(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<,"  OBJC  $(TARGET_DIR)$@")
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 0/2] Build fixes

[Daniel Jacobowitz]

On Thu, Nov 05, 2009 at 02:42:51PM +0100, Juan Quintela wrote:
> Hi
> 
> This series fixes the parallel build of qemu.  I tried to get the best of
> Daniel and Stefan ideas with mine.
> Once there fixed an obvious fix.
> 
> Daniel, can you check that parallel build also works for you?  It works for me (TM).

No, it doesn't work.

  GEN  config-all-devices.mak
  GEN   config-host.h
  GEN   config-all-devices.h
  CC    qemu-tool.o
  CC    cutils.o
In file included from /scratch/dan/qemu/src/qemu-trunk/qemu-tool.c:14:
/scratch/dan/qemu/src/qemu-trunk/qemu-common.h:32:25: error:
config-host.h: No such file or directory
make: *** [qemu-tool.o] Error 1

You have to define GENERATED_HEADERS before you use it in
dependencies, I believe.  But you've got it way down below the
rules.mak invocation.

Also, your changes to $(QEMU_PROG) and recurse-all are not necessary;
they're the same sort of ordering assumption that caused the original
bug.  You only need to depend on generated files if you're going to
use them in this rule's commands; for headers, that means invoke a .c
to .o compilation.  The dependencies should be on exactly the rules
that need them.

I didn't bother with Makefile.target since the headers are generated
in the top level and that make will remain recursive.

-- 
Daniel Jacobowitz
CodeSourcery

[Qemu-devel] Re: [PATCH 0/2] Build fixes

[Juan Quintela]

Daniel Jacobowitz <dan@codesourcery.com> wrote:
> On Thu, Nov 05, 2009 at 02:42:51PM +0100, Juan Quintela wrote:
>> Hi
>> 
>> This series fixes the parallel build of qemu.  I tried to get the best of
>> Daniel and Stefan ideas with mine.
>> Once there fixed an obvious fix.
>> 
>> Daniel, can you check that parallel build also works for you?  It works for me (TM).
>
> No, it doesn't work.
>
>   GEN  config-all-devices.mak
>   GEN   config-host.h
>   GEN   config-all-devices.h
>   CC    qemu-tool.o
>   CC    cutils.o
> In file included from /scratch/dan/qemu/src/qemu-trunk/qemu-tool.c:14:
> /scratch/dan/qemu/src/qemu-trunk/qemu-common.h:32:25: error:
> config-host.h: No such file or directory
> make: *** [qemu-tool.o] Error 1

How are you compiling?
It works for me compiling in-tree with make -j3 (only 2 cores)

> You have to define GENERATED_HEADERS before you use it in
> dependencies, I believe.  But you've got it way down below the
> rules.mak invocation.

Will try it.

> Also, your changes to $(QEMU_PROG) and recurse-all are not necessary;

before recurse-all is necesary, because the sub-targets use
config-host.h (they don't use config-all-devices.h, but it don't hurt to
also generate it).

> they're the same sort of ordering assumption that caused the original
> bug.  You only need to depend on generated files if you're going to
> use them in this rule's commands; for headers, that means invoke a .c
> to .o compilation.  The dependencies should be on exactly the rules
> that need them.
>
> I didn't bother with Makefile.target since the headers are generated
> in the top level and that make will remain recursive.

Not all of them.
configure-target.h and config-device.h is generated there.

</me studies still more the Makefiles>

ok, subdir-% already depends on them.

I am sending a new patch with you suggestions, it works for me (again).

on-tree and out-of-tree compilation with make -j (still on my laptop
with 2 cores).

Can you test my new series?

Later, Juan.

[Qemu-devel] Re: [PATCH 0/2] Build fixes

[Daniel Jacobowitz]

On Thu, Nov 05, 2009 at 05:17:46PM +0100, Juan Quintela wrote:
> How are you compiling?
> It works for me compiling in-tree with make -j3 (only 2 cores)

I can reliably reproduce it by building all my QEMU configs at once.
I think that's a four-core or eight-core machine, -j4 in each build,
six separate qemu build trees.  I've reproduced it in a single build
tree a couple of times, but not reliably.

> > Also, your changes to $(QEMU_PROG) and recurse-all are not necessary;
> 
> before recurse-all is necesary, because the sub-targets use
> config-host.h (they don't use config-all-devices.h, but it don't hurt to
> also generate it).

Oh, you're right.

-- 
Daniel Jacobowitz
CodeSourcery

[Qemu-devel] Re: [PATCH 0/2] Build fixes

[Juan Quintela]

Daniel Jacobowitz <dan@codesourcery.com> wrote:
> On Thu, Nov 05, 2009 at 05:17:46PM +0100, Juan Quintela wrote:
>> How are you compiling?
>> It works for me compiling in-tree with make -j3 (only 2 cores)
>
> I can reliably reproduce it by building all my QEMU configs at once.
> I think that's a four-core or eight-core machine, -j4 in each build,
> six separate qemu build trees.  I've reproduced it in a single build
> tree a couple of times, but not reliably.

I have no way to reproduce reliablely :(

If last patch didn't work for you will try in a more powerful machine.

Later, Juan.

Re: [Qemu-devel] Re: [PATCH 0/2] Build fixes

[Paul Brook]

On Thursday 05 November 2009, Juan Quintela wrote:
> Daniel Jacobowitz <dan@codesourcery.com> wrote:
> > On Thu, Nov 05, 2009 at 05:17:46PM +0100, Juan Quintela wrote:
> >> How are you compiling?
> >> It works for me compiling in-tree with make -j3 (only 2 cores)
> >
> > I can reliably reproduce it by building all my QEMU configs at once.
> > I think that's a four-core or eight-core machine, -j4 in each build,
> > six separate qemu build trees.  I've reproduced it in a single build
> > tree a couple of times, but not reliably.
> 
> I have no way to reproduce reliablely :(

rm -f qemu-tool.[od] config-host.h
make qemu-tool.o

Reproduces reliably.
The following patch should expose any similar failures:

--- a/rules.mak
+++ b/rules.mak
@@ -44,5 +44,6 @@ cc-option = $(if $(shell $(CC) $1 $2 -S -o /dev/null -xc 
/dev/
        @test -f $@ || cp $< $@

 %.h-timestamp: %.mak
+       sleep 60
        $(call quiet-command, $(SRC_PATH)/create_config < $< > $@, "  GEN   
$*.h
        @cmp $@ $*.h >/dev/null 2>&1 || cp $@ $*.h


Paul

[Qemu-devel] Re: [PATCH 0/2] Build fixes

[Juan Quintela]

Paul Brook <paul@codesourcery.com> wrote:
> On Thursday 05 November 2009, Juan Quintela wrote:
>> Daniel Jacobowitz <dan@codesourcery.com> wrote:
>> > On Thu, Nov 05, 2009 at 05:17:46PM +0100, Juan Quintela wrote:
>> >> How are you compiling?
>> >> It works for me compiling in-tree with make -j3 (only 2 cores)
>> >
>> > I can reliably reproduce it by building all my QEMU configs at once.
>> > I think that's a four-core or eight-core machine, -j4 in each build,
>> > six separate qemu build trees.  I've reproduced it in a single build
>> > tree a couple of times, but not reliably.
>> 
>> I have no way to reproduce reliablely :(
>
> rm -f qemu-tool.[od] config-host.h
> make qemu-tool.o
>
> Reproduces reliably.
> The following patch should expose any similar failures:

My last series of build fixes serie fix this one also.
They are already on staging.

Thanks for the test case, Juan.

> --- a/rules.mak
> +++ b/rules.mak
> @@ -44,5 +44,6 @@ cc-option = $(if $(shell $(CC) $1 $2 -S -o /dev/null -xc 
> /dev/
>         @test -f $@ || cp $< $@
>
>  %.h-timestamp: %.mak
> +       sleep 60
>         $(call quiet-command, $(SRC_PATH)/create_config < $< > $@, "  GEN   
> $*.h
>         @cmp $@ $*.h >/dev/null 2>&1 || cp $@ $*.h
>
>
> Paul