vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
enough to fix the issue. Kbuild is correctly rebuilding it because the
command line is changed.
PowerPC builds each vdso directory twice; first in vdso_prepare to
generate vdso{32,64}-offsets.h, second as part of the ordinary build
process to embed vdso{32,64}.so.dbg into the kernel.
The problem shows up when CONFIG_PPC_WERROR=y due to the following line
in arch/powerpc/Kbuild:
subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
In the preparation stage, Kbuild directly visits the vdso directories,
hence it does not inherit subdir-ccflags-y. In the second descend,
Kbuild adds -Werror, which results in the command line flipping
with/without -Werror.
It implies a potential danger; if a more critical flag that would impact
the resulted vdso, the offsets recorded in the headers might be different
from real offsets in the embedded vdso images.
Removing the unneeded second descend solves the problem.
Link: https://lore.kernel.org/linuxppc-dev/[email protected]/
Reported-by: Michael Ellerman <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---
arch/powerpc/kernel/Makefile | 4 ++--
arch/powerpc/kernel/vdso32/Makefile | 5 +----
arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S | 0
arch/powerpc/kernel/vdso64/Makefile | 6 +-----
arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S | 0
5 files changed, 4 insertions(+), 11 deletions(-)
rename arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S (100%)
rename arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S (100%)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index fe2ef598e2ea..79ee7750937d 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -51,7 +51,7 @@ obj-y += ptrace/
obj-$(CONFIG_PPC64) += setup_64.o \
paca.o nvram_64.o note.o syscall_64.o
obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
-obj-$(CONFIG_VDSO32) += vdso32/
+obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
obj-$(CONFIG_PPC_DAWR) += dawr.o
@@ -60,7 +60,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o
obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o
obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o
-obj-$(CONFIG_PPC64) += vdso64/
+obj-$(CONFIG_PPC64) += vdso64_wrapper.o
obj-$(CONFIG_ALTIVEC) += vecemu.o
obj-$(CONFIG_PPC_BOOK3S_IDLE) += idle_book3s.o
procfs-y := proc_powerpc.o
diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index 59aa2944ecae..42fc3de89b39 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -30,7 +30,7 @@ CC32FLAGS += -m32
KBUILD_CFLAGS := $(filter-out -mcmodel=medium,$(KBUILD_CFLAGS))
endif
-targets := $(obj-vdso32) vdso32.so.dbg
+targets := $(obj-vdso32) vdso32.so.dbg vgettimeofday.o
obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
GCOV_PROFILE := n
@@ -46,9 +46,6 @@ obj-y += vdso32_wrapper.o
targets += vdso32.lds
CPPFLAGS_vdso32.lds += -P -C -Upowerpc
-# Force dependency (incbin is bad)
-$(obj)/vdso32_wrapper.o : $(obj)/vdso32.so.dbg
-
# link rule for the .so file, .lds has to be first
$(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) $(obj)/vgettimeofday.o FORCE
$(call if_changed,vdso32ld_and_check)
diff --git a/arch/powerpc/kernel/vdso32/vdso32_wrapper.S b/arch/powerpc/kernel/vdso32_wrapper.S
similarity index 100%
rename from arch/powerpc/kernel/vdso32/vdso32_wrapper.S
rename to arch/powerpc/kernel/vdso32_wrapper.S
diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
index d365810a689a..b50b39fedf74 100644
--- a/arch/powerpc/kernel/vdso64/Makefile
+++ b/arch/powerpc/kernel/vdso64/Makefile
@@ -17,7 +17,7 @@ endif
# Build rules
-targets := $(obj-vdso64) vdso64.so.dbg
+targets := $(obj-vdso64) vdso64.so.dbg vgettimeofday.o
obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
GCOV_PROFILE := n
@@ -29,15 +29,11 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
-Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
asflags-y := -D__VDSO64__ -s
-obj-y += vdso64_wrapper.o
targets += vdso64.lds
CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
$(obj)/vgettimeofday.o: %.o: %.c FORCE
-# Force dependency (incbin is bad)
-$(obj)/vdso64_wrapper.o : $(obj)/vdso64.so.dbg
-
# link rule for the .so file, .lds has to be first
$(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
$(call if_changed,vdso64ld_and_check)
diff --git a/arch/powerpc/kernel/vdso64/vdso64_wrapper.S b/arch/powerpc/kernel/vdso64_wrapper.S
similarity index 100%
rename from arch/powerpc/kernel/vdso64/vdso64_wrapper.S
rename to arch/powerpc/kernel/vdso64_wrapper.S
--
2.27.0
On Thu, Dec 24, 2020 at 2:12 AM Masahiro Yamada <[email protected]> wrote:
>
> vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
> enough to fix the issue. Kbuild is correctly rebuilding it because the
> command line is changed.
>
> PowerPC builds each vdso directory twice; first in vdso_prepare to
> generate vdso{32,64}-offsets.h, second as part of the ordinary build
> process to embed vdso{32,64}.so.dbg into the kernel.
>
> The problem shows up when CONFIG_PPC_WERROR=y due to the following line
> in arch/powerpc/Kbuild:
>
> subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>
> In the preparation stage, Kbuild directly visits the vdso directories,
> hence it does not inherit subdir-ccflags-y. In the second descend,
> Kbuild adds -Werror, which results in the command line flipping
> with/without -Werror.
>
> It implies a potential danger; if a more critical flag that would impact
> the resulted vdso, the offsets recorded in the headers might be different
> from real offsets in the embedded vdso images.
>
> Removing the unneeded second descend solves the problem.
>
> Link: https://lore.kernel.org/linuxppc-dev/[email protected]/
> Reported-by: Michael Ellerman <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
Michael, please take a look at this.
The unneeded rebuild problem is still remaining.
>
> arch/powerpc/kernel/Makefile | 4 ++--
> arch/powerpc/kernel/vdso32/Makefile | 5 +----
> arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S | 0
> arch/powerpc/kernel/vdso64/Makefile | 6 +-----
> arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S | 0
> 5 files changed, 4 insertions(+), 11 deletions(-)
> rename arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S (100%)
> rename arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S (100%)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index fe2ef598e2ea..79ee7750937d 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -51,7 +51,7 @@ obj-y += ptrace/
> obj-$(CONFIG_PPC64) += setup_64.o \
> paca.o nvram_64.o note.o syscall_64.o
> obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
> -obj-$(CONFIG_VDSO32) += vdso32/
> +obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
> obj-$(CONFIG_PPC_DAWR) += dawr.o
> @@ -60,7 +60,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o
> obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o
> obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
> obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o
> -obj-$(CONFIG_PPC64) += vdso64/
> +obj-$(CONFIG_PPC64) += vdso64_wrapper.o
> obj-$(CONFIG_ALTIVEC) += vecemu.o
> obj-$(CONFIG_PPC_BOOK3S_IDLE) += idle_book3s.o
> procfs-y := proc_powerpc.o
> diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
> index 59aa2944ecae..42fc3de89b39 100644
> --- a/arch/powerpc/kernel/vdso32/Makefile
> +++ b/arch/powerpc/kernel/vdso32/Makefile
> @@ -30,7 +30,7 @@ CC32FLAGS += -m32
> KBUILD_CFLAGS := $(filter-out -mcmodel=medium,$(KBUILD_CFLAGS))
> endif
>
> -targets := $(obj-vdso32) vdso32.so.dbg
> +targets := $(obj-vdso32) vdso32.so.dbg vgettimeofday.o
> obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
>
> GCOV_PROFILE := n
> @@ -46,9 +46,6 @@ obj-y += vdso32_wrapper.o
> targets += vdso32.lds
> CPPFLAGS_vdso32.lds += -P -C -Upowerpc
>
> -# Force dependency (incbin is bad)
> -$(obj)/vdso32_wrapper.o : $(obj)/vdso32.so.dbg
> -
> # link rule for the .so file, .lds has to be first
> $(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) $(obj)/vgettimeofday.o FORCE
> $(call if_changed,vdso32ld_and_check)
> diff --git a/arch/powerpc/kernel/vdso32/vdso32_wrapper.S b/arch/powerpc/kernel/vdso32_wrapper.S
> similarity index 100%
> rename from arch/powerpc/kernel/vdso32/vdso32_wrapper.S
> rename to arch/powerpc/kernel/vdso32_wrapper.S
> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> index d365810a689a..b50b39fedf74 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -17,7 +17,7 @@ endif
>
> # Build rules
>
> -targets := $(obj-vdso64) vdso64.so.dbg
> +targets := $(obj-vdso64) vdso64.so.dbg vgettimeofday.o
> obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
>
> GCOV_PROFILE := n
> @@ -29,15 +29,11 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
> -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
> asflags-y := -D__VDSO64__ -s
>
> -obj-y += vdso64_wrapper.o
> targets += vdso64.lds
> CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
>
> $(obj)/vgettimeofday.o: %.o: %.c FORCE
>
> -# Force dependency (incbin is bad)
> -$(obj)/vdso64_wrapper.o : $(obj)/vdso64.so.dbg
> -
> # link rule for the .so file, .lds has to be first
> $(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
> $(call if_changed,vdso64ld_and_check)
> diff --git a/arch/powerpc/kernel/vdso64/vdso64_wrapper.S b/arch/powerpc/kernel/vdso64_wrapper.S
> similarity index 100%
> rename from arch/powerpc/kernel/vdso64/vdso64_wrapper.S
> rename to arch/powerpc/kernel/vdso64_wrapper.S
> --
> 2.27.0
>
--
Best Regards
Masahiro Yamada
On Thu, 24 Dec 2020 02:11:41 +0900, Masahiro Yamada wrote:
> vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
> enough to fix the issue. Kbuild is correctly rebuilding it because the
> command line is changed.
>
> PowerPC builds each vdso directory twice; first in vdso_prepare to
> generate vdso{32,64}-offsets.h, second as part of the ordinary build
> process to embed vdso{32,64}.so.dbg into the kernel.
>
> [...]
Applied to powerpc/fixes.
[1/2] powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o
https://git.kernel.org/powerpc/c/bce74491c3008e27dd6e8f79a83b4faa77a08f7e
[2/2] powerpc/vdso64: remove meaningless vgettimeofday.o build rule
https://git.kernel.org/powerpc/c/66f0a9e058fad50e569ad752be72e52701991fd5
cheers
Le 28/01/2021 à 05:01, Masahiro Yamada a écrit :
> On Thu, Dec 24, 2020 at 2:12 AM Masahiro Yamada <[email protected]> wrote:
>>
>> vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
>> enough to fix the issue. Kbuild is correctly rebuilding it because the
>> command line is changed.
>>
>> PowerPC builds each vdso directory twice; first in vdso_prepare to
>> generate vdso{32,64}-offsets.h, second as part of the ordinary build
>> process to embed vdso{32,64}.so.dbg into the kernel.
>>
>> The problem shows up when CONFIG_PPC_WERROR=y due to the following line
>> in arch/powerpc/Kbuild:
>>
>> subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>>
>> In the preparation stage, Kbuild directly visits the vdso directories,
>> hence it does not inherit subdir-ccflags-y. In the second descend,
>> Kbuild adds -Werror, which results in the command line flipping
>> with/without -Werror.
>>
>> It implies a potential danger; if a more critical flag that would impact
>> the resulted vdso, the offsets recorded in the headers might be different
>> from real offsets in the embedded vdso images.
>>
>> Removing the unneeded second descend solves the problem.
>>
>> Link: https://lore.kernel.org/linuxppc-dev/[email protected]/
>> Reported-by: Michael Ellerman <[email protected]>
>> Signed-off-by: Masahiro Yamada <[email protected]>
>> ---
>
>
> Michael, please take a look at this.
>
> The unneeded rebuild problem is still remaining.
Seems like with this patch, vdso32_wrapper.o is not rebuilt when vdso32.so.dbg is rebuilt.
Leading to ... disasters.
I'll send a patch
>
>
>>
>> arch/powerpc/kernel/Makefile | 4 ++--
>> arch/powerpc/kernel/vdso32/Makefile | 5 +----
>> arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S | 0
>> arch/powerpc/kernel/vdso64/Makefile | 6 +-----
>> arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S | 0
>> 5 files changed, 4 insertions(+), 11 deletions(-)
>> rename arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S (100%)
>> rename arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S (100%)
>>
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index fe2ef598e2ea..79ee7750937d 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -51,7 +51,7 @@ obj-y += ptrace/
>> obj-$(CONFIG_PPC64) += setup_64.o \
>> paca.o nvram_64.o note.o syscall_64.o
>> obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
>> -obj-$(CONFIG_VDSO32) += vdso32/
>> +obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
>> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
>> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
>> obj-$(CONFIG_PPC_DAWR) += dawr.o
>> @@ -60,7 +60,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o
>> obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o
>> obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
>> obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o
>> -obj-$(CONFIG_PPC64) += vdso64/
>> +obj-$(CONFIG_PPC64) += vdso64_wrapper.o
>> obj-$(CONFIG_ALTIVEC) += vecemu.o
>> obj-$(CONFIG_PPC_BOOK3S_IDLE) += idle_book3s.o
>> procfs-y := proc_powerpc.o
>> diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
>> index 59aa2944ecae..42fc3de89b39 100644
>> --- a/arch/powerpc/kernel/vdso32/Makefile
>> +++ b/arch/powerpc/kernel/vdso32/Makefile
>> @@ -30,7 +30,7 @@ CC32FLAGS += -m32
>> KBUILD_CFLAGS := $(filter-out -mcmodel=medium,$(KBUILD_CFLAGS))
>> endif
>>
>> -targets := $(obj-vdso32) vdso32.so.dbg
>> +targets := $(obj-vdso32) vdso32.so.dbg vgettimeofday.o
>> obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
>>
>> GCOV_PROFILE := n
>> @@ -46,9 +46,6 @@ obj-y += vdso32_wrapper.o
>> targets += vdso32.lds
>> CPPFLAGS_vdso32.lds += -P -C -Upowerpc
>>
>> -# Force dependency (incbin is bad)
>> -$(obj)/vdso32_wrapper.o : $(obj)/vdso32.so.dbg
>> -
>> # link rule for the .so file, .lds has to be first
>> $(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) $(obj)/vgettimeofday.o FORCE
>> $(call if_changed,vdso32ld_and_check)
>> diff --git a/arch/powerpc/kernel/vdso32/vdso32_wrapper.S b/arch/powerpc/kernel/vdso32_wrapper.S
>> similarity index 100%
>> rename from arch/powerpc/kernel/vdso32/vdso32_wrapper.S
>> rename to arch/powerpc/kernel/vdso32_wrapper.S
>> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
>> index d365810a689a..b50b39fedf74 100644
>> --- a/arch/powerpc/kernel/vdso64/Makefile
>> +++ b/arch/powerpc/kernel/vdso64/Makefile
>> @@ -17,7 +17,7 @@ endif
>>
>> # Build rules
>>
>> -targets := $(obj-vdso64) vdso64.so.dbg
>> +targets := $(obj-vdso64) vdso64.so.dbg vgettimeofday.o
>> obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
>>
>> GCOV_PROFILE := n
>> @@ -29,15 +29,11 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
>> -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
>> asflags-y := -D__VDSO64__ -s
>>
>> -obj-y += vdso64_wrapper.o
>> targets += vdso64.lds
>> CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
>>
>> $(obj)/vgettimeofday.o: %.o: %.c FORCE
>>
>> -# Force dependency (incbin is bad)
>> -$(obj)/vdso64_wrapper.o : $(obj)/vdso64.so.dbg
>> -
>> # link rule for the .so file, .lds has to be first
>> $(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
>> $(call if_changed,vdso64ld_and_check)
>> diff --git a/arch/powerpc/kernel/vdso64/vdso64_wrapper.S b/arch/powerpc/kernel/vdso64_wrapper.S
>> similarity index 100%
>> rename from arch/powerpc/kernel/vdso64/vdso64_wrapper.S
>> rename to arch/powerpc/kernel/vdso64_wrapper.S
>> --
>> 2.27.0
>>
>
>