[XEN PATCH] libs/util: Fix parallel build between flex/bison and CC rules

Anthony PERARD posted 1 patch 1 year, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
tools/libs/util/Makefile | 8 ++++++++
1 file changed, 8 insertions(+)
[XEN PATCH] libs/util: Fix parallel build between flex/bison and CC rules
Posted by Anthony PERARD 1 year, 2 months ago
flex/bison generate two targets, and when those targets are
prerequisite of other rules they are considered independently by make.

We can have a situation where the .c file is out-of-date but not the
.h, git checkout for example. In this case, if a rule only have the .h
file as prerequiste, make will procced and start to build the object.
In parallel, another target can have the .c file as prerequisite and
make will find out it need re-generating and do so, changing the .h at
the same time. This parallel task breaks the first one.

To avoid this scenario, we put both the header and the source as
prerequisite for all object even if they only need the header.

Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

We could add those new prerequisite as order-only, but since the
header is going to be re-generated anyway, the object will be too.

I tried to simply have something like
"libxlu_cfg_l.h: libxlu_cfg_l.c", but make find it to be a circular
dependency.

I tried to use grouped target ( &: ) with a recent version of make
(4.3), but that didn't work. And we couldn't use it as it's a new
feature.

As reference, this is the minimum dependencies, as not all object
needs all generated headers, but keeping that in sync with source
change could be problematique:

libxlu_cfg_l.o libxlu_cfg_l.opic: libxlu_cfg_y.h
libxlu_cfg_y.o libxlu_cfg_y.opic: libxlu_cfg_l.h

libxlu_cfg.o libxlu_cfg.opic: libxlu_cfg_y.h libxlu_cfg_l.h
libxlu_disk.o libxlu_disk.opic: libxlu_cfg_y.h libxlu_disk_l.h
libxlu_vif.o:
libxlu_pci.o:

    And the .c files prerequisite
libxlu_cfg_l.o libxlu_cfg_l.opic: libxlu_cfg_y.c
libxlu_cfg_y.o libxlu_cfg_y.opic: libxlu_cfg_l.c
libxlu_cfg.o libxlu_cfg.opic: libxlu_cfg_y.c libxlu_cfg_l.c
libxlu_disk.o libxlu_disk.opic: libxlu_cfg_y.c libxlu_disk_l.c
---
 tools/libs/util/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/libs/util/Makefile b/tools/libs/util/Makefile
index 93ebf930a4..8a8c218eb5 100644
--- a/tools/libs/util/Makefile
+++ b/tools/libs/util/Makefile
@@ -28,6 +28,14 @@ include $(XEN_ROOT)/tools/libs/libs.mk
 
 $(OBJS-y) $(PIC_OBJS): $(AUTOINCS)
 
+# Adding the .c conterparts of the headers generated by flex/bison as
+# prerequisite of all objects.
+# This is to tell make that if only the .c file is out-of-date but not the
+# header, it should still wait for the .c file to be rebuilt.
+# Otherwise, make doesn't considered "%.c %.h" as grouped targets, and will run
+# the flex/bison rules in parallel of CC rules which only need the header.
+$(OBJS-y) $(PIC_OBJS): libxlu_cfg_l.c libxlu_cfg_y.c libxlu_disk_l.c
+
 %.c %.h: %.y
 	@rm -f $*.[ch]
 	$(BISON) --output=$*.c $<
-- 
Anthony PERARD
Re: [XEN PATCH] libs/util: Fix parallel build between flex/bison and CC rules
Posted by Andrew Cooper 1 year, 2 months ago
On 09/02/2023 12:11 pm, Anthony PERARD wrote:
> flex/bison generate two targets, and when those targets are
> prerequisite of other rules they are considered independently by make.
>
> We can have a situation where the .c file is out-of-date but not the
> .h, git checkout for example. In this case, if a rule only have the .h

Nit.  s/have/has/

(Can fix on commit.)

> file as prerequiste, make will procced and start to build the object.
> In parallel, another target can have the .c file as prerequisite and
> make will find out it need re-generating and do so, changing the .h at
> the same time. This parallel task breaks the first one.
>
> To avoid this scenario, we put both the header and the source as
> prerequisite for all object even if they only need the header.
>
> Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

This is a very subtle corner case.

~Andrew

[XEN PATCH v2] libs/util: Fix parallel build between flex/bison and CC rules
Posted by Anthony PERARD 1 year, 2 months ago
flex/bison generate two targets, and when those targets are
prerequisite of other rules they are considered independently by make.

We can have a situation where the .c file is out-of-date but not the
.h, git checkout for example. In this case, if a rule only have the .h
file as prerequiste, make will procced and start to build the object.
In parallel, another target can have the .c file as prerequisite and
make will find out it need re-generating and do so, changing the .h at
the same time. This parallel task breaks the first one.

To avoid this scenario, we put both the header and the source as
prerequisite for all object even if they only need the header.

Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

v2:
- fix patch context

We could add those new prerequisite as order-only, but since the
header is going to be re-generated anyway, the object will be too.

I tried to simply have something like
"libxlu_cfg_l.h: libxlu_cfg_l.c", but make find it to be a circular
dependency.

I tried to use grouped target ( &: ) with a recent version of make
(4.3), but that didn't work. And we couldn't use it as it's a new
feature.

As reference, this is the minimum dependencies, as not all object
needs all generated headers, but keeping that in sync with source
change could be problematique:

libxlu_cfg_l.o libxlu_cfg_l.opic: libxlu_cfg_y.h
libxlu_cfg_y.o libxlu_cfg_y.opic: libxlu_cfg_l.h

libxlu_cfg.o libxlu_cfg.opic: libxlu_cfg_y.h libxlu_cfg_l.h
libxlu_disk.o libxlu_disk.opic: libxlu_cfg_y.h libxlu_disk_l.h
libxlu_vif.o:
libxlu_pci.o:

    And the .c files prerequisite
libxlu_cfg_l.o libxlu_cfg_l.opic: libxlu_cfg_y.c
libxlu_cfg_y.o libxlu_cfg_y.opic: libxlu_cfg_l.c
libxlu_cfg.o libxlu_cfg.opic: libxlu_cfg_y.c libxlu_cfg_l.c
libxlu_disk.o libxlu_disk.opic: libxlu_cfg_y.c libxlu_disk_l.c
---
 tools/libs/util/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/libs/util/Makefile b/tools/libs/util/Makefile
index e016baf888..c3b21875dc 100644
--- a/tools/libs/util/Makefile
+++ b/tools/libs/util/Makefile
@@ -40,6 +40,14 @@ include $(XEN_ROOT)/tools/libs/libs.mk
 
 $(OBJS-y) $(PIC_OBJS): $(AUTOINCS)
 
+# Adding the .c conterparts of the headers generated by flex/bison as
+# prerequisite of all objects.
+# This is to tell make that if only the .c file is out-of-date but not the
+# header, it should still wait for the .c file to be rebuilt.
+# Otherwise, make doesn't considered "%.c %.h" as grouped targets, and will run
+# the flex/bison rules in parallel of CC rules which only need the header.
+$(OBJS-y) $(PIC_OBJS): libxlu_cfg_l.c libxlu_cfg_y.c libxlu_disk_l.c
+
 %.c %.h:: %.y
 	@rm -f $*.[ch]
 	$(BISON) --output=$*.c $<
-- 
Anthony PERARD