[PATCH] x86/build: suppress EFI-related tool chain checks upon local $(MAKE) recursion

Jan Beulich posted 1 patch 3 years, 2 months ago
Failed in applying to current master (apply log)
[PATCH] x86/build: suppress EFI-related tool chain checks upon local $(MAKE) recursion
Posted by Jan Beulich 3 years, 2 months ago
The xen-syms and xen.efi linking steps are serialized only when the
intermediate note.o file is necessary. Otherwise both may run in
parallel. This in turn means that the compiler / linker invocations to
create efi/check.o / efi/check.efi may also happen twice in parallel.
Obviously it's a bad idea to have multiple producers of the same output
race with one another - every once in a while one may e.g. observe

objdump: efi/check.efi: file format not recognized

We don't need this EFI related checking to occur when producing the
intermediate symbol and relocation table objects, and we have an easy
way of suppressing it: Simply pass in "efi-y=", overriding the
assignments done in the Makefile and thus forcing the tool chain checks
to be bypassed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Obviously the real (but more involved) solution would be to do away with
the recursive $(MAKE) invocations, by breaking up the long linking
rules. Representing them instead through multiple smaller rules with
suitable dependencies is certainly possible (and might even reduce
redundancy).

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -182,13 +182,13 @@ $(TARGET)-syms: prelink.o xen.lds
 	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort \
 		>$(@D)/.$(@F).0.S
-	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
+	$(MAKE) -f $(BASEDIR)/Rules.mk efi-y= $(@D)/.$(@F).0.o
 	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
 	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
 		>$(@D)/.$(@F).1.S
-	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
+	$(MAKE) -f $(BASEDIR)/Rules.mk efi-y= $(@D)/.$(@F).1.o
 	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(@D)/.$(@F).1.o -o $@
 	$(NM) -pa --format=sysv $(@D)/$(@F) \
@@ -249,14 +249,14 @@ endif
 	$(MKRELOC) $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S
 	$(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S
-	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o
+	$(MAKE) -f $(BASEDIR)/Rules.mk efi-y= $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o
 	$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
 	          $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \
 	                $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(note_file_option) -o $(@D)/.$(@F).$(base).1 &&) :
 	$(MKRELOC) $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S
 	$(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S
-	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o
+	$(MAKE) -f $(BASEDIR)/Rules.mk efi-y= $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o
 	$(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
 	                $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(note_file_option) -o $@
 	$(NM) -pa --format=sysv $(@D)/$(@F) \


Re: [PATCH] x86/build: suppress EFI-related tool chain checks upon local $(MAKE) recursion
Posted by Anthony PERARD 3 years, 1 month ago
On Tue, Sep 21, 2021 at 05:43:38PM +0200, Jan Beulich wrote:
> The xen-syms and xen.efi linking steps are serialized only when the
> intermediate note.o file is necessary. Otherwise both may run in
> parallel. This in turn means that the compiler / linker invocations to
> create efi/check.o / efi/check.efi may also happen twice in parallel.
> Obviously it's a bad idea to have multiple producers of the same output
> race with one another - every once in a while one may e.g. observe
> 
> objdump: efi/check.efi: file format not recognized
> 
> We don't need this EFI related checking to occur when producing the
> intermediate symbol and relocation table objects, and we have an easy
> way of suppressing it: Simply pass in "efi-y=", overriding the
> assignments done in the Makefile and thus forcing the tool chain checks
> to be bypassed.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

> ---
> Obviously the real (but more involved) solution would be to do away with
> the recursive $(MAKE) invocations, by breaking up the long linking
> rules. Representing them instead through multiple smaller rules with
> suitable dependencies is certainly possible (and might even reduce
> redundancy).

There is an alternative to that. Linux have a script which does the kind
of step we do. So maybe doing the same and move the recipe into a script
would work too? This would allow to share the recipe between x86 and Arm
as the link phase of xen-syms is nearly identical. But to avoid calling
make from the script we would have to duplicate the recipe of %.o:%.S.
The xen.efi rules is still x86 only and I don't know whether we could
use the same script as for xen-syms.

I don't know which option would be better those two and the current
state.

Cheers,

-- 
Anthony PERARD

Re: [PATCH] x86/build: suppress EFI-related tool chain checks upon local $(MAKE) recursion
Posted by Jan Beulich 3 years, 1 month ago
On 08.10.2021 13:09, Anthony PERARD wrote:
> On Tue, Sep 21, 2021 at 05:43:38PM +0200, Jan Beulich wrote:
>> The xen-syms and xen.efi linking steps are serialized only when the
>> intermediate note.o file is necessary. Otherwise both may run in
>> parallel. This in turn means that the compiler / linker invocations to
>> create efi/check.o / efi/check.efi may also happen twice in parallel.
>> Obviously it's a bad idea to have multiple producers of the same output
>> race with one another - every once in a while one may e.g. observe
>>
>> objdump: efi/check.efi: file format not recognized
>>
>> We don't need this EFI related checking to occur when producing the
>> intermediate symbol and relocation table objects, and we have an easy
>> way of suppressing it: Simply pass in "efi-y=", overriding the
>> assignments done in the Makefile and thus forcing the tool chain checks
>> to be bypassed.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks.

>> ---
>> Obviously the real (but more involved) solution would be to do away with
>> the recursive $(MAKE) invocations, by breaking up the long linking
>> rules. Representing them instead through multiple smaller rules with
>> suitable dependencies is certainly possible (and might even reduce
>> redundancy).
> 
> There is an alternative to that. Linux have a script which does the kind
> of step we do. So maybe doing the same and move the recipe into a script
> would work too? This would allow to share the recipe between x86 and Arm
> as the link phase of xen-syms is nearly identical. But to avoid calling
> make from the script we would have to duplicate the recipe of %.o:%.S.
> The xen.efi rules is still x86 only and I don't know whether we could
> use the same script as for xen-syms.

Hmm, maybe. The main aspect I'm not sure about is that, as you say, we
have two linking targets while Linux has just one. If there are no
interdependencies regardless of build mode, doing what you suggest may
be fine. If there were interdependencies, your approach may lead to
serialization of both rules, while breaking up may allow large parts to
get carried out in parallel.

Also, not exactly fitting the pattern above, lines like

	$(MAKE) -f $(BASEDIR)/Rules.mk efi-y= $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o

when split up allow both objects to (continue to) be built in parallel;
arranging for this from a script may be more involved (albeit my knowledge
of how make works is lacking here - maybe an instance invoked from a shell
script can use the parent make's job server just as well as one invoked
from a Makefile). As you may imply, I don't think we should be duplicating
the %.o:%.S rule, but indeed have make deal with this.

Jan