diff mbox

[v2] ftrace: On PowerPC we don't need frame pointers for CALLER_ADDRs

Message ID 20090203145649.GA19955@oksana.dev.rtsoft.ru (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Anton Vorontsov Feb. 3, 2009, 2:56 p.m. UTC 
According to this discussion:

http://lkml.org/lkml/2008/7/25/338
http://lkml.org/lkml/2008/7/26/72

Frame pointers do nothing useful on PowerPC, so lib/Kconfig.debug
makes CONFIG_FRAME_POINTER unselectable on PPC targets. But ftrace.h
requires CONFIG_FRAME_POINTER for CALLER_ADDR macros. Therefore
tracing is completely useless on PowerPC:

[...]
  <idle>-0       0X.h3    2us+:      0:140:R   + [000]  1733:120:S mvtsd
  <idle>-0       0X.h3    9us+: 0 (0)
  <idle>-0       0X..3   72us : 0 (0)
  <idle>-0       0X..3   73us :      0:140:R ==> [000]  1733:120:R mvtsd

This patch introduces a ARCH_HAS_NORMAL_FRAME_POINTERS Kconfig symbol,
when selected the CALLER_ADDR macros are available without the
FRAME_POINTER Kconfig symbol.

With this patch the trace output turns into:

[...]
  <idle>-0       0X.h3    2us+:      0:140:R   + [000]  1740:120:S mvtsd
  <idle>-0       0X.h3    9us+: hrtimer_wakeup (__run_hrtimer)
  <idle>-0       0X..3   87us : cpu_idle (__got2_end)
  <idle>-0       0X..3   89us :      0:140:R ==> [000]  1740:120:R mvtsd

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---

On Mon, Feb 02, 2009 at 09:04:15AM -0500, Steven Rostedt wrote:
[...]
> > > -#ifdef CONFIG_FRAME_POINTER
> > > +#if defined(CONFIG_FRAME_POINTER) || defined(CONFIG_PPC)
> 
> Perhaps we should add a HAVE_NORMAL_FRAME_POINTERS in 
> arch/powerpc/Kconfig under PPC and then we can change the above line to:
> 
> #if defined(CONFIG_FRAME_POINTERS) || \
> 		defined(CONFIG_HAVE_NORMAL_FRAME_POINTERS)
> 
> This way when another arch wants to belong to this, we do not need to
> have a list of archs here.

Would it be better if we introduce ARCH_HAS_NORMAL_FRAME_POINTERS
in lib/Kconfig.debug, along with ARCH_WANT_FRAME_POINTERS?

Note that we can't use ARCH_WANT_FRAME_POINTERS for our needs since
that symbol is used for other (mostly cosmetic) purposes: whether we
we want CONFIG_FRAME_POINTER depend on CONFIG_DEBUG_KERNEL, and
whether frame pointers should be default =y (see commit
da4276b8299a6544dc41ac2485d3ffca5811b3fb).

 arch/powerpc/Kconfig   |    1 +
 include/linux/ftrace.h |    3 ++-
 lib/Kconfig.debug      |    6 ++++++
 3 files changed, 9 insertions(+), 1 deletions(-)

Comments

Ingo Molnar Feb. 3, 2009, 4:06 p.m. UTC | #1 
* Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> According to this discussion:
> 
> http://lkml.org/lkml/2008/7/25/338
> http://lkml.org/lkml/2008/7/26/72
> 
> Frame pointers do nothing useful on PowerPC, so lib/Kconfig.debug
> makes CONFIG_FRAME_POINTER unselectable on PPC targets. But ftrace.h
> requires CONFIG_FRAME_POINTER for CALLER_ADDR macros. [...]

hm, why not add PPC to FRAME_POINTERS list of architectures, and select it 
from the powerpc arch Kconfig? Does that cause complications somewhere?

	Ingo
Anton Vorontsov Feb. 3, 2009, 4:19 p.m. UTC | #2 
On Tue, Feb 03, 2009 at 05:06:45PM +0100, Ingo Molnar wrote:
> 
> * Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > According to this discussion:
> > 
> > http://lkml.org/lkml/2008/7/25/338
> > http://lkml.org/lkml/2008/7/26/72
> > 
> > Frame pointers do nothing useful on PowerPC, so lib/Kconfig.debug
> > makes CONFIG_FRAME_POINTER unselectable on PPC targets. But ftrace.h
> > requires CONFIG_FRAME_POINTER for CALLER_ADDR macros. [...]
> 
> hm, why not add PPC to FRAME_POINTERS list of architectures, and select it 
> from the powerpc arch Kconfig? Does that cause complications somewhere?

-fno-omit-frame-pointers makes the code worse w/o any actual
benefit that we would use. Plus, there is a long standing bug in
gcc that makes -fno-omit-frame-pointer generate wrong code for PPC
targets:

http://lkml.org/lkml/2008/9/2/25

That is, the only tracer that needs[1] -fno-omit-frame-pointer is
"FUNCTION_TRCER", but we workaround the issue via -mno-sched-epilog,
quoting arch/powerpc/Makefile:

# Work around a gcc code-gen bug with -fno-omit-frame-pointer.
ifeq ($(CONFIG_FUNCTION_TRACER),y)
KBUILD_CFLAGS           += -mno-sched-epilog
endif

[1] Btw, why exactly do we need the -fno-omit-frame-pointer for
"FUNCTION_TRCER" tracer? Why just -pg isn't sufficient?..
Steven Rostedt Feb. 3, 2009, 4:32 p.m. UTC | #3 
On Tue, 2009-02-03 at 19:19 +0300, Anton Vorontsov wrote:
> On Tue, Feb 03, 2009 at 05:06:45PM +0100, Ingo Molnar wrote:

> [1] Btw, why exactly do we need the -fno-omit-frame-pointer for
> "FUNCTION_TRCER" tracer? Why just -pg isn't sufficient?..
> 

The problem is this that is in the toplevel Makefile:


ifdef CONFIG_FRAME_POINTER
KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
else
KBUILD_CFLAGS	+= -fomit-frame-pointer
endif


-pg is incompatible with -fomit-frame-pointer

-- Steve
Anton Vorontsov Feb. 3, 2009, 6:59 p.m. UTC | #4 
On Tue, Feb 03, 2009 at 11:32:18AM -0500, Steven Rostedt wrote:
> 
> On Tue, 2009-02-03 at 19:19 +0300, Anton Vorontsov wrote:
> > On Tue, Feb 03, 2009 at 05:06:45PM +0100, Ingo Molnar wrote:
> 
> > [1] Btw, why exactly do we need the -fno-omit-frame-pointer for
> > "FUNCTION_TRCER" tracer? Why just -pg isn't sufficient?..
> > 
> 
> The problem is this that is in the toplevel Makefile:
> 
> 
> ifdef CONFIG_FRAME_POINTER
> KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
> else
> KBUILD_CFLAGS	+= -fomit-frame-pointer
> endif
> 
> 
> -pg is incompatible with -fomit-frame-pointer

Ah...

$ gcc -pg -fomit-frame-pointer -S c.c
gcc: -pg and -fomit-frame-pointer are incompatible

It's hard-coded in gcc, in the code that don't know about
architecture details.

But on PowerPC -O1 implies -fomit-frame-pointer, that is

gcc -pg -O1 -fno-omit-frame-pointer
and
gcc -pg -O1

produce different outputs. Thus -pg -O should be the same
as "-pg -O -fomit-framepointer".
Benjamin Herrenschmidt Feb. 4, 2009, 12:34 a.m. UTC | #5 
On Tue, 2009-02-03 at 21:59 +0300, Anton Vorontsov wrote:
> On Tue, Feb 03, 2009 at 11:32:18AM -0500, Steven Rostedt wrote:
> > 
> > On Tue, 2009-02-03 at 19:19 +0300, Anton Vorontsov wrote:
> > > On Tue, Feb 03, 2009 at 05:06:45PM +0100, Ingo Molnar wrote:
> > 
> > > [1] Btw, why exactly do we need the -fno-omit-frame-pointer for
> > > "FUNCTION_TRCER" tracer? Why just -pg isn't sufficient?..
> > > 
> > 
> > The problem is this that is in the toplevel Makefile:
> > 
> > 
> > ifdef CONFIG_FRAME_POINTER
> > KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
> > else
> > KBUILD_CFLAGS	+= -fomit-frame-pointer
> > endif
> > 

And don't forget the gcc bug that miscompiles function epilogues on ppc
with -fno-omit-frame-pointer (iirc), which we need to work around using
-mno-sched-epilog.

Currently, we set that only if CONFIG_FUNCTION_TRACER is set, ie, we
assume that we only set -fno-omit-frame-pointer when
CONFIG_FUNCTION_TRACER is set.

Ben.
Usha Rani Konudula Feb. 4, 2009, 8:17 a.m. UTC | #6 
unsubscribe linux-kernel
Usha Rani Konudula Feb. 4, 2009, 8:37 a.m. UTC | #7 
unsubscribe linux-kernel
Anton Vorontsov Feb. 4, 2009, 3:07 p.m. UTC | #8 
On Tue, Feb 03, 2009 at 07:19:55PM +0300, Anton Vorontsov wrote:
> On Tue, Feb 03, 2009 at 05:06:45PM +0100, Ingo Molnar wrote:
> > 
> > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> > 
> > > According to this discussion:
> > > 
> > > http://lkml.org/lkml/2008/7/25/338
> > > http://lkml.org/lkml/2008/7/26/72
> > > 
> > > Frame pointers do nothing useful on PowerPC, so lib/Kconfig.debug
> > > makes CONFIG_FRAME_POINTER unselectable on PPC targets. But ftrace.h
> > > requires CONFIG_FRAME_POINTER for CALLER_ADDR macros. [...]
> > 
> > hm, why not add PPC to FRAME_POINTERS list of architectures, and select it 
> > from the powerpc arch Kconfig? Does that cause complications somewhere?
> 
> -fno-omit-frame-pointers makes the code worse w/o any actual
> benefit that we would use. Plus, there is a long standing bug in
> gcc that makes -fno-omit-frame-pointer generate wrong code for PPC
> targets:
> 
> http://lkml.org/lkml/2008/9/2/25
> 
> That is, the only tracer that needs[1] -fno-omit-frame-pointer is
> "FUNCTION_TRCER", but we workaround the issue via -mno-sched-epilog,
> quoting arch/powerpc/Makefile:
> 
> # Work around a gcc code-gen bug with -fno-omit-frame-pointer.
> ifeq ($(CONFIG_FUNCTION_TRACER),y)
> KBUILD_CFLAGS           += -mno-sched-epilog
> endif

Thinking about it more... we can workaround the bug the other way,
and then permit CONFIG_FRAME_POINTER on PowerPC.

Patches are coming...
diff mbox

Patch

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 74cc312..d1c67bd 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -111,6 +111,7 @@  config PPC
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
+	select ARCH_HAS_NORMAL_FRAME_POINTERS
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select HAVE_IDE
 	select HAVE_IOREMAP_PROT
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7840e71..ede3fe2 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -237,7 +237,8 @@  static inline void __ftrace_enabled_restore(int enabled)
 #endif
 }
 
-#ifdef CONFIG_FRAME_POINTER
+#if defined(CONFIG_FRAME_POINTER) || \
+		defined(CONFIG_ARCH_HAS_NORMAL_FRAME_POINTERS)
 /* TODO: need to fix this for ARM */
 # define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
 # define CALLER_ADDR1 ((unsigned long)__builtin_return_address(1))
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 29044f5..808f4e2 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -579,6 +579,12 @@  config ARCH_WANT_FRAME_POINTERS
 	bool
 	help
 
+config ARCH_HAS_NORMAL_FRAME_POINTERS
+	bool
+	help
+	  Architectures should select this symbol if their ABI implies
+	  having a frame pointer.
+
 config FRAME_POINTER
 	bool "Compile the kernel with frame pointers"
 	depends on DEBUG_KERNEL && \