Re: [PATCH 04/10] ARM: ftrace: allow building without framepointers

[Rabin Vincent]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/