ARM: ftrace: allow building without frame pointers
in [Kernel]
|
Prev: [PATCH 3/6] ldisc n_tty: add new method n_tty_inherit_ops()
Next: ftrace: pass KBUILD_CFLAGS to record_mcount.pl
From: Steven Rostedt on 23 Feb 2010 08:30 On Mon, 2010-02-22 at 20:05 +0100, Frederic Weisbecker wrote: > > Btw, you described that mcount is used in some gcc versions and > __gnu_mcount_nc in newers. > Shouldn't we have a gcc version dependency here? (not sure we can > do this from Kconfig though). > Maybe we can add something to recordmcount.pl? > > > > config DEBUG_USER > > bool "Verbose user fault messages" > > help > > diff --git a/arch/arm/kernel/armksyms.c b/arch/arm/kernel/armksyms.c > > index 8214bfe..e5e1e53 100644 > > --- a/arch/arm/kernel/armksyms.c > > +++ b/arch/arm/kernel/armksyms.c > > @@ -165,6 +165,8 @@ EXPORT_SYMBOL(_find_next_bit_be); > > #endif > > > > #ifdef CONFIG_FUNCTION_TRACER > > +#ifdef CONFIG_OLD_MCOUNT > > EXPORT_SYMBOL(mcount); > > +#endif > > EXPORT_SYMBOL(__gnu_mcount_nc); > > #endif > > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S > > index d412d7c..c98e3a3 100644 > > --- a/arch/arm/kernel/entry-common.S > > +++ b/arch/arm/kernel/entry-common.S > > @@ -169,6 +169,12 @@ gnu_trace: > > ldmia sp!, {r0-r3, ip, lr} > > mov pc, ip > > > > +#ifdef CONFIG_OLD_MCOUNT > > +/* > > + * This is under an ifdef in order to force link-time errors for people trying > > + * to build with !FRAME_POINTER with a GCC which doesn't use the new-style > > + * mcount. > > + */ > > ENTRY(mcount) > > stmdb sp!, {r0-r3, lr} > > ldr r0, =ftrace_trace_function > > @@ -187,6 +193,7 @@ trace: > > mov pc, r2 > > ldr lr, [fp, #-4] @ restore lr > > ldmia sp!, {r0-r3, pc} > > +#endif > > > > #endif /* CONFIG_DYNAMIC_FTRACE */ > > > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > > index 6c22d8a..7468ffe 100644 > > --- a/kernel/trace/Kconfig > > +++ b/kernel/trace/Kconfig > > @@ -126,7 +126,7 @@ if FTRACE > > config FUNCTION_TRACER > > bool "Kernel Function Tracer" > > depends on HAVE_FUNCTION_TRACER > > - select FRAME_POINTER > > + select FRAME_POINTER if (!ARM_UNWIND) > > > So, if I understand well. If people have ARM_UNWIND but > FUNCTION_TRACER, it might crash at link time in case > they don't have a recent enough gcc version to > support the new -pg style? > > That doesn't look good. > > Ideally, we need HAVE_FUNCTION_TRACER to be enabled in arm > only if (old gcc && frame pointers) || new gcc > > And then, we need config OLD_MCOUNT: > old gcc && FUNCTION_TRACER > > and config NEW_MCOUNT: > new gcc && FUNCTION_TRACER > > so that we can selectively build mcount or __gnu_mcount_nc. > > Hmm, I fear we can't check gcc version from Kconfig, as I'm > grepping on Kconfig files... I would not check gcc version through KCONFIG, but you can have the gcc version checked and define another macro in a header somewhere, like asm/ftrace.h. Then we could do something different while it is compiling. Disclaimer: I have to go back and read the entire thread, to know what exactly is going on ;-) -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Steven Rostedt on 23 Feb 2010 08:50 On Sun, 2010-02-14 at 01:18 +0530, Rabin Vincent wrote: > With a new enough GCC, ARM function tracing can be supported without the > need for frame pointers. This is essential for Thumb-2 support, since > frame pointers aren't available then. > > Signed-off-by: Rabin Vincent <rabin(a)rab.in> > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 6c22d8a..7468ffe 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -126,7 +126,7 @@ if FTRACE > config FUNCTION_TRACER > bool "Kernel Function Tracer" > depends on HAVE_FUNCTION_TRACER > - select FRAME_POINTER > + select FRAME_POINTER if (!ARM_UNWIND) > select KALLSYMS > select GENERIC_TRACER > select CONTEXT_SWITCH_TRACER Ah, evolution is keeping these patches out of order, so I did not see this change before commenting about the change to the Makefile. A better comment is needed in the Makefile. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rabin Vincent on 23 Feb 2010 13:00 On Tue, Feb 23, 2010 at 06:11:16PM +0100, Frederic Weisbecker wrote: > On Tue, Feb 23, 2010 at 08:18:33AM -0500, Steven Rostedt wrote: > > On Mon, 2010-02-22 at 20:05 +0100, Frederic Weisbecker wrote: > > > > > > Btw, you described that mcount is used in some gcc versions and > > > __gnu_mcount_nc in newers. > > > Shouldn't we have a gcc version dependency here? (not sure we can > > > do this from Kconfig though). > > > > > Maybe we can add something to recordmcount.pl? > > May be yeah, with an explicit message that describes the issue. Adding something to recordmcount.pl to detect this would be insufficient, because this affects non-dynamic ftrace too. > > > > +++ b/arch/arm/kernel/entry-common.S > > > > @@ -169,6 +169,12 @@ gnu_trace: > > > > ldmia sp!, {r0-r3, ip, lr} > > > > mov pc, ip > > > > > > > > +#ifdef CONFIG_OLD_MCOUNT > > > > +/* > > > > + * This is under an ifdef in order to force link-time errors for people trying > > > > + * to build with !FRAME_POINTER with a GCC which doesn't use the new-style > > > > + * mcount. > > > > + */ > > > > ENTRY(mcount) > > > > stmdb sp!, {r0-r3, lr} > > > > ldr r0, =ftrace_trace_function > > > > @@ -187,6 +193,7 @@ trace: > > > > mov pc, r2 > > > > ldr lr, [fp, #-4] @ restore lr > > > > ldmia sp!, {r0-r3, pc} > > > > +#endif > > > > > > > > #endif /* CONFIG_DYNAMIC_FTRACE */ > > > > > > > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > > > > index 6c22d8a..7468ffe 100644 > > > > --- a/kernel/trace/Kconfig > > > > +++ b/kernel/trace/Kconfig > > > > @@ -126,7 +126,7 @@ if FTRACE > > > > config FUNCTION_TRACER > > > > bool "Kernel Function Tracer" > > > > depends on HAVE_FUNCTION_TRACER > > > > - select FRAME_POINTER > > > > + select FRAME_POINTER if (!ARM_UNWIND) > > > > > > > > > So, if I understand well. If people have ARM_UNWIND but > > > FUNCTION_TRACER, it might crash at link time in case > > > they don't have a recent enough gcc version to > > > support the new -pg style? They'd probably get an "undefined reference to mcount" message. I forced the link time error because otherwise they'll have runtime problems. > > > > > > That doesn't look good. > > > > > > Ideally, we need HAVE_FUNCTION_TRACER to be enabled in arm > > > only if (old gcc && frame pointers) || new gcc > > > > > > And then, we need config OLD_MCOUNT: > > > old gcc && FUNCTION_TRACER > > > > > > and config NEW_MCOUNT: > > > new gcc && FUNCTION_TRACER > > > > > > so that we can selectively build mcount or __gnu_mcount_nc. There's more to this. When we build with FRAME_POINTER, we need to always build in both version of mcount. This is to allow modules to be built with different versions of the compiler (one which uses the old mcount and one with the new mcount). > > > > > > Hmm, I fear we can't check gcc version from Kconfig, as I'm > > > grepping on Kconfig files... > > > > > > I would not check gcc version through KCONFIG, but you can have the gcc > > version checked and define another macro in a header somewhere, like > > asm/ftrace.h. Then we could do something different while it is > > compiling. > > Yeah. I understand now why we can't do that on Kconfig time, HOSTCC > can be different than CC. > > The most important thing is to have a message that describes the issue > if the compiler-config combination is impossible. The easiest option to have a more obvious message than a linker error would be to add something like this in entry-common.S: #if (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4)) \ && !defined(OLD_MCOUNT) #warning Ftrace on GCC < 4.4 requires frame pointers #endif Messages like this are also present in other places in ARM (for example arch/arm/kernel/unwind.c), so this wouldn't be this first. Note that the above message would help in all cases except the one where someone builds the kernel with !fp and GCC 4.4+, and then builds a module with an older GCC. That would still be only a linker error. Rabin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Steven Rostedt on 23 Feb 2010 13:10 On Tue, 2010-02-23 at 23:28 +0530, Rabin Vincent wrote: > The easiest option to have a more obvious message than a linker error > would be to add something like this in entry-common.S: > > #if (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4)) \ > && !defined(OLD_MCOUNT) > #warning Ftrace on GCC < 4.4 requires frame pointers > #endif > > Messages like this are also present in other places in ARM (for example > arch/arm/kernel/unwind.c), so this wouldn't be this first. > > Note that the above message would help in all cases except the one where > someone builds the kernel with !fp and GCC 4.4+, and then builds a > module with an older GCC. That would still be only a linker error. I was going to recommend the #if above. But shouldn't it be a #error instead of a #warning? -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rabin Vincent on 23 Feb 2010 13:40
On Tue, Feb 23, 2010 at 01:03:05PM -0500, Steven Rostedt wrote: > On Tue, 2010-02-23 at 23:28 +0530, Rabin Vincent wrote: > > The easiest option to have a more obvious message than a linker error > > would be to add something like this in entry-common.S: > > > > #if (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4)) \ > > && !defined(OLD_MCOUNT) > > #warning Ftrace on GCC < 4.4 requires frame pointers > > #endif > > > > Messages like this are also present in other places in ARM (for example > > arch/arm/kernel/unwind.c), so this wouldn't be this first. > > > > Note that the above message would help in all cases except the one where > > someone builds the kernel with !fp and GCC 4.4+, and then builds a > > module with an older GCC. That would still be only a linker error. > > I was going to recommend the #if above. But shouldn't it be a #error > instead of a #warning? Yes, I'll make it a #error. (I had it at #warning in case some one had a custom compiler that had the new mcount patch backported, and chose to ignore the warning. But it's probably better to leave it at #error and see if someone actually complains about it, rather than have someone miss the message with it as a #warning.) Rabin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |