All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Steven Rostedt <srostedt@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>, Paul Mackerras <paulus@samba.org>,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org
Subject: [PATCH v2] ftrace: On PowerPC we don't need frame pointers for CALLER_ADDRs
Date: Tue, 3 Feb 2009 17:56:49 +0300	[thread overview]
Message-ID: <20090203145649.GA19955@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <1233583455.16878.3.camel@localhost.localdomain>

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(-)

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 && \
-- 
1.5.6.5

WARNING: multiple messages have this Message-ID (diff)
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Steven Rostedt <srostedt@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Ingo Molnar <mingo@elte.hu>, Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] ftrace: On PowerPC we don't need frame pointers for CALLER_ADDRs
Date: Tue, 3 Feb 2009 17:56:49 +0300	[thread overview]
Message-ID: <20090203145649.GA19955@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <1233583455.16878.3.camel@localhost.localdomain>

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(-)

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 && \
-- 
1.5.6.5


  reply	other threads:[~2009-02-03 14:56 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-31 19:06 [PATCH] ftrace: On PowerPC we don't need frame pointers for CALLER_ADDRs Anton Vorontsov
2009-01-31 19:06 ` Anton Vorontsov
2009-02-02  0:35 ` Benjamin Herrenschmidt
2009-02-02  0:35   ` Benjamin Herrenschmidt
2009-02-02 14:04   ` Steven Rostedt
2009-02-02 14:04     ` Steven Rostedt
2009-02-03 14:56     ` Anton Vorontsov [this message]
2009-02-03 14:56       ` [PATCH v2] " Anton Vorontsov
2009-02-03 16:06       ` Ingo Molnar
2009-02-03 16:06         ` Ingo Molnar
2009-02-03 16:19         ` Anton Vorontsov
2009-02-03 16:19           ` Anton Vorontsov
2009-02-03 16:32           ` Steven Rostedt
2009-02-03 16:32             ` Steven Rostedt
2009-02-03 18:59             ` Anton Vorontsov
2009-02-03 18:59               ` Anton Vorontsov
2009-02-04  0:34               ` Benjamin Herrenschmidt
2009-02-04  0:34                 ` Benjamin Herrenschmidt
2009-02-04 15:07           ` Anton Vorontsov
2009-02-04 15:07             ` Anton Vorontsov
2009-02-04 15:08             ` [PATCH 1/3] Makefile: Include arch Makefiles as late as possible Anton Vorontsov
2009-02-04 15:08               ` Anton Vorontsov
2009-02-04 15:08               ` Anton Vorontsov
2009-02-04 21:26               ` Ingo Molnar
2009-02-04 21:26               ` Ingo Molnar
2009-02-04 21:26               ` Ingo Molnar
2009-02-04 21:26               ` Ingo Molnar
2009-02-04 21:26                 ` Ingo Molnar
2009-02-11  3:51                 ` Benjamin Herrenschmidt
2009-02-11  3:51                 ` Benjamin Herrenschmidt
2009-02-11  3:51                 ` Benjamin Herrenschmidt
2009-02-11  3:51                   ` Benjamin Herrenschmidt
2009-02-11 13:23                   ` Ingo Molnar
2009-02-11 13:23                     ` Ingo Molnar
2009-02-11 14:11                     ` Steven Rostedt
2009-02-11 14:11                     ` Steven Rostedt
2009-02-11 14:11                       ` Steven Rostedt
2009-02-11 14:11                     ` Steven Rostedt
2009-02-11 14:11                     ` Steven Rostedt
2009-02-14 19:58                   ` Sam Ravnborg
2009-02-14 19:58                     ` Sam Ravnborg
2009-02-11  3:51                 ` Benjamin Herrenschmidt
2009-02-14 19:57                 ` Sam Ravnborg
2009-02-14 19:57                   ` Sam Ravnborg
2009-02-14 22:03                   ` Ingo Molnar
2009-02-14 22:03                     ` Ingo Molnar
2009-02-15  0:19                     ` Benjamin Herrenschmidt
2009-02-15  0:19                       ` Benjamin Herrenschmidt
2009-02-15  8:09                       ` Ingo Molnar
2009-02-15  8:09                         ` Ingo Molnar
2009-02-15  0:19                     ` Benjamin Herrenschmidt
2009-02-15  0:19                     ` Benjamin Herrenschmidt
2009-02-15  0:19                     ` Benjamin Herrenschmidt
2009-02-16 14:20                   ` Anton Vorontsov
2009-02-16 14:20                     ` Anton Vorontsov
2009-02-16 14:20                     ` Anton Vorontsov
2009-02-16 14:53                     ` Anton Vorontsov
2009-02-16 14:53                       ` Anton Vorontsov
2009-02-16 20:04                       ` Sam Ravnborg
2009-02-16 20:04                         ` Sam Ravnborg
2009-02-16 16:08                     ` Anton Vorontsov
2009-02-16 16:08                       ` Anton Vorontsov
2009-02-16 17:22                       ` Ingo Molnar
2009-02-16 17:22                         ` Ingo Molnar
2009-02-04 15:08             ` [PATCH 2/3] powerpc: Make it possible to safely select CONFIG_FRAME_POINTER Anton Vorontsov
2009-02-04 15:08               ` Anton Vorontsov
2009-02-05  0:31               ` Benjamin Herrenschmidt
2009-02-05  0:31               ` Benjamin Herrenschmidt
2009-02-05  0:31                 ` Benjamin Herrenschmidt
2009-02-05  0:35                 ` Steven Rostedt
2009-02-05  0:35                   ` Steven Rostedt
2009-02-05  1:12                   ` Anton Vorontsov
2009-02-05  1:12                     ` Anton Vorontsov
2009-02-05  1:15                   ` Benjamin Herrenschmidt
2009-02-05  1:15                   ` Benjamin Herrenschmidt
2009-02-05  1:15                     ` Benjamin Herrenschmidt
2009-02-05  1:15                     ` Benjamin Herrenschmidt
2009-02-05  1:30                     ` Anton Vorontsov
2009-02-05  1:30                       ` Anton Vorontsov
2009-02-05 15:45                       ` Anton Vorontsov
2009-02-05 15:45                         ` Anton Vorontsov
2009-02-05  1:15                   ` Benjamin Herrenschmidt
2009-02-05  1:15                   ` Benjamin Herrenschmidt
2009-02-05  0:31               ` Benjamin Herrenschmidt
2009-02-05  0:31               ` Benjamin Herrenschmidt
2009-02-04 15:08             ` [PATCH 3/3] tracing: Tracers that use CALLER_ADDR macros should select FRAME_POINTER Anton Vorontsov
2009-02-04 15:08               ` Anton Vorontsov
2009-02-04 15:26               ` Frédéric Weisbecker
2009-02-04 15:26                 ` Frédéric Weisbecker
2009-02-04 15:26                 ` Frédéric Weisbecker
2009-02-04 15:31                 ` Steven Rostedt
2009-02-04 15:31                 ` Steven Rostedt
2009-02-04 15:31                 ` Steven Rostedt
2009-02-04 15:31                   ` Steven Rostedt
2009-02-04 15:31                   ` Steven Rostedt
2009-02-04 15:31                 ` Steven Rostedt
2009-02-04 15:36                 ` Anton Vorontsov
2009-02-04 15:36                   ` Anton Vorontsov
2009-02-04 15:36                   ` Anton Vorontsov
2009-02-04 16:50                   ` Frédéric Weisbecker
2009-02-04 16:50                     ` Frédéric Weisbecker
2009-02-04 16:50                     ` Frédéric Weisbecker
2009-02-04  8:17       ` [PATCH v2] ftrace: On PowerPC we don't need frame pointers forCALLER_ADDRs Usha Rani Konudula
2009-02-04  8:17         ` Usha Rani Konudula
2009-02-04  8:37         ` Usha Rani Konudula
2009-02-04  8:37           ` Usha Rani Konudula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090203145649.GA19955@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=srostedt@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.