[RESEND] arm64: fix vdso-offsets.h dependency

On Thu, May 12, 2016 at 05:39:15PM +0100, Kevin Brodsky wrote:
> I am not completely satisfied with the fix, since it uses a hack with

> the prepare and prepare0 rules that should not be used in arch

> Makefiles. However, all of my other attempts (including explicit

> dependencies on gettimeofday.S, etc. in arm64/kernel/Makefile) failed

> in some way. Hopefully, a Makefile wizard will come up with a better

> solution.


This is the patch I'm going to push to arm64 for-next/core. Thanks for
the report and attempt at fixing it, it saved me from trying to
understand what was going on:

-------------8<------------------------------------
From 19a5ab422b6ca3b3c8f656ca6d697dbfb577d360 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>

Date: Fri, 8 Jul 2016 12:13:20 +0100
Subject: [PATCH] arm64: Fix vdso-offsets.h dependency

arch/arm64/kernel/{vdso,signal}.c include generated/vdso-offsets.h, and
therefore the symbol offsets must be generated before these files are
compiled.

The current rules in arm64/kernel/Makefile do not actually enforce
this, because even though $(obj)/vdso is listed as a prerequisite for
vdso-offsets.h, this does not result in the intended effect of
building the vdso subdirectory (before all the other objects). As a
consequence, depending on the order in which the rules are followed,
vdso-offsets.h is updated or not before arm64/kernel/{vdso,signal}.o
are built. The current rules also impose an unnecessary dependency on
vdso-offsets.h for all arm64/kernel/*.o, resulting in unnecessary
rebuilds.

This patch removes the arch/arm64/kernel/vdso/vdso-offsets.h file
generation, leaving only the include/generated/vdso-offsets.h one. It
adds a forced dependency check of the vdso-offsets.h file in
arch/arm64/kernel/Makefile which, if not up to date according to the
arch/arm64/kernel/vdso/Makefile rules (depending on vdso.so.dbg), will
trigger the vdso/ subdirectory build and vdso-offsets.h re-generation.
Automatic kbuild dependency rules between kernel/{vdso,signal}.c rules
and vdso-offsets.h will guarantee that the vDSO object is built first,
followed by the generated symbol offsets header file.

Reported-by: Kevin Brodsky <kevin.brodsky@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

---
 arch/arm64/kernel/Makefile      | 7 ++++---
 arch/arm64/kernel/vdso/Makefile | 7 +++----
 2 files changed, 7 insertions(+), 7 deletions(-)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Mon, Jul 11, 2016 at 04:29:26PM +0100, Kevin Brodsky wrote:
> On 08/07/16 12:27, Catalin Marinas wrote:

> >On Thu, May 12, 2016 at 05:39:15PM +0100, Kevin Brodsky wrote:

> >>I am not completely satisfied with the fix, since it uses a hack with

> >>the prepare and prepare0 rules that should not be used in arch

> >>Makefiles. However, all of my other attempts (including explicit

> >>dependencies on gettimeofday.S, etc. in arm64/kernel/Makefile) failed

> >>in some way. Hopefully, a Makefile wizard will come up with a better

> >>solution.

> >This is the patch I'm going to push to arm64 for-next/core. Thanks for

> >the report and attempt at fixing it, it saved me from trying to

> >understand what was going on:

> 

> First, thanks for taking care of this! Sorry for the delay in replying, I've been

> having trouble recently with my email client not showing up new messages in subfolders...

> 

> Now, unfortunately, I had already tried this solution (I think almost exactly this

> patch in fact), and it does not work. I confirmed this just now by applying the patch

> on master and compiling from a clean tree.The compilation of signal.c failed with:


I noticed this as well after an mrproper. The code seemed to be compiled
in order as long as there was an original generated/asm-offsets.h in
place.

> Therefore, please do not merge this patch, it can break the compilation quite easily.


Too late ;). But I'm reverting it now.

> > This indeed looks dodgy. I'm not sure about the makefile rules but would the above

> > override the "prepare" target in the top Makefile?

> 

> Rules are cumulative, they do not override each other. I am only making

> "vdso_prepare" an additional prerequisite of "prepare", with "vdso_prepare" depending

> on "prepare0" to ensure that asm-offsets.h is generated first. What is dodgy is that

> we are not supposed to add prerequisites to "prepare" in arch Makefiles, but again, I

> don't see how we can avoid doing that here. It seems to me that this is an oversight

> in the top-level Makefile, and I don't think that adding a prerequisite to "prepare"

> is unreasonable.


I'll merge your patch. An alternative would be to parse the vdso ELF at
run-time in the kernel and generate the offsets.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel