Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o
On Tue, 22 Sep 2020, Julien Grall wrote:
> Hi Jan,
>
> On 22/09/2020 11:55, Jan Beulich wrote:
> > On 22.09.2020 11:24, Julien Grall wrote:
> > > On 22/09/2020 09:28, Jan Beulich wrote:
> > > > On 21.09.2020 13:39, Julien Grall wrote:
> > > > > On 21/09/2020 11:17, Jan Beulich wrote:
> > > > > > On 14.09.2020 12:15, Jan Beulich wrote:
> > > > > > > Switch to $(call if_changed,ld) where possible; presumably not
> > > > > > > doing so
> > > > > > > in e321576f4047 ("xen/build: start using if_changed") right away
> > > > > > > was an
> > > > > > > oversight, as it did for Arm in (just) one case. It failed to add
> > > > > > > prelink.o to $(targets), though, causing - judging from the
> > > > > > > observed
> > > > > > > behavior on x86 - undue rebuilds of the final binary (because of
> > > > > > > prelink.o getting rebuild for $(cmd_prelink.o) being empty, in
> > > > > > > turn
> > > > > > > because of .prelink.o.cmd not getting read) during "make
> > > > > > > install-xen".
> > > > > > >
> > > > > > > Signed-off-by: Jan Beulich
> > > > > > > ---
> > > > > > > xen/arch/arm/Makefile | 4 +++-
> > > > > > > xen/arch/x86/Makefile | 18 ++
> > > > > > > 2 files changed, 13 insertions(+), 9 deletions(-)
> > > > > >
> > > > > > May I ask for an Arm-side ack (or otherwise) here, please?
> > > > >
> > > > > Acked-by: Julien Grall
> > > >
> > > > Thanks. On the Arm side this is actually addressing a (minor) bug,
> > >
> > > Just to confirm, the bug is: Xen will be rebuilt when it is not
> > > necessary, right?
> >
> > Yes. When building as non-root but installing as root, this would
> > typically involve an owner change of some of the involved files.
> > That's how I did notice the issue on x86 (after switching to
> > if_changed) in the first place.
>
> Thanks for the explanation. I think it would be fine to backport.
> @Stefano, what do you think?
I am OK with that
Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o
Hi Jan,
On 22/09/2020 11:55, Jan Beulich wrote:
On 22.09.2020 11:24, Julien Grall wrote:
On 22/09/2020 09:28, Jan Beulich wrote:
On 21.09.2020 13:39, Julien Grall wrote:
On 21/09/2020 11:17, Jan Beulich wrote:
On 14.09.2020 12:15, Jan Beulich wrote:
Switch to $(call if_changed,ld) where possible; presumably not doing so
in e321576f4047 ("xen/build: start using if_changed") right away was an
oversight, as it did for Arm in (just) one case. It failed to add
prelink.o to $(targets), though, causing - judging from the observed
behavior on x86 - undue rebuilds of the final binary (because of
prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
because of .prelink.o.cmd not getting read) during "make install-xen".
Signed-off-by: Jan Beulich
---
xen/arch/arm/Makefile | 4 +++-
xen/arch/x86/Makefile | 18 ++
2 files changed, 13 insertions(+), 9 deletions(-)
May I ask for an Arm-side ack (or otherwise) here, please?
Acked-by: Julien Grall
Thanks. On the Arm side this is actually addressing a (minor) bug,
Just to confirm, the bug is: Xen will be rebuilt when it is not
necessary, right?
Yes. When building as non-root but installing as root, this would
typically involve an owner change of some of the involved files.
That's how I did notice the issue on x86 (after switching to
if_changed) in the first place.
Thanks for the explanation. I think it would be fine to backport.
@Stefano, what do you think?
Cheers,
--
Julien Grall
Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o
On 22.09.2020 11:24, Julien Grall wrote:
> On 22/09/2020 09:28, Jan Beulich wrote:
>> On 21.09.2020 13:39, Julien Grall wrote:
>>> On 21/09/2020 11:17, Jan Beulich wrote:
On 14.09.2020 12:15, Jan Beulich wrote:
> Switch to $(call if_changed,ld) where possible; presumably not doing so
> in e321576f4047 ("xen/build: start using if_changed") right away was an
> oversight, as it did for Arm in (just) one case. It failed to add
> prelink.o to $(targets), though, causing - judging from the observed
> behavior on x86 - undue rebuilds of the final binary (because of
> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
> because of .prelink.o.cmd not getting read) during "make install-xen".
>
> Signed-off-by: Jan Beulich
> ---
>xen/arch/arm/Makefile | 4 +++-
>xen/arch/x86/Makefile | 18 ++
>2 files changed, 13 insertions(+), 9 deletions(-)
May I ask for an Arm-side ack (or otherwise) here, please?
>>>
>>> Acked-by: Julien Grall
>>
>> Thanks. On the Arm side this is actually addressing a (minor) bug,
>
> Just to confirm, the bug is: Xen will be rebuilt when it is not
> necessary, right?
Yes. When building as non-root but installing as root, this would
typically involve an owner change of some of the involved files.
That's how I did notice the issue on x86 (after switching to
if_changed) in the first place.
Jan
Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o
Hi Jan,
On 22/09/2020 09:28, Jan Beulich wrote:
On 21.09.2020 13:39, Julien Grall wrote:
On 21/09/2020 11:17, Jan Beulich wrote:
On 14.09.2020 12:15, Jan Beulich wrote:
Switch to $(call if_changed,ld) where possible; presumably not doing so
in e321576f4047 ("xen/build: start using if_changed") right away was an
oversight, as it did for Arm in (just) one case. It failed to add
prelink.o to $(targets), though, causing - judging from the observed
behavior on x86 - undue rebuilds of the final binary (because of
prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
because of .prelink.o.cmd not getting read) during "make install-xen".
Signed-off-by: Jan Beulich
---
xen/arch/arm/Makefile | 4 +++-
xen/arch/x86/Makefile | 18 ++
2 files changed, 13 insertions(+), 9 deletions(-)
May I ask for an Arm-side ack (or otherwise) here, please?
Acked-by: Julien Grall
Thanks. On the Arm side this is actually addressing a (minor) bug,
Just to confirm, the bug is: Xen will be rebuilt when it is not
necessary, right?
Cheers,
--
Julien Grall
Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o
On 21.09.2020 13:39, Julien Grall wrote:
> On 21/09/2020 11:17, Jan Beulich wrote:
>> On 14.09.2020 12:15, Jan Beulich wrote:
>>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>>> oversight, as it did for Arm in (just) one case. It failed to add
>>> prelink.o to $(targets), though, causing - judging from the observed
>>> behavior on x86 - undue rebuilds of the final binary (because of
>>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>>> because of .prelink.o.cmd not getting read) during "make install-xen".
>>>
>>> Signed-off-by: Jan Beulich
>>> ---
>>> xen/arch/arm/Makefile | 4 +++-
>>> xen/arch/x86/Makefile | 18 ++
>>> 2 files changed, 13 insertions(+), 9 deletions(-)
>>
>> May I ask for an Arm-side ack (or otherwise) here, please?
>
> Acked-by: Julien Grall
Thanks. On the Arm side this is actually addressing a (minor) bug,
so I wonder whether I should queue this up for backporting. Do you
have an opinion either way?
Jan
Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o
Hi Jan,
On 21/09/2020 11:17, Jan Beulich wrote:
On 14.09.2020 12:15, Jan Beulich wrote:
Switch to $(call if_changed,ld) where possible; presumably not doing so
in e321576f4047 ("xen/build: start using if_changed") right away was an
oversight, as it did for Arm in (just) one case. It failed to add
prelink.o to $(targets), though, causing - judging from the observed
behavior on x86 - undue rebuilds of the final binary (because of
prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
because of .prelink.o.cmd not getting read) during "make install-xen".
Signed-off-by: Jan Beulich
---
xen/arch/arm/Makefile | 4 +++-
xen/arch/x86/Makefile | 18 ++
2 files changed, 13 insertions(+), 9 deletions(-)
May I ask for an Arm-side ack (or otherwise) here, please?
Acked-by: Julien Grall
Cheers,
Jan
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 51173d97127e..296c5e68bbc3 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -95,12 +95,14 @@ prelink_lto.o: $(ALL_OBJS)
# Link it with all the binary objects
prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
- $(LD) $(XEN_LDFLAGS) -r -o $@ $^
+ $(call if_changed,ld)
else
prelink.o: $(ALL_OBJS) FORCE
$(call if_changed,ld)
endif
+targets += prelink.o
+
$(TARGET)-syms: prelink.o xen.lds
$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \
$(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 74152f2a0dad..9b368632fb43 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -136,19 +136,21 @@ prelink_lto.o: $(ALL_OBJS)
$(LD_LTO) -r -o $@ $^
# Link it with all the binary objects
-prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
$(EFI_OBJS-y)
- $(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
$(EFI_OBJS-y) FORCE
+ $(call if_changed,ld)
-prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
- $(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS))
prelink_lto.o FORCE
+ $(call if_changed,ld)
else
-prelink.o: $(ALL_OBJS) $(EFI_OBJS-y)
- $(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink.o: $(ALL_OBJS) $(EFI_OBJS-y) FORCE
+ $(call if_changed,ld)
-prelink-efi.o: $(ALL_OBJS)
- $(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink-efi.o: $(ALL_OBJS) FORCE
+ $(call if_changed,ld)
endif
+targets += prelink.o prelink-efi.o
+
$(TARGET)-syms: prelink.o xen.lds
$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
$(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
--
Julien Grall
