[PATCH 0/2] Fix make -j race due to stubdom and tools make -C

[PATCH 0/2] Fix make -j race due to stubdom and tools make -C

[Ian Jackson]

Ian Jackson (2):
  build system: make install-stubdom depend on install-tools again
  DO NOT APPLY git-checkout.sh: add sleep to demonstrate race

 Makefile                | 2 +-
 scripts/git-checkout.sh | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 0/2] Fix make -j race due to stubdom and tools make -C

[Ian Jackson]

Ian Jackson (2):
  build system: make install-stubdom depend on install-tools again
  DO NOT APPLY git-checkout.sh: add sleep to demonstrate race

 Makefile                | 2 +-
 scripts/git-checkout.sh | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/2] build system: make install-stubdom depend on install-tools again

[Ian Jackson]

In d290e325179ccee966cd679d0fed48be6f4cc1b7
  "build system: don't let install-stubdom depend on install-tools"
the dependency of install-stubdom on install-tools was removed.

However, this was not correct.   stubdom/Makefile contains this:

  $(XEN_ROOT)/tools/qemu-xen-traditional-dir:
       $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find

As I have written before:

  With recursive make, it is necessary for the overall structure of the
  makefiles to sequence things so that each directory is entered exactly
  once, before its dependent directories are entered.  (It is possible
  to violate this rule without creating races but it is tricky and
  inadvisable.)

Since d290e325179c, it can happen that the command for the
qemu-xen-traditional-dir-find rule is run twice simultaneously - once
as a result of $(MAKE) -C tools install, and once as a result of
$(MAKE) -C stubdom install.  If you get unlucky, this causes lossage.
(This just happened to me in an osstest flight.)

In principle we could alternatively fix this by lifting the commands
in the qemu-xen-traditional-dir-find target (and perhaps other things
too) into the toplevel Makefile, as was done for mini-os.

But that seems overkill given how bad the stubdom build system is, and
the fact that we think at some point this qemu-trad will go away
entirely.  Adding the tools dependency back to the stubdom build is
by and large good enough.

(Someone who really wants to build stubdom without tools is welcome to
do this separation if they really want to.)

CC: Juergen Gross <jgross@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d959cd5b47..829ac63741 100644
--- a/Makefile
+++ b/Makefile
@@ -127,7 +127,7 @@ install-tools: install-tools-public-headers
 	$(MAKE) -C tools install
 
 .PHONY: install-stubdom
-install-stubdom: mini-os-dir install-tools-public-headers
+install-stubdom: mini-os-dir install-tools
 	$(MAKE) -C stubdom install
 ifeq (x86_64,$(XEN_TARGET_ARCH))
 	XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom install-grub
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/2] build system: make install-stubdom depend on install-tools again

[Ian Jackson]

In d290e325179ccee966cd679d0fed48be6f4cc1b7
  "build system: don't let install-stubdom depend on install-tools"
the dependency of install-stubdom on install-tools was removed.

However, this was not correct.   stubdom/Makefile contains this:

  $(XEN_ROOT)/tools/qemu-xen-traditional-dir:
       $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find

As I have written before:

  With recursive make, it is necessary for the overall structure of the
  makefiles to sequence things so that each directory is entered exactly
  once, before its dependent directories are entered.  (It is possible
  to violate this rule without creating races but it is tricky and
  inadvisable.)

Since d290e325179c, it can happen that the command for the
qemu-xen-traditional-dir-find rule is run twice simultaneously - once
as a result of $(MAKE) -C tools install, and once as a result of
$(MAKE) -C stubdom install.  If you get unlucky, this causes lossage.
(This just happened to me in an osstest flight.)

In principle we could alternatively fix this by lifting the commands
in the qemu-xen-traditional-dir-find target (and perhaps other things
too) into the toplevel Makefile, as was done for mini-os.

But that seems overkill given how bad the stubdom build system is, and
the fact that we think at some point this qemu-trad will go away
entirely.  Adding the tools dependency back to the stubdom build is
by and large good enough.

(Someone who really wants to build stubdom without tools is welcome to
do this separation if they really want to.)

CC: Juergen Gross <jgross@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d959cd5b47..829ac63741 100644
--- a/Makefile
+++ b/Makefile
@@ -127,7 +127,7 @@ install-tools: install-tools-public-headers
 	$(MAKE) -C tools install
 
 .PHONY: install-stubdom
-install-stubdom: mini-os-dir install-tools-public-headers
+install-stubdom: mini-os-dir install-tools
 	$(MAKE) -C stubdom install
 ifeq (x86_64,$(XEN_TARGET_ARCH))
 	XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom install-grub
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/2] DO NOT APPLY git-checkout.sh: add sleep to demonstrate race

[Ian Jackson]

This allows easy reproduction of git-checkout races: each checkout
waits until you kill its sleep.  You can see the multiple identical
checkout runes in ps etc.  A good rune is:
  watch 'ps -fHtpts/60'
(replacing pts/60 with the pty of your build job).

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 scripts/git-checkout.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh
index 20ae31ff23..290026ad32 100755
--- a/scripts/git-checkout.sh
+++ b/scripts/git-checkout.sh
@@ -11,6 +11,8 @@ DIR=$3
 
 set -e
 
+sleep 1000 ||:
+
 if test \! -d $DIR-remote; then
 	rm -rf $DIR-remote $DIR-remote.tmp
 	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 2/2] DO NOT APPLY git-checkout.sh: add sleep to demonstrate race

[Ian Jackson]

This allows easy reproduction of git-checkout races: each checkout
waits until you kill its sleep.  You can see the multiple identical
checkout runes in ps etc.  A good rune is:
  watch 'ps -fHtpts/60'
(replacing pts/60 with the pty of your build job).

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 scripts/git-checkout.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh
index 20ae31ff23..290026ad32 100755
--- a/scripts/git-checkout.sh
+++ b/scripts/git-checkout.sh
@@ -11,6 +11,8 @@ DIR=$3
 
 set -e
 
+sleep 1000 ||:
+
 if test \! -d $DIR-remote; then
 	rm -rf $DIR-remote $DIR-remote.tmp
 	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] build system: make install-stubdom depend on install-tools again

[Wei Liu]

On Mon, Apr 15, 2019 at 05:28:08PM +0100, Ian Jackson wrote:
> In d290e325179ccee966cd679d0fed48be6f4cc1b7
>   "build system: don't let install-stubdom depend on install-tools"
> the dependency of install-stubdom on install-tools was removed.
> 
> However, this was not correct.   stubdom/Makefile contains this:
> 
>   $(XEN_ROOT)/tools/qemu-xen-traditional-dir:
>        $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find
> 
> As I have written before:
> 
>   With recursive make, it is necessary for the overall structure of the
>   makefiles to sequence things so that each directory is entered exactly
>   once, before its dependent directories are entered.  (It is possible
>   to violate this rule without creating races but it is tricky and
>   inadvisable.)
> 
> Since d290e325179c, it can happen that the command for the
> qemu-xen-traditional-dir-find rule is run twice simultaneously - once
> as a result of $(MAKE) -C tools install, and once as a result of
> $(MAKE) -C stubdom install.  If you get unlucky, this causes lossage.
> (This just happened to me in an osstest flight.)
> 
> In principle we could alternatively fix this by lifting the commands
> in the qemu-xen-traditional-dir-find target (and perhaps other things
> too) into the toplevel Makefile, as was done for mini-os.
> 
> But that seems overkill given how bad the stubdom build system is, and
> the fact that we think at some point this qemu-trad will go away
> entirely.  Adding the tools dependency back to the stubdom build is
> by and large good enough.
> 
> (Someone who really wants to build stubdom without tools is welcome to
> do this separation if they really want to.)
> 
> CC: Juergen Gross <jgross@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] build system: make install-stubdom depend on install-tools again

[Wei Liu]

On Mon, Apr 15, 2019 at 05:28:08PM +0100, Ian Jackson wrote:
> In d290e325179ccee966cd679d0fed48be6f4cc1b7
>   "build system: don't let install-stubdom depend on install-tools"
> the dependency of install-stubdom on install-tools was removed.
> 
> However, this was not correct.   stubdom/Makefile contains this:
> 
>   $(XEN_ROOT)/tools/qemu-xen-traditional-dir:
>        $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find
> 
> As I have written before:
> 
>   With recursive make, it is necessary for the overall structure of the
>   makefiles to sequence things so that each directory is entered exactly
>   once, before its dependent directories are entered.  (It is possible
>   to violate this rule without creating races but it is tricky and
>   inadvisable.)
> 
> Since d290e325179c, it can happen that the command for the
> qemu-xen-traditional-dir-find rule is run twice simultaneously - once
> as a result of $(MAKE) -C tools install, and once as a result of
> $(MAKE) -C stubdom install.  If you get unlucky, this causes lossage.
> (This just happened to me in an osstest flight.)
> 
> In principle we could alternatively fix this by lifting the commands
> in the qemu-xen-traditional-dir-find target (and perhaps other things
> too) into the toplevel Makefile, as was done for mini-os.
> 
> But that seems overkill given how bad the stubdom build system is, and
> the fact that we think at some point this qemu-trad will go away
> entirely.  Adding the tools dependency back to the stubdom build is
> by and large good enough.
> 
> (Someone who really wants to build stubdom without tools is welcome to
> do this separation if they really want to.)
> 
> CC: Juergen Gross <jgross@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] build system: make install-stubdom depend on install-tools again

[Ian Jackson]

Wei Liu writes ("Re: [PATCH 1/2] build system: make install-stubdom depend on install-tools again"):
> On Mon, Apr 15, 2019 at 05:28:08PM +0100, Ian Jackson wrote:
> > In d290e325179ccee966cd679d0fed48be6f4cc1b7
> >   "build system: don't let install-stubdom depend on install-tools"
> > the dependency of install-stubdom on install-tools was removed.
> > 
> > However, this was not correct.   stubdom/Makefile contains this:
> > 
> >   $(XEN_ROOT)/tools/qemu-xen-traditional-dir:
> >        $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find
...
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks.  Since this is related to unblocking osstest (it is causing
spurious build failures in various push gates), I have pushed it.

Unfortunately I think this patch is not suitable for backport so this
bug will continue to trouble us in stable branches from 4.9 onwards.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] build system: make install-stubdom depend on install-tools again

[Ian Jackson]

Wei Liu writes ("Re: [PATCH 1/2] build system: make install-stubdom depend on install-tools again"):
> On Mon, Apr 15, 2019 at 05:28:08PM +0100, Ian Jackson wrote:
> > In d290e325179ccee966cd679d0fed48be6f4cc1b7
> >   "build system: don't let install-stubdom depend on install-tools"
> > the dependency of install-stubdom on install-tools was removed.
> > 
> > However, this was not correct.   stubdom/Makefile contains this:
> > 
> >   $(XEN_ROOT)/tools/qemu-xen-traditional-dir:
> >        $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find
...
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks.  Since this is related to unblocking osstest (it is causing
spurious build failures in various push gates), I have pushed it.

Unfortunately I think this patch is not suitable for backport so this
bug will continue to trouble us in stable branches from 4.9 onwards.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel