[XEN PATCH 0/4] xen: rework compat headers generation

[XEN PATCH 0/4] xen: rework compat headers generation

[Anthony PERARD]

Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-include-rework-v1

Hi,

This patch series is about 2 improvement. First one is to use $(if_changed, )
in "include/Makefile" to make the generation of the compat headers less verbose
and to have the command line part of the decision to rebuild the headers.
Second one is to replace one slow script by a much faster one, and save time
when generating the headers.

Thanks.

Anthony PERARD (4):
  build: xen/include: use if_changed
  build: set PERL
  build: replace get-fields.sh by a perl script
  build: remove auto.conf prerequisite from compat/xlat.h target

 xen/Makefile                 |   1 +
 xen/include/Makefile         | 106 ++++---
 xen/tools/compat-xlat-header | 539 +++++++++++++++++++++++++++++++++++
 xen/tools/get-fields.sh      | 528 ----------------------------------
 4 files changed, 614 insertions(+), 560 deletions(-)
 create mode 100755 xen/tools/compat-xlat-header
 delete mode 100644 xen/tools/get-fields.sh

-- 
Anthony PERARD

[XEN PATCH 1/4] build: xen/include: use if_changed

[Anthony PERARD]

Use "define" for the headers*_chk commands as otherwise the "#"
is interpreted as a comment and make can't find the end of
$(foreach,).

Adding several .PRECIOUS as without them `make` deletes the
intermediate targets. This is an issue because the macro $(if_changed,)
check if the target exist in order to decide whether to recreate the
target.

Removing the call to `mkdir` from the commands. Those aren't needed
anymore because a rune in Rules.mk creates the directory for each
$(targets).

Remove "export PYTHON" as it is already exported.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/include/Makefile | 108 ++++++++++++++++++++++++++++++-------------
 1 file changed, 76 insertions(+), 32 deletions(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 03baf10efb..6d9bcc19b0 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -45,38 +45,65 @@ public-$(CONFIG_ARM) := $(wildcard $(srcdir)/public/arch-arm/*.h $(srcdir)/publi
 .PHONY: all
 all: $(addprefix $(obj)/,$(headers-y))
 
-$(obj)/compat/%.h: $(obj)/compat/%.i $(srcdir)/Makefile $(srctree)/tools/compat-build-header.py
-	$(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
-	mv -f $@.new $@
-
-$(obj)/compat/%.i: $(obj)/compat/%.c $(srcdir)/Makefile
-	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
-
-$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srcdir)/Makefile $(srctree)/tools/compat-build-source.py
-	mkdir -p $(@D)
-	$(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new
-	mv -f $@.new $@
-
-$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh $(srcdir)/Makefile
-	export PYTHON=$(PYTHON); \
-	while read what name; do \
-		$(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
-	done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new
-	mv -f $@.new $@
+quiet_cmd_compat_h = GEN     $@
+cmd_compat_h = \
+    $(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
+    mv -f $@.new $@
+
+quiet_cmd_compat_i = CPP     $@
+cmd_compat_i = $(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
+
+quiet_cmd_compat_c = GEN     $@
+cmd_compat_c = \
+   $(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new; \
+   mv -f $@.new $@
+
+quiet_cmd_xlat_headers = GEN     $@
+cmd_xlat_headers = \
+    while read what name; do \
+        $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
+    done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new; \
+    mv -f $@.new $@
+
+targets += $(headers-y)
+$(obj)/compat/%.h: $(obj)/compat/%.i $(srctree)/tools/compat-build-header.py FORCE
+	$(call if_changed,compat_h)
+
+.PRECIOUS: $(obj)/compat/%.i
+targets += $(patsubst %.h, %.i, $(headers-y))
+$(obj)/compat/%.i: $(obj)/compat/%.c FORCE
+	$(call if_changed,compat_i)
+
+.PRECIOUS: $(obj)/compat/%.c
+targets += $(patsubst %.h, %.c, $(headers-y))
+$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srctree)/tools/compat-build-source.py FORCE
+	$(call if_changed,compat_c)
+
+targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y))
+$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh FORCE
+	$(call if_changed,xlat_headers)
+
+quiet_cmd_xlat_lst = GEN     $@
+cmd_xlat_lst = \
+	grep -v '^[[:blank:]]*$(pound)' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new; \
+	$(call move-if-changed,$@.new,$@)
 
 .PRECIOUS: $(obj)/compat/.xlat/%.lst
-$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst $(srcdir)/Makefile
-	mkdir -p $(@D)
-	grep -v '^[[:blank:]]*#' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new
-	$(call move-if-changed,$@.new,$@)
+targets += $(patsubst compat/%.h, compat/.xlat/%.lst, $(headers-y))
+$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
+	$(call if_changed,xlat_lst)
 
 xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
 xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
 
-$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf $(srcdir)/Makefile
-	cat $(filter %.h,$^) >$@.new
+quiet_cmd_xlat_h = GEN     $@
+cmd_xlat_h = \
+	cat $(filter %.h,$^) >$@.new; \
 	mv -f $@.new $@
 
+$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf FORCE
+	$(call if_changed,xlat_h)
+
 ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
 
 all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
@@ -102,27 +129,31 @@ PUBLIC_C99_HEADERS := $(call public-filter-headers,public-c99-headers)
 $(src)/public/io/9pfs.h-prereq := string
 $(src)/public/io/pvcalls.h-prereq := string
 
-$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) $(srcdir)/Makefile
+quiet_cmd_header_chk = CHK     $@
+cmd_header_chk = \
 	for i in $(filter %.h,$^); do \
 	    $(CC) -x c -ansi -Wall -Werror -include stdint.h \
 	          -S -o /dev/null $$i || exit 1; \
 	    echo $$i; \
-	done >$@.new
+	done >$@.new; \
 	mv $@.new $@
 
-$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) $(srcdir)/Makefile
-	rm -f $@.new
+quiet_cmd_headers99_chk = CHK     $@
+define cmd_headers99_chk
+	rm -f $@.new; \
 	$(foreach i, $(filter %.h,$^),                                        \
 	    echo "#include "\"$(i)\"                                          \
 	    | $(CC) -x c -std=c99 -Wall -Werror                               \
 	      -include stdint.h                                               \
 	      $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include $(j).h) \
 	      -S -o /dev/null -                                               \
-	    || exit $$?; echo $(i) >> $@.new;)
+	    || exit $$?; echo $(i) >> $@.new;) \
 	mv $@.new $@
+endef
 
-$(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
-	rm -f $@.new
+quiet_cmd_headerscxx_chk = CHK     $@
+define cmd_headerscxx_chk
+	rm -f $@.new; \
 	if ! $(CXX) -v >/dev/null 2>&1; then                                  \
 	    touch $@.new;                                                     \
 	    exit 0;                                                           \
@@ -133,8 +164,21 @@ $(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
 	      -include stdint.h -include $(srcdir)/public/xen.h               \
 	      $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \
 	      -S -o /dev/null -                                               \
-	    || exit $$?; echo $(i) >> $@.new;)
+	    || exit $$?; echo $(i) >> $@.new;) \
 	mv $@.new $@
+endef
+
+targets += headers.chk
+$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) FORCE
+	$(call if_changed,header_chk)
+
+targets += headers99.chk
+$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) FORCE
+	$(call if_changed,headers99_chk)
+
+targets += headers++.chk
+$(obj)/headers++.chk: $(PUBLIC_HEADERS) FORCE
+	$(call if_changed,headerscxx_chk)
 
 endif
 
-- 
Anthony PERARD

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Jan Beulich]

On 01.06.2022 18:59, Anthony PERARD wrote:
> Use "define" for the headers*_chk commands as otherwise the "#"
> is interpreted as a comment and make can't find the end of
> $(foreach,).

In cmd_xlat_lst you use $(pound) - any reason this doesn't work in
these rules? Note that I don't mind the use of "define", just that I'm
puzzled by the justification.

Jan

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Anthony PERARD]

On Thu, Jun 02, 2022 at 11:11:15AM +0200, Jan Beulich wrote:
> On 01.06.2022 18:59, Anthony PERARD wrote:
> > Use "define" for the headers*_chk commands as otherwise the "#"
> > is interpreted as a comment and make can't find the end of
> > $(foreach,).
> 
> In cmd_xlat_lst you use $(pound) - any reason this doesn't work in
> these rules? Note that I don't mind the use of "define", just that I'm
> puzzled by the justification.

I think I just forgot about $(pound) when I tried to debug why the
command didn't work in some environment (that is when not using define).

I think the second thing that make me not replacing '#' by "$(pound)" is
that reading "#include" in the Makefile is probably better that
reading "$(pound)include".

I guess we should add something to the justification like:
    That allow us to keep writing "#include" in the Makefile without
    having to replace that by "$(pound)include" which would be a bit
    less obvious about the command line purpose.

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Jan Beulich]

On 06.06.2022 15:39, Anthony PERARD wrote:
> On Thu, Jun 02, 2022 at 11:11:15AM +0200, Jan Beulich wrote:
>> On 01.06.2022 18:59, Anthony PERARD wrote:
>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>> is interpreted as a comment and make can't find the end of
>>> $(foreach,).
>>
>> In cmd_xlat_lst you use $(pound) - any reason this doesn't work in
>> these rules? Note that I don't mind the use of "define", just that I'm
>> puzzled by the justification.
> 
> I think I just forgot about $(pound) when I tried to debug why the
> command didn't work in some environment (that is when not using define).
> 
> I think the second thing that make me not replacing '#' by "$(pound)" is
> that reading "#include" in the Makefile is probably better that
> reading "$(pound)include".
> 
> I guess we should add something to the justification like:
>     That allow us to keep writing "#include" in the Makefile without
>     having to replace that by "$(pound)include" which would be a bit
>     less obvious about the command line purpose.

I'd be okay with this, and I'd also be okay with adding this while
committing. With it added
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Bertrand Marquis]

Hi Anthony,

> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> Use "define" for the headers*_chk commands as otherwise the "#"
> is interpreted as a comment and make can't find the end of
> $(foreach,).
> 
> Adding several .PRECIOUS as without them `make` deletes the
> intermediate targets. This is an issue because the macro $(if_changed,)
> check if the target exist in order to decide whether to recreate the
> target.
> 
> Removing the call to `mkdir` from the commands. Those aren't needed
> anymore because a rune in Rules.mk creates the directory for each
> $(targets).
> 
> Remove "export PYTHON" as it is already exported.

With this change, compiling for x86 is now ending up in:
CHK     include/headers99.chk
make[9]: execvp: /bin/sh: Argument list too long
make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127

Not quite sure yet why but I wanted to signal it early as other might be impacted.

Arm and arm64 builds are not impacted.

Cheers
Bertrand

> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> xen/include/Makefile | 108 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 76 insertions(+), 32 deletions(-)
> 
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index 03baf10efb..6d9bcc19b0 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -45,38 +45,65 @@ public-$(CONFIG_ARM) := $(wildcard $(srcdir)/public/arch-arm/*.h $(srcdir)/publi
> .PHONY: all
> all: $(addprefix $(obj)/,$(headers-y))
> 
> -$(obj)/compat/%.h: $(obj)/compat/%.i $(srcdir)/Makefile $(srctree)/tools/compat-build-header.py
> -	$(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
> -	mv -f $@.new $@
> -
> -$(obj)/compat/%.i: $(obj)/compat/%.c $(srcdir)/Makefile
> -	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
> -
> -$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srcdir)/Makefile $(srctree)/tools/compat-build-source.py
> -	mkdir -p $(@D)
> -	$(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new
> -	mv -f $@.new $@
> -
> -$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh $(srcdir)/Makefile
> -	export PYTHON=$(PYTHON); \
> -	while read what name; do \
> -		$(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
> -	done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new
> -	mv -f $@.new $@
> +quiet_cmd_compat_h = GEN     $@
> +cmd_compat_h = \
> +    $(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
> +    mv -f $@.new $@
> +
> +quiet_cmd_compat_i = CPP     $@
> +cmd_compat_i = $(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
> +
> +quiet_cmd_compat_c = GEN     $@
> +cmd_compat_c = \
> +   $(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new; \
> +   mv -f $@.new $@
> +
> +quiet_cmd_xlat_headers = GEN     $@
> +cmd_xlat_headers = \
> +    while read what name; do \
> +        $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
> +    done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new; \
> +    mv -f $@.new $@
> +
> +targets += $(headers-y)
> +$(obj)/compat/%.h: $(obj)/compat/%.i $(srctree)/tools/compat-build-header.py FORCE
> +	$(call if_changed,compat_h)
> +
> +.PRECIOUS: $(obj)/compat/%.i
> +targets += $(patsubst %.h, %.i, $(headers-y))
> +$(obj)/compat/%.i: $(obj)/compat/%.c FORCE
> +	$(call if_changed,compat_i)
> +
> +.PRECIOUS: $(obj)/compat/%.c
> +targets += $(patsubst %.h, %.c, $(headers-y))
> +$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srctree)/tools/compat-build-source.py FORCE
> +	$(call if_changed,compat_c)
> +
> +targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y))
> +$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh FORCE
> +	$(call if_changed,xlat_headers)
> +
> +quiet_cmd_xlat_lst = GEN     $@
> +cmd_xlat_lst = \
> +	grep -v '^[[:blank:]]*$(pound)' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new; \
> +	$(call move-if-changed,$@.new,$@)
> 
> .PRECIOUS: $(obj)/compat/.xlat/%.lst
> -$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst $(srcdir)/Makefile
> -	mkdir -p $(@D)
> -	grep -v '^[[:blank:]]*#' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new
> -	$(call move-if-changed,$@.new,$@)
> +targets += $(patsubst compat/%.h, compat/.xlat/%.lst, $(headers-y))
> +$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
> +	$(call if_changed,xlat_lst)
> 
> xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
> xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
> 
> -$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf $(srcdir)/Makefile
> -	cat $(filter %.h,$^) >$@.new
> +quiet_cmd_xlat_h = GEN     $@
> +cmd_xlat_h = \
> +	cat $(filter %.h,$^) >$@.new; \
> 	mv -f $@.new $@
> 
> +$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf FORCE
> +	$(call if_changed,xlat_h)
> +
> ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
> 
> all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
> @@ -102,27 +129,31 @@ PUBLIC_C99_HEADERS := $(call public-filter-headers,public-c99-headers)
> $(src)/public/io/9pfs.h-prereq := string
> $(src)/public/io/pvcalls.h-prereq := string
> 
> -$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) $(srcdir)/Makefile
> +quiet_cmd_header_chk = CHK     $@
> +cmd_header_chk = \
> 	for i in $(filter %.h,$^); do \
> 	    $(CC) -x c -ansi -Wall -Werror -include stdint.h \
> 	          -S -o /dev/null $$i || exit 1; \
> 	    echo $$i; \
> -	done >$@.new
> +	done >$@.new; \
> 	mv $@.new $@
> 
> -$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) $(srcdir)/Makefile
> -	rm -f $@.new
> +quiet_cmd_headers99_chk = CHK     $@
> +define cmd_headers99_chk
> +	rm -f $@.new; \
> 	$(foreach i, $(filter %.h,$^),                                        \
> 	    echo "#include "\"$(i)\"                                          \
> 	    | $(CC) -x c -std=c99 -Wall -Werror                               \
> 	      -include stdint.h                                               \
> 	      $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include $(j).h) \
> 	      -S -o /dev/null -                                               \
> -	    || exit $$?; echo $(i) >> $@.new;)
> +	    || exit $$?; echo $(i) >> $@.new;) \
> 	mv $@.new $@
> +endef
> 
> -$(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
> -	rm -f $@.new
> +quiet_cmd_headerscxx_chk = CHK     $@
> +define cmd_headerscxx_chk
> +	rm -f $@.new; \
> 	if ! $(CXX) -v >/dev/null 2>&1; then                                  \
> 	    touch $@.new;                                                     \
> 	    exit 0;                                                           \
> @@ -133,8 +164,21 @@ $(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
> 	      -include stdint.h -include $(srcdir)/public/xen.h               \
> 	      $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \
> 	      -S -o /dev/null -                                               \
> -	    || exit $$?; echo $(i) >> $@.new;)
> +	    || exit $$?; echo $(i) >> $@.new;) \
> 	mv $@.new $@
> +endef
> +
> +targets += headers.chk
> +$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) FORCE
> +	$(call if_changed,header_chk)
> +
> +targets += headers99.chk
> +$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) FORCE
> +	$(call if_changed,headers99_chk)
> +
> +targets += headers++.chk
> +$(obj)/headers++.chk: $(PUBLIC_HEADERS) FORCE
> +	$(call if_changed,headerscxx_chk)
> 
> endif
> 
> -- 
> Anthony PERARD
> 
> 

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Jan Beulich]

On 09.06.2022 12:16, Bertrand Marquis wrote:
>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>
>> Use "define" for the headers*_chk commands as otherwise the "#"
>> is interpreted as a comment and make can't find the end of
>> $(foreach,).
>>
>> Adding several .PRECIOUS as without them `make` deletes the
>> intermediate targets. This is an issue because the macro $(if_changed,)
>> check if the target exist in order to decide whether to recreate the
>> target.
>>
>> Removing the call to `mkdir` from the commands. Those aren't needed
>> anymore because a rune in Rules.mk creates the directory for each
>> $(targets).
>>
>> Remove "export PYTHON" as it is already exported.
> 
> With this change, compiling for x86 is now ending up in:
> CHK     include/headers99.chk
> make[9]: execvp: /bin/sh: Argument list too long
> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
> 
> Not quite sure yet why but I wanted to signal it early as other might be impacted.
> 
> Arm and arm64 builds are not impacted.

Hmm, that patch has passed the smoke push gate already, so there likely is
more to it than there being an unconditional issue. I did build-test this
before pushing, and I've just re-tested on a 2nd system without seeing an
issue.

Also please remember to trim your replies.

Jan

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Bertrand Marquis]

Hi,

> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>> 
>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>> is interpreted as a comment and make can't find the end of
>>> $(foreach,).
>>> 
>>> Adding several .PRECIOUS as without them `make` deletes the
>>> intermediate targets. This is an issue because the macro $(if_changed,)
>>> check if the target exist in order to decide whether to recreate the
>>> target.
>>> 
>>> Removing the call to `mkdir` from the commands. Those aren't needed
>>> anymore because a rune in Rules.mk creates the directory for each
>>> $(targets).
>>> 
>>> Remove "export PYTHON" as it is already exported.
>> 
>> With this change, compiling for x86 is now ending up in:
>> CHK     include/headers99.chk
>> make[9]: execvp: /bin/sh: Argument list too long
>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>> 
>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>> 
>> Arm and arm64 builds are not impacted.
> 
> Hmm, that patch has passed the smoke push gate already, so there likely is
> more to it than there being an unconditional issue. I did build-test this
> before pushing, and I've just re-tested on a 2nd system without seeing an
> issue.

I have the problem only when building using Yocto, I did a normal build and the
issue is not coming.

Doing a verbose compilation I have this (sorry for the long lines):

 for i in include/public/vcpu.h include/public/errno.h include/public/kexec.h include/public/argo.h include/public/xen.h include/public/nmi.h include/public/xencomm.h include/public/xenoprof.h include/public/device_tree_defs.h include/public/version.h include/public/memory.h include/public/features.h include/public/sched.h include/public/xen-compat.h include/public/callback.h include/public/vm_event.h include/public/grant_table.h include/public/physdev.h include/public/tmem.h include/public/hypfs.h include/public/platform.h include/public/pmu.h include/public/elfnote.h include/public/trace.h include/public/event_channel.h include/public/io/vscsiif.h include/public/io/kbdif.h include/public/io/protocols.h include/public/io/ring.h include/public/io/displif.h include/public/io/fsif.h include/public/io/blkif.h include/public/io/console.h include/public/io/sndif.h include/public/io/fbif.h include/public/io/libxenvchan.h include/public/io/netif.h include/public/io/usbif.h include/public/io/pciif.h include/public/io/tpmif.h include/public/io/xs_wire.h include/public/io/xenbus.h include/public/io/cameraif.h include/public/hvm/pvdrivers.h include/public/hvm/e820.h include/public/hvm/hvm_xs_strings.h include/public/hvm/dm_op.h include/public/hvm/ioreq.h include/public/hvm/hvm_info_table.h include/public/hvm/hvm_vcpu.h include/public/hvm/hvm_op.h include/public/hvm/params.h; do x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -ansi -Wall -Werror -include stdint.h -S -o /dev/null $i || exit 1; echo $i; done >include/headers.chk.new; mv include/headers.chk.new include/headers.chk
|       rm -f include/headers99.chk.new;  echo "#include "\"include/public/io/9pfs.h\" | x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -std=c99 -Wall -Werror -include stdint.h  -include string.h -S -o /dev/null - || exit $?; echo include/public/io/9pfs.h >> include/headers99.chk.new;  echo "#include "\"include/public/io/pvcalls.h\" | x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -std=c99 -Wall -Werror -include stdint.h  -include string.h -S -o /dev/null - || exit $?; echo include/public/io/pvcalls.h >> include/headers99.chk.new; mv include/headers99.chk.new include/headers99.chk
| make[9]: execvp: /bin/sh: Argument list too long
| make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
| make[9]: *** Waiting for unfinished jobs....

So the command passed to the sub shell by make is quite long.

No idea why this comes out only when building in Yocto but I will dig a bit.

> 
> Also please remember to trim your replies.
> 

Will do.

Bertrand

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Jan Beulich]

On 09.06.2022 13:51, Bertrand Marquis wrote:
>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>>>
>>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>>> is interpreted as a comment and make can't find the end of
>>>> $(foreach,).
>>>>
>>>> Adding several .PRECIOUS as without them `make` deletes the
>>>> intermediate targets. This is an issue because the macro $(if_changed,)
>>>> check if the target exist in order to decide whether to recreate the
>>>> target.
>>>>
>>>> Removing the call to `mkdir` from the commands. Those aren't needed
>>>> anymore because a rune in Rules.mk creates the directory for each
>>>> $(targets).
>>>>
>>>> Remove "export PYTHON" as it is already exported.
>>>
>>> With this change, compiling for x86 is now ending up in:
>>> CHK     include/headers99.chk
>>> make[9]: execvp: /bin/sh: Argument list too long
>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>
>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>
>>> Arm and arm64 builds are not impacted.
>>
>> Hmm, that patch has passed the smoke push gate already, so there likely is
>> more to it than there being an unconditional issue. I did build-test this
>> before pushing, and I've just re-tested on a 2nd system without seeing an
>> issue.
> 
> I have the problem only when building using Yocto, I did a normal build and the
> issue is not coming.
> 
> Doing a verbose compilation I have this (sorry for the long lines):
> 
>  for i in include/public/vcpu.h include/public/errno.h include/public/kexec.h include/public/argo.h include/public/xen.h include/public/nmi.h include/public/xencomm.h include/public/xenoprof.h include/public/device_tree_defs.h include/public/version.h include/public/memory.h include/public/features.h include/public/sched.h include/public/xen-compat.h include/public/callback.h include/public/vm_event.h include/public/grant_table.h include/public/physdev.h include/public/tmem.h include/public/hypfs.h include/public/platform.h include/public/pmu.h include/public/elfnote.h include/public/trace.h include/public/event_channel.h include/public/io/vscsiif.h include/public/io/kbdif.h include/public/io/protocols.h include/public/io/ring.h include/public/io/displif.h include/public/io/fsif.h include/public/io/blkif.h include/public/io/console.h include/public/io/sndif.h include/public/io/fbif.h include/public/io/libxenvchan.h include/public/io/netif.h include/public/io/usbif.h include/public/io/pciif.h include/public/io/tpmif.h include/public/io/xs_wire.h include/public/io/xenbus.h include/public/io/cameraif.h include/public/hvm/pvdrivers.h include/public/hvm/e820.h include/public/hvm/hvm_xs_strings.h include/public/hvm/dm_op.h include/public/hvm/ioreq.h include/public/hvm/hvm_info_table.h include/public/hvm/hvm_vcpu.h include/public/hvm/hvm_op.h include/public/hvm/params.h; do x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -ansi -Wall -Werror -include stdint.h -S -o /dev/null $i || exit 1; echo $i; done >include/headers.chk.new; mv include/headers.chk.new include/headers.chk
> |       rm -f include/headers99.chk.new;  echo "#include "\"include/public/io/9pfs.h\" | x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -std=c99 -Wall -Werror -include stdint.h  -include string.h -S -o /dev/null - || exit $?; echo include/public/io/9pfs.h >> include/headers99.chk.new;  echo "#include "\"include/public/io/pvcalls.h\" | x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -std=c99 -Wall -Werror -include stdint.h  -include string.h -S -o /dev/null - || exit $?; echo include/public/io/pvcalls.h >> include/headers99.chk.new; mv include/headers99.chk.new include/headers99.chk
> | make[9]: execvp: /bin/sh: Argument list too long
> | make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
> | make[9]: *** Waiting for unfinished jobs....
> 
> So the command passed to the sub shell by make is quite long.
> 
> No idea why this comes out only when building in Yocto but I will dig a bit.

Maybe Yocto has an unusually low limit on command arguments' total size?
The whole thing is just over 2500 chars, which doesn't look to be unusually
long for Unix-like environments.

Jan

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Bertrand Marquis]

Hi,

> On 9 Jun 2022, at 13:16, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 09.06.2022 13:51, Bertrand Marquis wrote:
>>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>>>> 
>>>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>>>> is interpreted as a comment and make can't find the end of
>>>>> $(foreach,).
>>>>> 
>>>>> Adding several .PRECIOUS as without them `make` deletes the
>>>>> intermediate targets. This is an issue because the macro $(if_changed,)
>>>>> check if the target exist in order to decide whether to recreate the
>>>>> target.
>>>>> 
>>>>> Removing the call to `mkdir` from the commands. Those aren't needed
>>>>> anymore because a rune in Rules.mk creates the directory for each
>>>>> $(targets).
>>>>> 
>>>>> Remove "export PYTHON" as it is already exported.
>>>> 
>>>> With this change, compiling for x86 is now ending up in:
>>>> CHK     include/headers99.chk
>>>> make[9]: execvp: /bin/sh: Argument list too long
>>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>> 
>>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>> 
>>>> Arm and arm64 builds are not impacted.
>>> 
>>> Hmm, that patch has passed the smoke push gate already, so there likely is
>>> more to it than there being an unconditional issue. I did build-test this
>>> before pushing, and I've just re-tested on a 2nd system without seeing an
>>> issue.
>> 
>> I have the problem only when building using Yocto, I did a normal build and the
>> issue is not coming.
>> 
>> Doing a verbose compilation I have this (sorry for the long lines):
>> 
>> for i in include/public/vcpu.h include/public/errno.h include/public/kexec.h include/public/argo.h include/public/xen.h include/public/nmi.h include/public/xencomm.h include/public/xenoprof.h include/public/device_tree_defs.h include/public/version.h include/public/memory.h include/public/features.h include/public/sched.h include/public/xen-compat.h include/public/callback.h include/public/vm_event.h include/public/grant_table.h include/public/physdev.h include/public/tmem.h include/public/hypfs.h include/public/platform.h include/public/pmu.h include/public/elfnote.h include/public/trace.h include/public/event_channel.h include/public/io/vscsiif.h include/public/io/kbdif.h include/public/io/protocols.h include/public/io/ring.h include/public/io/displif.h include/public/io/fsif.h include/public/io/blkif.h include/public/io/console.h include/public/io/sndif.h include/public/io/fbif.h include/public/io/libxenvchan.h include/public/io/netif.h include/public/io/usbif.h include/public/io/pciif.h include/public/io/tpmif.h include/public/io/xs_wire.h include/public/io/xenbus.h include/public/io/cameraif.h include/public/hvm/pvdrivers.h include/public/hvm/e820.h include/public/hvm/hvm_xs_strings.h include/public/hvm/dm_op.h include/public/hvm/ioreq.h include/public/hvm/hvm_info_table.h include/public/hvm/hvm_vcpu.h include/public/hvm/hvm_op.h include/public/hvm/params.h; do x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -ansi -Wall -Werror -include stdint.h -S -o /dev/null $i || exit 1; echo $i; done >include/headers.chk.new; mv include/headers.chk.new include/headers.chk
>> |       rm -f include/headers99.chk.new;  echo "#include "\"include/public/io/9pfs.h\" | x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -std=c99 -Wall -Werror -include stdint.h  -include string.h -S -o /dev/null - || exit $?; echo include/public/io/9pfs.h >> include/headers99.chk.new;  echo "#include "\"include/public/io/pvcalls.h\" | x86_64-poky-linux-gcc  --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot  -x c -std=c99 -Wall -Werror -include stdint.h  -include string.h -S -o /dev/null - || exit $?; echo include/public/io/pvcalls.h >> include/headers99.chk.new; mv include/headers99.chk.new include/headers99.chk
>> | make[9]: execvp: /bin/sh: Argument list too long
>> | make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>> | make[9]: *** Waiting for unfinished jobs....
>> 
>> So the command passed to the sub shell by make is quite long.
>> 
>> No idea why this comes out only when building in Yocto but I will dig a bit.
> 
> Maybe Yocto has an unusually low limit on command arguments' total size?
> The whole thing is just over 2500 chars, which doesn't look to be unusually
> long for Unix-like environments.
> 

Actually the command to generate headers++.chk is 15294 characters when I build xen normally.
In Yocto it becomes a lot bigger as CC includes a sysroot parameter with a path.

Bertrand

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Anthony PERARD]

On Thu, Jun 09, 2022 at 11:51:20AM +0000, Bertrand Marquis wrote:
> Hi,
> 
> > On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
> > 
> > On 09.06.2022 12:16, Bertrand Marquis wrote:
> >> With this change, compiling for x86 is now ending up in:
> >> CHK     include/headers99.chk
> >> make[9]: execvp: /bin/sh: Argument list too long
> >> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
> >> 
> >> Not quite sure yet why but I wanted to signal it early as other might be impacted.
> >> 
> >> Arm and arm64 builds are not impacted.
> > 
> > Hmm, that patch has passed the smoke push gate already, so there likely is
> > more to it than there being an unconditional issue. I did build-test this
> > before pushing, and I've just re-tested on a 2nd system without seeing an
> > issue.
> 
> I have the problem only when building using Yocto, I did a normal build and the
> issue is not coming.
> 

Will the following patch help?


From 0f32f749304b233c0d5574dc6b14f66e8709feba Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Thu, 9 Jun 2022 13:42:52 +0100
Subject: [XEN PATCH] build,include: rework shell script for headers++.chk

The command line generated for headers++.chk by make is quite long,
and in some environment it is too long. This issue have been seen in
Yocto build environment.

Error messages:
    make[9]: execvp: /bin/sh: Argument list too long
    make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127

Rework to that we do the foreach loop in shell rather that make to
reduce the command line size by a lot. We also need a way to get
headers prerequisite for some public headers so we use a switch "case"
in shell to be able to do some simple pattern matching. Variables
alone in POSIX shell don't allow to work with associative array or
variables with "/".

Reported-by: Bertrand Marquis <Bertrand.Marquis@arm.com>
Fixes: 28e13c7f43 ("build: xen/include: use if_changed")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/include/Makefile | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 6d9bcc19b0..ca5e868f38 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -158,13 +158,22 @@ define cmd_headerscxx_chk
 	    touch $@.new;                                                     \
 	    exit 0;                                                           \
 	fi;                                                                   \
-	$(foreach i, $(filter %.h,$^),                                        \
-	    echo "#include "\"$(i)\"                                          \
+	get_prereq() {                                                        \
+	    case $$1 in                                                       \
+	    $(foreach i, $(filter %.h,$^),                                    \
+	    $(if $($(patsubst $(srctree)/%,%,$i)-prereq),                     \
+		$(patsubst $(srctree)/%,%,$i)$(close)                         \
+		echo "$(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq),   \
+			-include c$(j))";;))                                  \
+	    esac;                                                             \
+	};                                                                    \
+	for i in $(filter %.h,$^); do                                         \
+	    echo "#include "\"$$i\"                                           \
 	    | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__        \
 	      -include stdint.h -include $(srcdir)/public/xen.h               \
-	      $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \
+	      `get_prereq $$i`                                                \
 	      -S -o /dev/null -                                               \
-	    || exit $$?; echo $(i) >> $@.new;) \
+	    || exit $$?; echo $$i >> $@.new; done;                            \
 	mv $@.new $@
 endef
 



-- 
Anthony PERARD

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Bertrand Marquis]

Hi Antony,

> On 9 Jun 2022, at 13:55, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> On Thu, Jun 09, 2022 at 11:51:20AM +0000, Bertrand Marquis wrote:
>> Hi,
>> 
>>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>> With this change, compiling for x86 is now ending up in:
>>>> CHK     include/headers99.chk
>>>> make[9]: execvp: /bin/sh: Argument list too long
>>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>> 
>>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>> 
>>>> Arm and arm64 builds are not impacted.
>>> 
>>> Hmm, that patch has passed the smoke push gate already, so there likely is
>>> more to it than there being an unconditional issue. I did build-test this
>>> before pushing, and I've just re-tested on a 2nd system without seeing an
>>> issue.
>> 
>> I have the problem only when building using Yocto, I did a normal build and the
>> issue is not coming.
>> 
> 
> Will the following patch help?

Yes it does, thanks a lot.

You can add my:
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> 
> From 0f32f749304b233c0d5574dc6b14f66e8709feba Mon Sep 17 00:00:00 2001
> From: Anthony PERARD <anthony.perard@citrix.com>
> Date: Thu, 9 Jun 2022 13:42:52 +0100
> Subject: [XEN PATCH] build,include: rework shell script for headers++.chk
> 
> The command line generated for headers++.chk by make is quite long,
> and in some environment it is too long. This issue have been seen in
> Yocto build environment.
> 
> Error messages:
>    make[9]: execvp: /bin/sh: Argument list too long
>    make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
> 
> Rework to that we do the foreach loop in shell rather that make to
> reduce the command line size by a lot. We also need a way to get
> headers prerequisite for some public headers so we use a switch "case"
> in shell to be able to do some simple pattern matching. Variables
> alone in POSIX shell don't allow to work with associative array or
> variables with "/".
> 
> Reported-by: Bertrand Marquis <Bertrand.Marquis@arm.com>
> Fixes: 28e13c7f43 ("build: xen/include: use if_changed")
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> xen/include/Makefile | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index 6d9bcc19b0..ca5e868f38 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -158,13 +158,22 @@ define cmd_headerscxx_chk
> 	    touch $@.new;                                                     \
> 	    exit 0;                                                           \
> 	fi;                                                                   \
> -	$(foreach i, $(filter %.h,$^),                                        \
> -	    echo "#include "\"$(i)\"                                          \
> +	get_prereq() {                                                        \
> +	    case $$1 in                                                       \
> +	    $(foreach i, $(filter %.h,$^),                                    \
> +	    $(if $($(patsubst $(srctree)/%,%,$i)-prereq),                     \
> +		$(patsubst $(srctree)/%,%,$i)$(close)                         \
> +		echo "$(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq),   \
> +			-include c$(j))";;))                                  \
> +	    esac;                                                             \
> +	};                                                                    \
> +	for i in $(filter %.h,$^); do                                         \
> +	    echo "#include "\"$$i\"                                           \
> 	    | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__        \
> 	      -include stdint.h -include $(srcdir)/public/xen.h               \
> -	      $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \
> +	      `get_prereq $$i`                                                \
> 	      -S -o /dev/null -                                               \
> -	    || exit $$?; echo $(i) >> $@.new;) \
> +	    || exit $$?; echo $$i >> $@.new; done;                            \
> 	mv $@.new $@
> endef
> 
> 
> 
> 
> -- 
> Anthony PERARD

Re: [XEN PATCH 1/4] build: xen/include: use if_changed

[Bertrand Marquis]

Hi Anthony,

> On 9 Jun 2022, at 13:55, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> On Thu, Jun 09, 2022 at 11:51:20AM +0000, Bertrand Marquis wrote:
>> Hi,
>> 
>>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>> With this change, compiling for x86 is now ending up in:
>>>> CHK     include/headers99.chk
>>>> make[9]: execvp: /bin/sh: Argument list too long
>>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>> 
>>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>> 
>>>> Arm and arm64 builds are not impacted.
>>> 
>>> Hmm, that patch has passed the smoke push gate already, so there likely is
>>> more to it than there being an unconditional issue. I did build-test this
>>> before pushing, and I've just re-tested on a 2nd system without seeing an
>>> issue.
>> 
>> I have the problem only when building using Yocto, I did a normal build and the
>> issue is not coming.
>> 
> 
> Will the following patch help?
> 
> 
> From 0f32f749304b233c0d5574dc6b14f66e8709feba Mon Sep 17 00:00:00 2001
> From: Anthony PERARD <anthony.perard@citrix.com>
> Date: Thu, 9 Jun 2022 13:42:52 +0100
> Subject: [XEN PATCH] build,include: rework shell script for headers++.chk
> 
> The command line generated for headers++.chk by make is quite long,
> and in some environment it is too long. This issue have been seen in
> Yocto build environment.
> 
> Error messages:
>    make[9]: execvp: /bin/sh: Argument list too long
>    make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
> 
> Rework to that we do the foreach loop in shell rather that make to
> reduce the command line size by a lot. We also need a way to get
> headers prerequisite for some public headers so we use a switch "case"
> in shell to be able to do some simple pattern matching. Variables
> alone in POSIX shell don't allow to work with associative array or
> variables with "/".
> 
> Reported-by: Bertrand Marquis <Bertrand.Marquis@arm.com>
> Fixes: 28e13c7f43 ("build: xen/include: use if_changed")
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> xen/include/Makefile | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index 6d9bcc19b0..ca5e868f38 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -158,13 +158,22 @@ define cmd_headerscxx_chk
> 	    touch $@.new;                                                     \
> 	    exit 0;                                                           \
> 	fi;                                                                   \
> -	$(foreach i, $(filter %.h,$^),                                        \
> -	    echo "#include "\"$(i)\"                                          \
> +	get_prereq() {                                                        \
> +	    case $$1 in                                                       \
> +	    $(foreach i, $(filter %.h,$^),                                    \
> +	    $(if $($(patsubst $(srctree)/%,%,$i)-prereq),                     \
> +		$(patsubst $(srctree)/%,%,$i)$(close)                         \
> +		echo "$(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq),   \
> +			-include c$(j))";;))                                  \
> +	    esac;                                                             \
> +	};                                                                    \
> +	for i in $(filter %.h,$^); do                                         \
> +	    echo "#include "\"$$i\"                                           \
> 	    | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__        \
> 	      -include stdint.h -include $(srcdir)/public/xen.h               \
> -	      $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \
> +	      `get_prereq $$i`                                                \
> 	      -S -o /dev/null -                                               \
> -	    || exit $$?; echo $(i) >> $@.new;) \
> +	    || exit $$?; echo $$i >> $@.new; done;                            \
> 	mv $@.new $@
> endef
> 

Just a small reminder that you need to push this patch officially :-)

Cheers
Bertrand

[XEN PATCH 2/4] build: set PERL

[Anthony PERARD]

We are going to use it in a moment.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/Makefile b/xen/Makefile
index 82f5310b12..a6650a2acc 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -22,6 +22,7 @@ PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null)
 export PYTHON		?= $(PYTHON_INTERPRETER)
 
 export CHECKPOLICY	?= checkpolicy
+export PERL		?= perl
 
 $(if $(filter __%, $(MAKECMDGOALS)), \
     $(error targets prefixed with '__' are only for internal use))
-- 
Anthony PERARD

Re: [XEN PATCH 2/4] build: set PERL

[Jan Beulich]

On 01.06.2022 18:59, Anthony PERARD wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -22,6 +22,7 @@ PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null)
>  export PYTHON		?= $(PYTHON_INTERPRETER)
>  
>  export CHECKPOLICY	?= checkpolicy
> +export PERL		?= perl

For the intended use, is there a minimum version requirement? If so,
it needs documenting in ./README (and it preferably wouldn't be any
newer than from around the times our other dependencies are). And
even when the uses are fully backwards compatible, I think the need
for the tool wants mentioning there.

Jan

Re: [XEN PATCH 2/4] build: set PERL

[Anthony PERARD]

On Thu, Jun 02, 2022 at 11:01:30AM +0200, Jan Beulich wrote:
> On 01.06.2022 18:59, Anthony PERARD wrote:
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -22,6 +22,7 @@ PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null)
> >  export PYTHON		?= $(PYTHON_INTERPRETER)
> >  
> >  export CHECKPOLICY	?= checkpolicy
> > +export PERL		?= perl
> 
> For the intended use, is there a minimum version requirement? If so,
> it needs documenting in ./README (and it preferably wouldn't be any
> newer than from around the times our other dependencies are). And
> even when the uses are fully backwards compatible, I think the need
> for the tool wants mentioning there.

I don't think there's a minimum version. The script works in our
Gitlab CI, or at least the builds don't break.

Yes, it would be better to document the tool, I'll add it to the README.
(We already use it in the toolstack, at least for libxl, so it was at
least partially needed before.)

Thanks,

-- 
Anthony PERARD

[XEN PATCH 3/4] build: replace get-fields.sh by a perl script

[Anthony PERARD]

The get-fields.sh which generate all the include/compat/.xlat/*.h
headers is quite slow. It takes for example nearly 3 seconds to
generate platform.h on a recent machine, or 2.3 seconds for memory.h.

Since it's only text processing, rewriting the mix of shell/sed/python
into a single perl script make the generation of those file a lot
faster.

I tried to keep a similar look for the code, to keep the code similar
between the shell and perl, and to ease review. So some code in perl
might look weird or could be written better.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/include/Makefile         |   6 +-
 xen/tools/compat-xlat-header | 539 +++++++++++++++++++++++++++++++++++
 xen/tools/get-fields.sh      | 528 ----------------------------------
 3 files changed, 541 insertions(+), 532 deletions(-)
 create mode 100755 xen/tools/compat-xlat-header
 delete mode 100644 xen/tools/get-fields.sh

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 6d9bcc19b0..b7e7148665 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -60,9 +60,7 @@ cmd_compat_c = \
 
 quiet_cmd_xlat_headers = GEN     $@
 cmd_xlat_headers = \
-    while read what name; do \
-        $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
-    done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new; \
+    $(PERL) $(srctree)/tools/compat-xlat-header $< $(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst > $@.new; \
     mv -f $@.new $@
 
 targets += $(headers-y)
@@ -80,7 +78,7 @@ $(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srctree)/tools/compat-
 	$(call if_changed,compat_c)
 
 targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y))
-$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh FORCE
+$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/compat-xlat-header FORCE
 	$(call if_changed,xlat_headers)
 
 quiet_cmd_xlat_lst = GEN     $@
diff --git a/xen/tools/compat-xlat-header b/xen/tools/compat-xlat-header
new file mode 100755
index 0000000000..f1f42a9dde
--- /dev/null
+++ b/xen/tools/compat-xlat-header
@@ -0,0 +1,539 @@
+#!/usr/bin/perl -w
+
+use strict;
+use warnings;
+
+open COMPAT_LIST, "<$ARGV[1]" or die "can't open $ARGV[1], $!";
+open HEADER, "<$ARGV[0]" or die "can't open $ARGV[0], $!";
+
+my @typedefs;
+
+my @header_tokens;
+while (<HEADER>) {
+    next if m/^\s*#.*/;
+    s/([\]\[,;:{}])/ $1 /g;
+    s/^\s+//;
+    push(@header_tokens, split(/\s+/));
+}
+
+sub get_fields {
+    my ($looking_for) = @_;
+    my $level = 1;
+    my $aggr = 0;
+    my ($name, @fields);
+
+    foreach (@header_tokens) {
+        if (/^(struct|union)$/) {
+            unless ($level != 1) {
+                $aggr = 1;
+                @fields = ();
+                $name = '';
+            }
+        } elsif ($_ eq '{') {
+            $level++;
+        } elsif ($_ eq '}') {
+            $level--;
+            if ($level == 1 and $name eq $looking_for) {
+                push (@fields, $_);
+                return @fields;
+            }
+        } elsif (/^[a-zA-Z_].*/) {
+            unless ($aggr == 0 or $name ne "") {
+                $name = $_;
+            }
+        }
+        unless ($aggr == 0) {
+            push (@fields, $_);
+        }
+    }
+    return ();
+}
+
+sub get_typedefs {
+    my $level = 1;
+    my $state = 0;
+    my @typedefs;
+    foreach (@_) {
+        if ($_ eq 'typedef') {
+            unless ($level != 1) {
+                $state = 1;
+            }
+        } elsif (m/^COMPAT_HANDLE\(.*\)$/) {
+            unless ($level != 1 or $state != 1) {
+                $state = 2;
+            }
+        } elsif (m/^[\[\{]$/) {
+            $level++;
+        } elsif (m/^[\]\}]$/) {
+            $level--;
+        } elsif ($_ eq ';') {
+            unless  ($level != 1) {
+                $state = 0;
+            }
+        } elsif (m/^[a-zA-Z_].*$/) {
+            unless ($level != 1 or $state != 2) {
+                push (@typedefs, $_);
+            }
+        }
+    }
+    return @typedefs;
+}
+
+sub build_enums {
+    my ($name, @tokens) = @_;
+
+    my $level = 1;
+    my $kind = '';
+    my $named = '';
+    my (@fields, @members, $id);
+
+    foreach (@tokens) {
+        if (m/^(struct|union)$/) {
+            unless ($level != 2) {
+                @fields = ('');
+            }
+            $kind="$_;$kind";
+        } elsif ($_ eq '{') {
+            $level++;
+        } elsif ($_ eq '}') {
+            $level--;
+            if ($level == 1) {
+                my $subkind = $kind;
+                $subkind =~ s/;.*//;
+                if ($subkind eq 'union') {
+                    print "\nenum XLAT_$name {\n";
+                    foreach (@members) {
+                        print "    XLAT_${name}_$_,\n";
+                    }
+                    print "};\n";
+                }
+                return;
+            } elsif ($level == 2) {
+                $named = '?';
+            }
+        } elsif (/^[a-zA-Z_].*$/) {
+            $id = $_;
+            my $k = $kind;
+            $k =~ s/.*?;//;
+            if ($named ne '' and $k ne '') {
+                shift @fields if @fields > 0 and $fields[0] eq '';
+                build_enums("${name}_$_", @fields);
+                $named = '!';
+            }
+        } elsif ($_ eq ',') {
+            unless ($level != 2) {
+                push (@members, $id);
+            }
+        } elsif ($_ eq ';') {
+            unless ($level != 2) {
+                push (@members, $id);
+            }
+            unless ($named eq '') {
+                $kind =~ s/.*?;//;
+            }
+            $named = '';
+        }
+        unless (@fields == 0) {
+            push (@fields, $_);
+        }
+    }
+}
+
+sub handle_field {
+    my ($prefix, $name, $id, $type, @fields) = @_;
+
+    if (@fields == 0) {
+        print " \\\n";
+        if ($type eq '') {
+            print "$prefix\(_d_)->$id = (_s_)->$id;"
+        } else {
+            my $k = $id;
+            $k =~ s/\./_/g;
+            print "${prefix}XLAT_${name}_HNDL_${k}(_d_, _s_);";
+        }
+    } elsif ("@fields" !~ m/[{}]/) {
+        my $tag = "@fields";
+        $tag =~ s/\s*(struct|union)\s+(compat_)?(\w+)\s.*/$3/;
+        print " \\\n";
+        print "${prefix}XLAT_$tag(&(_d_)->$id, &(_s_)->$id);"
+    } else {
+        my $func_id = $id;
+        my @func_tokens = @fields;
+        my $kind = '';
+        my $array = "";
+        my $level = 1;
+        my $arrlvl = 1;
+        my $array_type = '';
+        my $id = '';
+        my $type = '';
+        @fields = ();
+        foreach (@func_tokens) {
+            if (/^(struct|union)$/) {
+                unless ($level != 2) {
+                    @fields = ('');
+                }
+                if ($level == 1) {
+                    $kind = $_;
+                    if ($kind eq 'union') {
+                        my $tmp = $func_id;
+                        $tmp =~ s/\./_/g;
+                        print " \\\n";
+                        print  "${prefix}switch ($tmp) {"
+                    }
+                }
+            } elsif ($_ eq '{') {
+                $level++;
+                $id = '';
+            } elsif ($_ eq '}') {
+                $level--;
+                $id = '';
+                if ($level == 1 and $kind eq 'union') {
+                    print " \\\n";
+                    print "$prefix}"
+                }
+            } elsif ($_ eq '[') {
+                if ($level != 2 or $arrlvl != 1) {
+                    # skip
+                } elsif ($array eq '') {
+                    $array = ' ';
+                } else {
+                    $array = "$array;";
+                }
+                $arrlvl++;
+            } elsif ($_ eq ']') {
+                $arrlvl--;
+            } elsif (m/^COMPAT_HANDLE\((.*)\)$/) {
+                if ($level == 2 and $id eq '') {
+                    $type = $1;
+                    $type =~ s/^compat_//;
+                }
+            } elsif ($_ eq "compat_domain_handle_t") {
+                if ($level == 2 and $id eq '') {
+                    $array_type = $_;
+                }
+            } elsif (m/^[a-zA-Z_].*$/) {
+                if ($id eq '' and $type eq '' and $array_type eq '') {
+                    foreach $id (@typedefs) {
+                        unless ($id ne $_) {
+                            $type = $id;
+                        }
+                    }
+                    if ($type eq '') {
+                        $id = $_;
+                    } else {
+                        $id = '';
+                    }
+                } else {
+                    $id = $_;
+                }
+            } elsif (m/^[,;]$/) {
+                if ($level == 2 and $id !~ /^_pad\d*$/) {
+                    if ($kind eq 'union') {
+                        my $tmp = "$func_id.$id";
+                        $tmp =~ s/\./_/g;
+                        print " \\\n";
+                        print  "${prefix}case XLAT_${name}_$tmp:";
+                        shift @fields if @fields > 0 and $fields[0] eq '';
+                        handle_field("$prefix    ", $name,  "$func_id.$id", $type, @fields);
+                    } elsif ($array eq '' and $array_type eq '') {
+                        shift @fields if @fields > 0 and $fields[0] eq '';
+                        handle_field($prefix, $name, "$func_id.$id", $type, @fields);
+                    } elsif ($array eq '') {
+                        copy_array("    ", "$func_id.$id");
+                    } else {
+                        $array =~ s/^.*?;//;
+                        shift @fields if @fields > 0 and $fields[0] eq '';
+                        handle_array($prefix, $name, "$func_id.$id", $array, $type, @fields);
+                    }
+                    unless ($_ ne ';') {
+                        @fields = ();
+                        $id = '';
+                        $type = '';
+                    }
+                    $array = '';
+                    if ($kind eq 'union') {
+                        print " \\\n";
+                        print "$prefix    break;";
+                    }
+                }
+            } else {
+                if ($array ne '') {
+                    $array = "$array $_";
+                }
+            }
+            unless (@fields == 0) {
+                push (@fields, $_);
+            }
+        }
+    }
+}
+
+sub copy_array {
+    my ($prefix, $id) = @_;
+
+    print " \\\n";
+    print  "${prefix}if ((_d_)->$id != (_s_)->$id) \\\n";
+    print  "$prefix    memcpy((_d_)->$id, (_s_)->$id, sizeof((_d_)->$id));";
+}
+
+sub handle_array {
+    my ($prefix, $name, $id, $array, $type, @fields) = @_;
+
+    my $i = $array;
+    $i =~ s/[^;]//g;
+    $i = length($i);
+    $i = "i$i";
+
+    print " \\\n";
+    print "$prefix\{ \\\n";
+    print "$prefix    unsigned int $i; \\\n";
+    my $tmp = $array;
+    $tmp =~ s/;.*$//;
+    $tmp =~ s/^\s*(.*)\s*$/$1/;
+    print "$prefix    for ($i = 0; $i < $tmp; ++$i) {";
+    if ($array !~ m/^.*?;/) {
+        handle_field("$prefix        ", $name, "$id\[$i]", $type, @fields);
+    } else {
+        handle_array("$prefix        " ,$name, "$id\[$i]", $', $type, @fields);
+    }
+    print " \\\n";
+    print "$prefix    } \\\n";
+    print "$prefix\}";
+}
+
+sub build_body {
+    my ($name, @tokens) = @_;
+    my $level = 1;
+    my $id = '';
+    my $array = '';
+    my $arrlvl = 1;
+    my $array_type = '';
+    my $type = '';
+    my @fields;
+
+    printf "\n#define XLAT_$name(_d_, _s_) do {";
+
+    foreach (@tokens) {
+        if (/^(struct|union)$/) {
+            unless ($level != 2) {
+                @fields = ('');
+            }
+        } elsif ($_ eq '{') {
+            $level++;
+            $id = '';
+        } elsif ($_ eq '}') {
+            $level--;
+            $id = '';
+        } elsif ($_ eq '[') {
+            if ($level != 2 or $arrlvl != 1) {
+                # skip
+            } elsif ($array eq '') {
+                $array = ' ';
+            } else {
+                $array = "$array;";
+            }
+            $arrlvl++;
+        } elsif ($_ eq ']') {
+            $arrlvl--;
+        } elsif (m/^COMPAT_HANDLE\((.*)\)$/) {
+            if ($level == 2 and $id eq '') {
+                $type = $1;
+                $type =~ s/^compat_//;
+            }
+        } elsif ($_ eq "compat_domain_handle_t") {
+            if ($level == 2 and $id eq '') {
+                $array_type = $_;
+            }
+        } elsif (m/^[a-zA-Z_].*$/) {
+            if ($array ne '') {
+                $array = "$array $_";
+            } elsif ($id eq '' and $type eq '' and $array_type eq '') {
+                foreach $id (@typedefs) {
+                    unless ($id eq $_) {
+                        $type = $id;
+                    }
+                }
+                if ($type eq '') {
+                    $id = $_;
+                } else {
+                    $id = '';
+                }
+            } else {
+                $id = $_;
+            }
+        } elsif (m/^[,;]$/) {
+            if ($level == 2 and $id !~ /^_pad\d*$/) {
+                if ($array eq '' and $array_type eq '') {
+                    shift @fields if @fields > 0 and $fields[0] eq '';
+                    handle_field("    ", $name, $id, $type, @fields);
+                } elsif ($array eq '') {
+                    copy_array("    ", $id);
+                } else {
+                    my $tmp = $array;
+                    $tmp =~ s/^.*?;//;
+                    shift @fields if @fields > 0 and $fields[0] eq '';
+                    handle_array("    ", $name, $id, $tmp, $type, @fields);
+                }
+                unless ($_ ne ';') {
+                    @fields = ();
+                    $id = '';
+                    $type = '';
+                }
+                $array = '';
+            }
+        } else {
+            if ($array ne '') {
+                $array = "$array $_";
+            }
+        }
+        unless (@fields == 0) {
+            push (@fields, $_);
+        }
+    }
+    print " \\\n} while (0)\n";
+}
+
+sub check_field {
+    my ($kind, $name, $field, @extrafields) = @_;
+
+    if ("@extrafields" !~ m/[{}]/) {
+        print "; \\\n";
+        if (@extrafields != 0) {
+            foreach (@extrafields) {
+                if (m/^(struct|union)$/) {
+                    # skip
+                } elsif (m/^[a-zA-Z_].*/) {
+                    s/^xen_//;
+                    print "    CHECK_$_";
+                    last;
+                } else {
+                    die "Malformed compound declaration: '$_'";
+                }
+            }
+        } elsif ($field !~ m/\./) {
+            print "    CHECK_FIELD_($kind, $name, $field)";
+        } else {
+            my $n = $field =~ s/\./, /g;
+            print  "    CHECK_SUBFIELD_${n}_($kind, $name, $field)";
+        }
+    } else {
+        my $level = 1;
+        my @fields = ();
+        my $id = '';
+
+        foreach (@extrafields) {
+            if (m/^(struct|union)$/) {
+                unless ($level != 2) {
+                    @fields = ('');
+                }
+            } elsif ($_ eq '{') {
+                $level++;
+                $id = '';
+            } elsif ($_ eq '}') {
+                $level--;
+                $id = '';
+            } elsif (/^compat_.*_t$/) {
+                if ($level == 2) {
+                    @fields = ('');
+                    s/_t$//;
+                    s/^compat_//;
+                }
+            } elsif (/^evtchn_.*_compat_t$/) {
+                if ($level == 2 and $_ ne "evtchn_port_compat_t") {
+                    @fields = ('');
+                    s/_compat_t$//;
+                }
+            } elsif (/^[a-zA-Z_].*$/) {
+                $id = $_;
+            } elsif (/^[,;]$/) {
+                if ($level == 2 and $id !~ /^_pad\d*$/) {
+                    shift @fields if @fields > 0 and $fields[0] eq '';
+                    check_field($kind, $name, "$field.$id", @fields);
+                    unless ($_ ne ";") {
+                        @fields = ();
+                        $id = '';
+                    }
+                }
+            }
+            unless (@fields == 0) {
+                push (@fields, $_);
+            }
+        }
+    }
+}
+
+sub build_check {
+    my ($name, @tokens) = @_;
+    my $level = 1;
+    my (@fields, $kind, $id);
+    my $arrlvl = 1;
+
+    print "\n";
+    print "#define CHECK_$name \\\n";
+
+    foreach (@tokens) {
+        if (/^(struct|union)$/) {
+            if ($level == 1) {
+                $kind = $_;
+                print "    CHECK_SIZE_($kind, $name)";
+            } elsif ($level == 2) {
+                @fields = ('');
+            }
+        } elsif ($_ eq '{') {
+            $level++;
+            $id = '';
+        } elsif ($_ eq '}') {
+            $level--;
+            $id = '';
+        } elsif ($_ eq '[') {
+            $arrlvl++;
+        } elsif ($_ eq ']') {
+            $arrlvl--;
+        } elsif (/^compat_.*_t$/) {
+            if ($level == 2 and $_ ne "compat_argo_port_t") {
+                @fields = ('');
+                s/_t$//;
+                s/^compat_//;
+            }
+        } elsif (/^[a-zA-Z_].*$/) {
+            unless ($level != 2 or $arrlvl != 1) {
+                $id = $_;
+            }
+        } elsif (/^[,;]$/) {
+            if ($level == 2 and $id !~ /^_pad\d*$/) {
+                shift @fields if @fields > 0 and $fields[0] eq '';
+                check_field($kind, $name, $id, @fields);
+                unless ($_ ne ";") {
+                    @fields = ();
+                    $id = '';
+                }
+            }
+        }
+
+        unless (@fields == 0) {
+            push (@fields, $_);
+        }
+    }
+    print "\n";
+}
+
+@typedefs = get_typedefs(@header_tokens);
+
+while (<COMPAT_LIST>) {
+    my ($what, $name) = split(/\s+/, $_);
+    $name =~ s/^xen//;
+
+    my @fields = get_fields("compat_$name");
+    if (@fields == 0) {
+	die "Fields of 'compat_$name' not found in '$ARGV[1]'";
+    }
+
+    if ($what eq "!") {
+        build_enums($name, @fields);
+        build_body($name, @fields);
+    } elsif ($what eq "?") {
+        build_check($name, @fields);
+    } else {
+        die "Invalid translation indicator: '$what'";
+    }
+}
diff --git a/xen/tools/get-fields.sh b/xen/tools/get-fields.sh
deleted file mode 100644
index 002db2093f..0000000000
--- a/xen/tools/get-fields.sh
+++ /dev/null
@@ -1,528 +0,0 @@
-test -n "$1" -a -n "$2" -a -n "$3"
-set -ef
-
-SED=sed
-if test -x /usr/xpg4/bin/sed; then
-	SED=/usr/xpg4/bin/sed
-fi
-if test -z ${PYTHON}; then
-	PYTHON=`/usr/bin/env python`
-fi
-if test -z ${PYTHON}; then
-	echo "Python not found"
-	exit 1
-fi
-
-get_fields ()
-{
-	local level=1 aggr=0 name= fields=
-	for token in $2
-	do
-		case "$token" in
-		struct|union)
-			test $level != 1 || aggr=1 fields= name=
-			;;
-		"{")
-			level=$(expr $level + 1)
-			;;
-		"}")
-			level=$(expr $level - 1)
-			if [ $level = 1 -a $name = $1 ]
-			then
-				echo "$fields }"
-				return 0
-			fi
-			;;
-		[a-zA-Z_]*)
-			test $aggr = 0 -o -n "$name" || name="$token"
-			;;
-		esac
-		test $aggr = 0 || fields="$fields $token"
-	done
-}
-
-get_typedefs ()
-{
-	local level=1 state=
-	for token in $1
-	do
-		case "$token" in
-		typedef)
-			test $level != 1 || state=1
-			;;
-		COMPAT_HANDLE\(*\))
-			test $level != 1 -o "$state" != 1 || state=2
-			;;
-		[\{\[])
-			level=$(expr $level + 1)
-			;;
-		[\}\]])
-			level=$(expr $level - 1)
-			;;
-		";")
-			test $level != 1 || state=
-			;;
-		[a-zA-Z_]*)
-			test $level != 1 -o "$state" != 2 || echo "$token"
-			;;
-		esac
-	done
-}
-
-build_enums ()
-{
-	local level=1 kind= fields= members= named= id= token
-	for token in $2
-	do
-		case "$token" in
-		struct|union)
-			test $level != 2 || fields=" "
-			kind="$token;$kind"
-			;;
-		"{")
-			level=$(expr $level + 1)
-			;;
-		"}")
-			level=$(expr $level - 1)
-			if [ $level = 1 ]
-			then
-				if [ "${kind%%;*}" = union ]
-				then
-					echo
-					echo "enum XLAT_$1 {"
-					for m in $members
-					do
-						echo "    XLAT_${1}_$m,"
-					done
-					echo "};"
-				fi
-				return 0
-			elif [ $level = 2 ]
-			then
-				named='?'
-			fi
-			;;
-		[a-zA-Z]*)
-			id=$token
-			if [ -n "$named" -a -n "${kind#*;}" ]
-			then
-				build_enums ${1}_$token "$fields"
-				named='!'
-			fi
-			;;
-		",")
-			test $level != 2 || members="$members $id"
-			;;
-		";")
-			test $level != 2 || members="$members $id"
-			test -z "$named" || kind=${kind#*;}
-			named=
-			;;
-		esac
-		test -z "$fields" || fields="$fields $token"
-	done
-}
-
-handle_field ()
-{
-	if [ -z "$5" ]
-	then
-		echo " \\"
-		if [ -z "$4" ]
-		then
-			printf %s "$1(_d_)->$3 = (_s_)->$3;"
-		else
-			printf %s "$1XLAT_${2}_HNDL_$(echo $3 | $SED 's,\.,_,g')(_d_, _s_);"
-		fi
-	elif [ -z "$(echo "$5" | $SED 's,[^{}],,g')" ]
-	then
-		local tag=$(echo "$5" | ${PYTHON} -c '
-import re,sys
-for line in sys.stdin.readlines():
-    sys.stdout.write(re.subn(r"\s*(struct|union)\s+(compat_)?(\w+)\s.*", r"\3", line)[0].rstrip() + "\n")
-')
-		echo " \\"
-		printf %s "${1}XLAT_$tag(&(_d_)->$3, &(_s_)->$3);"
-	else
-		local level=1 kind= fields= id= array= arrlvl=1 array_type= type= token
-		for token in $5
-		do
-			case "$token" in
-			struct|union)
-				test $level != 2 || fields=" "
-				if [ $level = 1 ]
-				then
-					kind=$token
-					if [ $kind = union ]
-					then
-						echo " \\"
-						printf %s "${1}switch ($(echo $3 | $SED 's,\.,_,g')) {"
-					fi
-				fi
-				;;
-			"{")
-				level=$(expr $level + 1) id=
-				;;
-			"}")
-				level=$(expr $level - 1) id=
-				if [ $level = 1 -a $kind = union ]
-				then
-					echo " \\"
-					printf %s "$1}"
-				fi
-				;;
-			"[")
-				if [ $level != 2 -o $arrlvl != 1 ]
-				then
-					:
-				elif [ -z "$array" ]
-				then
-					array=" "
-				else
-					array="$array;"
-				fi
-				arrlvl=$(expr $arrlvl + 1)
-				;;
-			"]")
-				arrlvl=$(expr $arrlvl - 1)
-				;;
-			COMPAT_HANDLE\(*\))
-				if [ $level = 2 -a -z "$id" ]
-				then
-					type=${token#COMPAT_HANDLE?}
-					type=${type%?}
-					type=${type#compat_}
-				fi
-				;;
-			compat_domain_handle_t)
-				if [ $level = 2 -a -z "$id" ]
-				then
-					array_type=$token
-				fi
-				;;
-			[a-zA-Z]*)
-				if [ -z "$id" -a -z "$type" -a -z "$array_type" ]
-				then
-					for id in $typedefs
-					do
-						test $id != "$token" || type=$id
-					done
-					if [ -z "$type" ]
-					then
-						id=$token
-					else
-						id=
-					fi
-				else
-					id=$token
-				fi
-				;;
-			[\,\;])
-				if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
-				then
-					if [ $kind = union ]
-					then
-						echo " \\"
-						printf %s "${1}case XLAT_${2}_$(echo $3.$id | $SED 's,\.,_,g'):"
-						handle_field "$1    " $2 $3.$id "$type" "$fields"
-					elif [ -z "$array" -a -z "$array_type" ]
-					then
-						handle_field "$1" $2 $3.$id "$type" "$fields"
-					elif [ -z "$array" ]
-					then
-						copy_array "    " $3.$id
-					else
-						handle_array "$1" $2 $3.$id "${array#*;}" "$type" "$fields"
-					fi
-					test "$token" != ";" || fields= id= type=
-					array=
-					if [ $kind = union ]
-					then
-						echo " \\"
-						printf %s "$1    break;"
-					fi
-				fi
-				;;
-			*)
-				if [ -n "$array" ]
-				then
-					array="$array $token"
-				fi
-				;;
-			esac
-			test -z "$fields" || fields="$fields $token"
-		done
-	fi
-}
-
-copy_array ()
-{
-	echo " \\"
-	echo "${1}if ((_d_)->$2 != (_s_)->$2) \\"
-	printf %s "$1    memcpy((_d_)->$2, (_s_)->$2, sizeof((_d_)->$2));"
-}
-
-handle_array ()
-{
-	local i="i$(echo $4 | $SED 's,[^;], ,g' | wc -w | $SED 's,[[:space:]]*,,g')"
-	echo " \\"
-	echo "$1{ \\"
-	echo "$1    unsigned int $i; \\"
-	printf %s "$1    for ($i = 0; $i < "${4%%;*}"; ++$i) {"
-	if [ "$4" = "${4#*;}" ]
-	then
-		handle_field "$1        " $2 $3[$i] "$5" "$6"
-	else
-		handle_array "$1        " $2 $3[$i] "${4#*;}" "$5" "$6"
-	fi
-	echo " \\"
-	echo "$1    } \\"
-	printf %s "$1}"
-}
-
-build_body ()
-{
-	echo
-	printf %s "#define XLAT_$1(_d_, _s_) do {"
-	local level=1 fields= id= array= arrlvl=1 array_type= type= token
-	for token in $2
-	do
-		case "$token" in
-		struct|union)
-			test $level != 2 || fields=" "
-			;;
-		"{")
-			level=$(expr $level + 1) id=
-			;;
-		"}")
-			level=$(expr $level - 1) id=
-			;;
-		"[")
-			if [ $level != 2 -o $arrlvl != 1 ]
-			then
-				:
-			elif [ -z "$array" ]
-			then
-				array=" "
-			else
-				array="$array;"
-			fi
-			arrlvl=$(expr $arrlvl + 1)
-			;;
-		"]")
-			arrlvl=$(expr $arrlvl - 1)
-			;;
-		COMPAT_HANDLE\(*\))
-			if [ $level = 2 -a -z "$id" ]
-			then
-				type=${token#COMPAT_HANDLE?}
-				type=${type%?}
-				type=${type#compat_}
-			fi
-			;;
-		compat_domain_handle_t)
-			if [ $level = 2 -a -z "$id" ]
-			then
-				array_type=$token
-			fi
-			;;
-		[a-zA-Z_]*)
-			if [ -n "$array" ]
-			then
-				array="$array $token"
-			elif [ -z "$id" -a -z "$type" -a -z "$array_type" ]
-			then
-				for id in $typedefs
-				do
-					test $id != "$token" || type=$id
-				done
-				if [ -z "$type" ]
-				then
-					id=$token
-				else
-					id=
-				fi
-			else
-				id=$token
-			fi
-			;;
-		[\,\;])
-			if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
-			then
-				if [ -z "$array" -a -z "$array_type" ]
-				then
-					handle_field "    " $1 $id "$type" "$fields"
-				elif [ -z "$array" ]
-				then
-					copy_array "    " $id
-				else
-					handle_array "    " $1 $id "${array#*;}" "$type" "$fields"
-				fi
-				test "$token" != ";" || fields= id= type=
-				array=
-			fi
-			;;
-		*)
-			if [ -n "$array" ]
-			then
-				array="$array $token"
-			fi
-			;;
-		esac
-		test -z "$fields" || fields="$fields $token"
-	done
-	echo " \\"
-	echo "} while (0)"
-}
-
-check_field ()
-{
-	if [ -z "$(echo "$4" | $SED 's,[^{}],,g')" ]
-	then
-		echo "; \\"
-		local n=$(echo $3 | $SED 's,[^.], ,g' | wc -w | $SED 's,[[:space:]]*,,g')
-		if [ -n "$4" ]
-		then
-			for n in $4
-			do
-				case $n in
-				struct|union)
-					;;
-				[a-zA-Z_]*)
-					printf %s "    CHECK_${n#xen_}"
-					break
-					;;
-				*)
-					echo "Malformed compound declaration: '$n'" >&2
-					exit 1
-					;;
-				esac
-			done
-		elif [ $n = 0 ]
-		then
-			printf %s "    CHECK_FIELD_($1, $2, $3)"
-		else
-			printf %s "    CHECK_SUBFIELD_${n}_($1, $2, $(echo $3 | $SED 's!\.!, !g'))"
-		fi
-	else
-		local level=1 fields= id= token
-		for token in $4
-		do
-			case "$token" in
-			struct|union)
-				test $level != 2 || fields=" "
-				;;
-			"{")
-				level=$(expr $level + 1) id=
-				;;
-			"}")
-				level=$(expr $level - 1) id=
-				;;
-			compat_*_t)
-				if [ $level = 2 ]
-				then
-					fields=" "
-					token="${token%_t}"
-					token="${token#compat_}"
-				fi
-				;;
-			evtchn_*_compat_t)
-				if [ $level = 2 -a $token != evtchn_port_compat_t ]
-				then
-					fields=" "
-					token="${token%_compat_t}"
-				fi
-				;;
-			[a-zA-Z]*)
-				id=$token
-				;;
-			[\,\;])
-				if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
-				then
-					check_field $1 $2 $3.$id "$fields"
-					test "$token" != ";" || fields= id=
-				fi
-				;;
-			esac
-			test -z "$fields" || fields="$fields $token"
-		done
-	fi
-}
-
-build_check ()
-{
-	echo
-	echo "#define CHECK_$1 \\"
-	local level=1 fields= kind= id= arrlvl=1 token
-	for token in $2
-	do
-		case "$token" in
-		struct|union)
-			if [ $level = 1 ]
-			then
-				kind=$token
-				printf %s "    CHECK_SIZE_($kind, $1)"
-			elif [ $level = 2 ]
-			then
-				fields=" "
-			fi
-			;;
-		"{")
-			level=$(expr $level + 1) id=
-			;;
-		"}")
-			level=$(expr $level - 1) id=
-			;;
-		"[")
-			arrlvl=$(expr $arrlvl + 1)
-			;;
-		"]")
-			arrlvl=$(expr $arrlvl - 1)
-			;;
-		compat_*_t)
-			if [ $level = 2 -a $token != compat_argo_port_t ]
-			then
-				fields=" "
-				token="${token%_t}"
-				token="${token#compat_}"
-			fi
-			;;
-		[a-zA-Z_]*)
-			test $level != 2 -o $arrlvl != 1 || id=$token
-			;;
-		[\,\;])
-			if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
-			then
-				check_field $kind $1 $id "$fields"
-				test "$token" != ";" || fields= id=
-			fi
-			;;
-		esac
-		test -z "$fields" || fields="$fields $token"
-	done
-	echo ""
-}
-
-list="$($SED -e 's,^[[:space:]]#.*,,' -e 's!\([]\[,;:{}]\)! \1 !g' $3)"
-fields="$(get_fields $(echo $2 | $SED 's,^compat_xen,compat_,') "$list")"
-if [ -z "$fields" ]
-then
-	echo "Fields of '$2' not found in '$3'" >&2
-	exit 1
-fi
-name=${2#compat_}
-name=${name#xen}
-case "$1" in
-"!")
-	typedefs="$(get_typedefs "$list")"
-	build_enums $name "$fields"
-	build_body $name "$fields"
-	;;
-"?")
-	build_check $name "$fields"
-	;;
-*)
-	echo "Invalid translation indicator: '$1'" >&2
-	exit 1
-	;;
-esac
-- 
Anthony PERARD

Re: [XEN PATCH 3/4] build: replace get-fields.sh by a perl script

[Elliott Mitchell]

On Wed, Jun 01, 2022 at 05:59:08PM +0100, Anthony PERARD wrote:
> diff --git a/xen/tools/compat-xlat-header b/xen/tools/compat-xlat-header
> new file mode 100755
> index 0000000000..f1f42a9dde
> --- /dev/null
> +++ b/xen/tools/compat-xlat-header
> @@ -0,0 +1,539 @@
> +#!/usr/bin/perl -w
> +
> +use strict;
> +use warnings;

I hope to take more of a look at this, but one thing I immediately
notice:  -w is redundant with "use warnings;".  I strongly prefer
"use warnings;", but others may have different preferences.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [XEN PATCH 3/4] build: replace get-fields.sh by a perl script

[Anthony PERARD]

On Wed, Jun 01, 2022 at 10:32:10AM -0700, Elliott Mitchell wrote:
> On Wed, Jun 01, 2022 at 05:59:08PM +0100, Anthony PERARD wrote:
> > diff --git a/xen/tools/compat-xlat-header b/xen/tools/compat-xlat-header
> > new file mode 100755
> > index 0000000000..f1f42a9dde
> > --- /dev/null
> > +++ b/xen/tools/compat-xlat-header
> > @@ -0,0 +1,539 @@
> > +#!/usr/bin/perl -w
> > +
> > +use strict;
> > +use warnings;
> 
> I hope to take more of a look at this, but one thing I immediately
> notice:  -w is redundant with "use warnings;".  I strongly prefer
> "use warnings;", but others may have different preferences.

