| Message ID | 20201223171142.707053-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o | expand |
On Thu, Dec 24, 2020 at 2:12 AM Masahiro Yamada <masahiroy@kernel.org> 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/87tuslxhry.fsf@mpe.ellerman.id.au/ > Reported-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- 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 >
Masahiro Yamada <masahiroy@kernel.org> writes: > On Thu, Dec 24, 2020 at 2:12 AM Masahiro Yamada <masahiroy@kernel.org> 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/87tuslxhry.fsf@mpe.ellerman.id.au/ >> Reported-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> >> --- > > > Michael, please take a look at this. > > The unneeded rebuild problem is still remaining. Sorry missed those. I guess I'll pick these up as fixes for v5.10. cheers
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 <masahiroy@kernel.org> 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/87tuslxhry.fsf@mpe.ellerman.id.au/ >> Reported-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> >> --- > > > 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 >> > >
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
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/87tuslxhry.fsf@mpe.ellerman.id.au/ Reported-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- 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%)