Sounds good, I might have copy the shebang and the "use*" from an other
script in our repo, without checking what the -w stand for.

Thanks,

-- 
Anthony PERARD

[XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target

[Anthony PERARD]

Now that the command line generating "xlat.h" is check on rebuild, the
header will be regenerated whenever the list of xlat headers changes
due to change in ".config". We don't need to force a regeneration for
every changes in ".config".

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/include/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index b7e7148665..937a8bc884 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -99,7 +99,7 @@ cmd_xlat_h = \
 	cat $(filter %.h,$^) >$@.new; \
 	mv -f $@.new $@
 
-$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf FORCE
+$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) FORCE
 	$(call if_changed,xlat_h)
 
 ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
-- 
Anthony PERARD

Re: [XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target

[Jan Beulich]

On 01.06.2022 18:59, Anthony PERARD wrote:
> Now that the command line generating "xlat.h" is check on rebuild, the
> header will be regenerated whenever the list of xlat headers changes
> due to change in ".config". We don't need to force a regeneration for
> every changes in ".config".

This looks to be dependent on only patch 1; I wonder if it shouldn't be
viewed as an integral part of that adjustment. Anyway - if you want to
keep it on its own:
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [XEN PATCH 0/4] xen: rework compat headers generation

[Andrew Cooper]

On 01/06/2022 17:59, Anthony PERARD wrote:
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-include-rework-v1
>
> Hi,
>
> This patch series is about 2 improvement. First one is to use $(if_changed, )
> in "include/Makefile" to make the generation of the compat headers less verbose
> and to have the command line part of the decision to rebuild the headers.
> Second one is to replace one slow script by a much faster one, and save time
> when generating the headers.
>
> Thanks.
>
> Anthony PERARD (4):
>   build: xen/include: use if_changed
>   build: set PERL
>   build: replace get-fields.sh by a perl script
>   build: remove auto.conf prerequisite from compat/xlat.h target
>
>  xen/Makefile                 |   1 +
>  xen/include/Makefile         | 106 ++++---
>  xen/tools/compat-xlat-header | 539 +++++++++++++++++++++++++++++++++++
>  xen/tools/get-fields.sh      | 528 ----------------------------------

Excellent.  I was planning to ask you about this.  (I also need to
refreshing my half series cleaning up other bits of the build.)

One trivial observation is that it would probably be nicer to name the
script with a .pl extension.

Any numbers on what the speedup in patch 3 is?

Are the generated compat headers identical before and after this
series?  If yes, I'm very tempted to ack and commit it straight away.

~Andrew

Re: [XEN PATCH 0/4] xen: rework compat headers generation

[Anthony PERARD]

On Wed, Jun 01, 2022 at 05:17:36PM +0000, Andrew Cooper wrote:
> On 01/06/2022 17:59, Anthony PERARD wrote:
> > Patch series available in this git branch:
> > https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-include-rework-v1
> >
> > Hi,
> >
> > This patch series is about 2 improvement. First one is to use $(if_changed, )
> > in "include/Makefile" to make the generation of the compat headers less verbose
> > and to have the command line part of the decision to rebuild the headers.
> > Second one is to replace one slow script by a much faster one, and save time
> > when generating the headers.
> >
> > Thanks.
> >
> > Anthony PERARD (4):
> >   build: xen/include: use if_changed
> >   build: set PERL
> >   build: replace get-fields.sh by a perl script
> >   build: remove auto.conf prerequisite from compat/xlat.h target
> >
> >  xen/Makefile                 |   1 +
> >  xen/include/Makefile         | 106 ++++---
> >  xen/tools/compat-xlat-header | 539 +++++++++++++++++++++++++++++++++++
> >  xen/tools/get-fields.sh      | 528 ----------------------------------
> 
> Excellent.  I was planning to ask you about this.  (I also need to
> refreshing my half series cleaning up other bits of the build.)
> 
> One trivial observation is that it would probably be nicer to name the
> script with a .pl extension.

Sound fine, I don't think it matter much here.

> Any numbers on what the speedup in patch 3 is?

Yes, on my machine when doing a full build, with `ccache` enabled, it
saves about 1.17 seconds (out of ~17s), and without ccache, it saves
about 2.0 seconds (out of ~37s).

Without ccache:

before:
    $ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null >/dev/null; rm -r build; done
    make --no-print-directory -j$(nproc) -s O=build > /dev/null  244.98s user 29.24s system 683% cpu 40.146 total
    make --no-print-directory -j$(nproc) -s O=build > /dev/null  47.05s user 11.50s system 332% cpu 17.610 total
    make --no-print-directory -j$(nproc) -s O=build > /dev/null  47.35s user 11.22s system 330% cpu 17.733 total
    make --no-print-directory -j$(nproc) -s O=build > /dev/null  47.31s user 11.23s system 333% cpu 17.577 total

after:
    $ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null>/dev/null; rm -r build; done
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  237.28s user 25.97s system 667% cpu 39.456 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  37.60s user 8.20s system 282% cpu 16.214 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  37.95s user 8.67s system 279% cpu 16.651 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  38.02s user 8.40s system 280% cpu 16.545 total

And without ccache:

before:
    $ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null>/dev/null; rm -r build; done
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  206.37s user 22.19s system 640% cpu 35.695 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  221.45s user 22.26s system 667% cpu 36.537 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  233.95s user 23.80s system 686% cpu 37.518 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  234.27s user 23.83s system 680% cpu 37.923 total

after:
    $ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null>/dev/null; rm -r build; done
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  198.62s user 18.64s system 642% cpu 33.841 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  202.91s user 19.46s system 655% cpu 33.912 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  224.42s user 20.89s system 680% cpu 36.025 total
    make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null  222.93s user 21.29s system 683% cpu 35.708 total


> Are the generated compat headers identical before and after this
> series?  If yes, I'm very tempted to ack and commit it straight away.

Yes, the headers are identical. Hopefully, I've managed to check with
all compat headers enabled, since that depends on .config.

Cheers,

-- 
Anthony PERARD