Hi all,
Here is another approach to fixing tracers vs. CALLER_ADDR problem
on PowerPC.
Preface for those who don't know or forgot what the problem is:
Gcc frame pointers do nothing useful on PowerPC (they're harmful,
actually), and thus lib/Kconfig.debug makes CONFIG_FRAME_POINTER
unselectable on PPC targets, but CALLER_ADDR macros are available
only with CONFIG_FRAME_POINTER, 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
While it should look like this:
[...]
<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
I've tried to fix the issue via expanding the #ifdef in the ftrace.h:
http://lkml.org/lkml/2009/1/31/141
Then Steven Rostedt suggested to implement something more generic,
i.e. HAVE_NORMAL_FRAME_POINTERS Kconfig symbol.
I found a way to solve the problem w/o additional symbols, but
with some Makefile magic (http://lkml.org/lkml/2009/2/4/273).
But because of top-level Makefile issues on other arches
(http://lkml.org/lkml/2009/2/14/89) I had to abandon the approach.
So, this patch set combines Steven Rostedt's idea and a small
Makefile change, so that now only top-level Makefile has to know
about the new symbol, and the rest of the kernel can stay with
using CONFIG_FRAME_POINTER.
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
This patch introduces ARCH_HAS_NORMAL_FRAME_POINTERS Kconfig symbol.
When defined, the top level Makefile won't add -fno-omit-frame-pointer
cflag (the flag is useless in PowerPC kernels, and also makes gcc
generate wrong code).
Also move ARCH_WANT_FRAME_POINTERS's help text.
Signed-off-by: Anton Vorontsov <[email protected]>
---
Makefile | 7 +++++--
arch/powerpc/Kconfig | 1 +
lib/Kconfig.debug | 16 ++++++++++------
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 46c04c5..bf41b05 100644
--- a/Makefile
+++ b/Makefile
@@ -538,9 +538,12 @@ KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
endif
ifdef CONFIG_FRAME_POINTER
-KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
+ KBUILD_CFLAGS += -fno-optimize-sibling-calls
+ ifndef ARCH_HAS_NORMAL_FRAME_POINTERS
+ KBUILD_CFLAGS += -fno-omit-frame-pointer
+ endif
else
-KBUILD_CFLAGS += -fomit-frame-pointer
+ KBUILD_CFLAGS += -fomit-frame-pointer
endif
ifdef CONFIG_DEBUG_INFO
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 97f9a64..4587e66 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -113,6 +113,7 @@ config PPC
select HAVE_FUNCTION_TRACER
select HAVE_FUNCTION_GRAPH_TRACER
select ARCH_WANT_OPTIONAL_GPIOLIB
+ select ARCH_HAS_NORMAL_FRAME_POINTERS
select HAVE_IDE
select HAVE_IOREMAP_PROT
select HAVE_EFFICIENT_UNALIGNED_ACCESS
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4b63b6b..fc8cd1f 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -661,20 +661,24 @@ config DEBUG_NOTIFIERS
This is a relatively cheap check but if you care about maximum
performance, say N.
-#
-# Select this config option from the architecture Kconfig, if it
-# it is preferred to always offer frame pointers as a config
-# option on the architecture (regardless of KERNEL_DEBUG):
-#
config ARCH_WANT_FRAME_POINTERS
bool
help
+ Select this config option from the architecture Kconfig, if it
+ it is preferred to always offer frame pointers as a config
+ option on the architecture (regardless of KERNEL_DEBUG).
+
+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 && \
(CRIS || M68K || M68KNOMMU || FRV || UML || S390 || \
- AVR32 || SUPERH || BLACKFIN || MN10300) || \
+ AVR32 || SUPERH || BLACKFIN || MN10300 || PPC) || \
ARCH_WANT_FRAME_POINTERS
default y if (DEBUG_INFO && UML) || ARCH_WANT_FRAME_POINTERS
help
--
1.5.6.5
The workarounds aren't needed any longer since the top level Makefile
doesn't pass -fno-omit-frame-pointer cflag for PowerPC.
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/powerpc/Makefile | 5 -----
arch/powerpc/kernel/Makefile | 12 ++++++------
arch/powerpc/platforms/powermac/Makefile | 2 +-
lib/Kconfig.debug | 6 +++---
4 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 551fc58..1dd7748 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -120,11 +120,6 @@ ifeq ($(CONFIG_6xx),y)
KBUILD_CFLAGS += -mcpu=powerpc
endif
-# Work around a gcc code-gen bug with -fno-omit-frame-pointer.
-ifeq ($(CONFIG_FUNCTION_TRACER),y)
-KBUILD_CFLAGS += -mno-sched-epilog
-endif
-
cpu-as-$(CONFIG_4xx) += -Wa,-m405
cpu-as-$(CONFIG_6xx) += -Wa,-maltivec
cpu-as-$(CONFIG_POWER4) += -Wa,-maltivec
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index dfec3d2..f86caeb 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -14,14 +14,14 @@ endif
ifdef CONFIG_FUNCTION_TRACER
# Do not trace early boot code
-CFLAGS_REMOVE_cputable.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_prom_init.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_btext.o = -pg -mno-sched-epilog
-CFLAGS_REMOVE_prom.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_cputable.o = -pg
+CFLAGS_REMOVE_prom_init.o = -pg
+CFLAGS_REMOVE_btext.o = -pg
+CFLAGS_REMOVE_prom.o = -pg
# do not trace tracer code
-CFLAGS_REMOVE_ftrace.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_ftrace.o = -pg
# timers used by tracing
-CFLAGS_REMOVE_time.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_time.o = -pg
endif
obj-y := cputable.o ptrace.o syscalls.o \
diff --git a/arch/powerpc/platforms/powermac/Makefile b/arch/powerpc/platforms/powermac/Makefile
index 50f1693..0eb8781 100644
--- a/arch/powerpc/platforms/powermac/Makefile
+++ b/arch/powerpc/platforms/powermac/Makefile
@@ -2,7 +2,7 @@ CFLAGS_bootx_init.o += -fPIC
ifdef CONFIG_FUNCTION_TRACER
# Do not trace early boot code
-CFLAGS_REMOVE_bootx_init.o = -pg -mno-sched-epilog
+CFLAGS_REMOVE_bootx_init.o = -pg
endif
obj-y += pic.o setup.o time.o feature.o pci.o \
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index fc8cd1f..713620d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -493,7 +493,7 @@ config LOCKDEP
bool
depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
select STACKTRACE
- select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND
+ select FRAME_POINTER if !MIPS && !ARM_UNWIND
select KALLSYMS
select KALLSYMS_ALL
@@ -866,13 +866,13 @@ config FAULT_INJECTION_STACKTRACE_FILTER
depends on FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT
depends on !X86_64
select STACKTRACE
- select FRAME_POINTER if !PPC
+ select FRAME_POINTER
help
Provide stacktrace filter for fault-injection capabilities
config LATENCYTOP
bool "Latency measuring infrastructure"
- select FRAME_POINTER if !MIPS && !PPC
+ select FRAME_POINTER if !MIPS
select KALLSYMS
select KALLSYMS_ALL
select STACKTRACE
--
1.5.6.5
Irqsoff, switch and preempt tracers use CALLER_ADDR macros, so they
should select FRAME_POINTER. Otherwise traces are meaningless.
Signed-off-by: Anton Vorontsov <[email protected]>
---
kernel/trace/Kconfig | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 774aba7..9fc98a7 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -107,6 +107,7 @@ config IRQSOFF_TRACER
select TRACE_IRQFLAGS
select TRACING
select TRACER_MAX_TRACE
+ select FRAME_POINTER
help
This option measures the time spent in irqs-off critical
sections, with microsecond accuracy.
@@ -128,6 +129,7 @@ config PREEMPT_TRACER
depends on PREEMPT
select TRACING
select TRACER_MAX_TRACE
+ select FRAME_POINTER
help
This option measures the time spent in preemption off critical
sections, with microsecond accuracy.
@@ -156,6 +158,7 @@ config SCHED_TRACER
select TRACING
select CONTEXT_SWITCH_TRACER
select TRACER_MAX_TRACE
+ select FRAME_POINTER
help
This tracer tracks the latency of the highest priority task
to be scheduled in, starting from the point it has woken up.
--
1.5.6.5
On Fri, Mar 20, 2009 at 07:44:04PM +0300, Anton Vorontsov wrote:
> Hi all,
>
> Here is another approach to fixing tracers vs. CALLER_ADDR problem
> on PowerPC.
>
> Preface for those who don't know or forgot what the problem is:
>
> Gcc frame pointers do nothing useful on PowerPC (they're harmful,
> actually), and thus lib/Kconfig.debug makes CONFIG_FRAME_POINTER
> unselectable on PPC targets, but CALLER_ADDR macros are available
> only with CONFIG_FRAME_POINTER, 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
>
> While it should look like this:
>
> [...]
> <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
>
> I've tried to fix the issue via expanding the #ifdef in the ftrace.h:
> http://lkml.org/lkml/2009/1/31/141
>
> Then Steven Rostedt suggested to implement something more generic,
> i.e. HAVE_NORMAL_FRAME_POINTERS Kconfig symbol.
>
> I found a way to solve the problem w/o additional symbols, but
> with some Makefile magic (http://lkml.org/lkml/2009/2/4/273).
> But because of top-level Makefile issues on other arches
> (http://lkml.org/lkml/2009/2/14/89) I had to abandon the approach.
Oh, and btw, I'm aware of
commit c79a61f55773d2519fd0525bf58385f7d20752d3
Author: Uwe Kleine-Koenig <[email protected]>
Date: Fri Feb 27 21:30:03 2009 +0100
tracing: make CALLER_ADDRx overwriteable
But I think the patch set is still applicable, considering that
it removes gcc bug workaround in a nice way, and makes
CONFIG_FRAME_POINTER available on PowerPC, thus other code
can rely on that.
If not, I can just fill-in the asm/ftrace.h for PowerPC.
Thanks,
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
Ben,
Can you ACK this patch? Or even take it in your tree?
Thanks,
-- Steve
On Fri, 2009-03-20 at 19:44 +0300, Anton Vorontsov wrote:
> This patch introduces ARCH_HAS_NORMAL_FRAME_POINTERS Kconfig symbol.
> When defined, the top level Makefile won't add -fno-omit-frame-pointer
> cflag (the flag is useless in PowerPC kernels, and also makes gcc
> generate wrong code).
>
> Also move ARCH_WANT_FRAME_POINTERS's help text.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
> Makefile | 7 +++++--
> arch/powerpc/Kconfig | 1 +
> lib/Kconfig.debug | 16 ++++++++++------
> 3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 46c04c5..bf41b05 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -538,9 +538,12 @@ KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
> endif
>
> ifdef CONFIG_FRAME_POINTER
> -KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> + KBUILD_CFLAGS += -fno-optimize-sibling-calls
> + ifndef ARCH_HAS_NORMAL_FRAME_POINTERS
> + KBUILD_CFLAGS += -fno-omit-frame-pointer
> + endif
> else
> -KBUILD_CFLAGS += -fomit-frame-pointer
> + KBUILD_CFLAGS += -fomit-frame-pointer
> endif
>
> ifdef CONFIG_DEBUG_INFO
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 97f9a64..4587e66 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -113,6 +113,7 @@ config PPC
> select HAVE_FUNCTION_TRACER
> select HAVE_FUNCTION_GRAPH_TRACER
> select ARCH_WANT_OPTIONAL_GPIOLIB
> + select ARCH_HAS_NORMAL_FRAME_POINTERS
> select HAVE_IDE
> select HAVE_IOREMAP_PROT
> select HAVE_EFFICIENT_UNALIGNED_ACCESS
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 4b63b6b..fc8cd1f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -661,20 +661,24 @@ config DEBUG_NOTIFIERS
> This is a relatively cheap check but if you care about maximum
> performance, say N.
>
> -#
> -# Select this config option from the architecture Kconfig, if it
> -# it is preferred to always offer frame pointers as a config
> -# option on the architecture (regardless of KERNEL_DEBUG):
> -#
> config ARCH_WANT_FRAME_POINTERS
> bool
> help
> + Select this config option from the architecture Kconfig, if it
> + it is preferred to always offer frame pointers as a config
> + option on the architecture (regardless of KERNEL_DEBUG).
> +
> +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 && \
> (CRIS || M68K || M68KNOMMU || FRV || UML || S390 || \
> - AVR32 || SUPERH || BLACKFIN || MN10300) || \
> + AVR32 || SUPERH || BLACKFIN || MN10300 || PPC) || \
> ARCH_WANT_FRAME_POINTERS
> default y if (DEBUG_INFO && UML) || ARCH_WANT_FRAME_POINTERS
> help
Ben,
Can you ACK or take this patch too.
Thanks,
-- Steve
On Fri, 2009-03-20 at 19:44 +0300, Anton Vorontsov wrote:
> The workarounds aren't needed any longer since the top level Makefile
> doesn't pass -fno-omit-frame-pointer cflag for PowerPC.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
> arch/powerpc/Makefile | 5 -----
> arch/powerpc/kernel/Makefile | 12 ++++++------
> arch/powerpc/platforms/powermac/Makefile | 2 +-
> lib/Kconfig.debug | 6 +++---
> 4 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 551fc58..1dd7748 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -120,11 +120,6 @@ ifeq ($(CONFIG_6xx),y)
> KBUILD_CFLAGS += -mcpu=powerpc
> endif
>
> -# Work around a gcc code-gen bug with -fno-omit-frame-pointer.
> -ifeq ($(CONFIG_FUNCTION_TRACER),y)
> -KBUILD_CFLAGS += -mno-sched-epilog
> -endif
> -
> cpu-as-$(CONFIG_4xx) += -Wa,-m405
> cpu-as-$(CONFIG_6xx) += -Wa,-maltivec
> cpu-as-$(CONFIG_POWER4) += -Wa,-maltivec
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index dfec3d2..f86caeb 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -14,14 +14,14 @@ endif
>
> ifdef CONFIG_FUNCTION_TRACER
> # Do not trace early boot code
> -CFLAGS_REMOVE_cputable.o = -pg -mno-sched-epilog
> -CFLAGS_REMOVE_prom_init.o = -pg -mno-sched-epilog
> -CFLAGS_REMOVE_btext.o = -pg -mno-sched-epilog
> -CFLAGS_REMOVE_prom.o = -pg -mno-sched-epilog
> +CFLAGS_REMOVE_cputable.o = -pg
> +CFLAGS_REMOVE_prom_init.o = -pg
> +CFLAGS_REMOVE_btext.o = -pg
> +CFLAGS_REMOVE_prom.o = -pg
> # do not trace tracer code
> -CFLAGS_REMOVE_ftrace.o = -pg -mno-sched-epilog
> +CFLAGS_REMOVE_ftrace.o = -pg
> # timers used by tracing
> -CFLAGS_REMOVE_time.o = -pg -mno-sched-epilog
> +CFLAGS_REMOVE_time.o = -pg
> endif
>
> obj-y := cputable.o ptrace.o syscalls.o \
> diff --git a/arch/powerpc/platforms/powermac/Makefile b/arch/powerpc/platforms/powermac/Makefile
> index 50f1693..0eb8781 100644
> --- a/arch/powerpc/platforms/powermac/Makefile
> +++ b/arch/powerpc/platforms/powermac/Makefile
> @@ -2,7 +2,7 @@ CFLAGS_bootx_init.o += -fPIC
>
> ifdef CONFIG_FUNCTION_TRACER
> # Do not trace early boot code
> -CFLAGS_REMOVE_bootx_init.o = -pg -mno-sched-epilog
> +CFLAGS_REMOVE_bootx_init.o = -pg
> endif
>
> obj-y += pic.o setup.o time.o feature.o pci.o \
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index fc8cd1f..713620d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -493,7 +493,7 @@ config LOCKDEP
> bool
> depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
> select STACKTRACE
> - select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND
> + select FRAME_POINTER if !MIPS && !ARM_UNWIND
> select KALLSYMS
> select KALLSYMS_ALL
>
> @@ -866,13 +866,13 @@ config FAULT_INJECTION_STACKTRACE_FILTER
> depends on FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT
> depends on !X86_64
> select STACKTRACE
> - select FRAME_POINTER if !PPC
> + select FRAME_POINTER
> help
> Provide stacktrace filter for fault-injection capabilities
>
> config LATENCYTOP
> bool "Latency measuring infrastructure"
> - select FRAME_POINTER if !MIPS && !PPC
> + select FRAME_POINTER if !MIPS
> select KALLSYMS
> select KALLSYMS_ALL
> select STACKTRACE
On Fri, Mar 20, 2009 at 11:49:29PM -0400, Steven Rostedt wrote:
> Ben,
>
> Can you ACK this patch? Or even take it in your tree?
Benjamin, have you had a chance to look into this? Sam, could
you also take a look?
Thanks!
> On Fri, 2009-03-20 at 19:44 +0300, Anton Vorontsov wrote:
> > This patch introduces ARCH_HAS_NORMAL_FRAME_POINTERS Kconfig symbol.
> > When defined, the top level Makefile won't add -fno-omit-frame-pointer
> > cflag (the flag is useless in PowerPC kernels, and also makes gcc
> > generate wrong code).
> >
> > Also move ARCH_WANT_FRAME_POINTERS's help text.
> >
> > Signed-off-by: Anton Vorontsov <[email protected]>
> > ---
> > Makefile | 7 +++++--
> > arch/powerpc/Kconfig | 1 +
> > lib/Kconfig.debug | 16 ++++++++++------
> > 3 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 46c04c5..bf41b05 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -538,9 +538,12 @@ KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
> > endif
> >
> > ifdef CONFIG_FRAME_POINTER
> > -KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> > + KBUILD_CFLAGS += -fno-optimize-sibling-calls
> > + ifndef ARCH_HAS_NORMAL_FRAME_POINTERS
> > + KBUILD_CFLAGS += -fno-omit-frame-pointer
> > + endif
> > else
> > -KBUILD_CFLAGS += -fomit-frame-pointer
> > + KBUILD_CFLAGS += -fomit-frame-pointer
> > endif
> >
> > ifdef CONFIG_DEBUG_INFO
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 97f9a64..4587e66 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -113,6 +113,7 @@ config PPC
> > select HAVE_FUNCTION_TRACER
> > select HAVE_FUNCTION_GRAPH_TRACER
> > select ARCH_WANT_OPTIONAL_GPIOLIB
> > + select ARCH_HAS_NORMAL_FRAME_POINTERS
> > select HAVE_IDE
> > select HAVE_IOREMAP_PROT
> > select HAVE_EFFICIENT_UNALIGNED_ACCESS
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 4b63b6b..fc8cd1f 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -661,20 +661,24 @@ config DEBUG_NOTIFIERS
> > This is a relatively cheap check but if you care about maximum
> > performance, say N.
> >
> > -#
> > -# Select this config option from the architecture Kconfig, if it
> > -# it is preferred to always offer frame pointers as a config
> > -# option on the architecture (regardless of KERNEL_DEBUG):
> > -#
> > config ARCH_WANT_FRAME_POINTERS
> > bool
> > help
> > + Select this config option from the architecture Kconfig, if it
> > + it is preferred to always offer frame pointers as a config
> > + option on the architecture (regardless of KERNEL_DEBUG).
> > +
> > +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 && \
> > (CRIS || M68K || M68KNOMMU || FRV || UML || S390 || \
> > - AVR32 || SUPERH || BLACKFIN || MN10300) || \
> > + AVR32 || SUPERH || BLACKFIN || MN10300 || PPC) || \
> > ARCH_WANT_FRAME_POINTERS
> > default y if (DEBUG_INFO && UML) || ARCH_WANT_FRAME_POINTERS
> > help
>
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Sat, 2009-03-28 at 13:48 +0300, Anton Vorontsov wrote:
> On Fri, Mar 20, 2009 at 11:49:29PM -0400, Steven Rostedt wrote:
> > Ben,
> >
> > Can you ACK this patch? Or even take it in your tree?
>
> Benjamin, have you had a chance to look into this? Sam, could
> you also take a look?
Those patches look ok. I'm a little bit surprised that -mno-sched-epilog
isn't needed, gcc weirdness never ceases to amaze me, but the
Makefile/Kconfig churning isn't for me to judge. If Sam is happy, then
let's merge it.
Ben.
> Thanks!
>
> > On Fri, 2009-03-20 at 19:44 +0300, Anton Vorontsov wrote:
> > > This patch introduces ARCH_HAS_NORMAL_FRAME_POINTERS Kconfig symbol.
> > > When defined, the top level Makefile won't add -fno-omit-frame-pointer
> > > cflag (the flag is useless in PowerPC kernels, and also makes gcc
> > > generate wrong code).
> > >
> > > Also move ARCH_WANT_FRAME_POINTERS's help text.
> > >
> > > Signed-off-by: Anton Vorontsov <[email protected]>
> > > ---
> > > Makefile | 7 +++++--
> > > arch/powerpc/Kconfig | 1 +
> > > lib/Kconfig.debug | 16 ++++++++++------
> > > 3 files changed, 16 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 46c04c5..bf41b05 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -538,9 +538,12 @@ KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
> > > endif
> > >
> > > ifdef CONFIG_FRAME_POINTER
> > > -KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> > > + KBUILD_CFLAGS += -fno-optimize-sibling-calls
> > > + ifndef ARCH_HAS_NORMAL_FRAME_POINTERS
> > > + KBUILD_CFLAGS += -fno-omit-frame-pointer
> > > + endif
> > > else
> > > -KBUILD_CFLAGS += -fomit-frame-pointer
> > > + KBUILD_CFLAGS += -fomit-frame-pointer
> > > endif
> > >
> > > ifdef CONFIG_DEBUG_INFO
> > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > > index 97f9a64..4587e66 100644
> > > --- a/arch/powerpc/Kconfig
> > > +++ b/arch/powerpc/Kconfig
> > > @@ -113,6 +113,7 @@ config PPC
> > > select HAVE_FUNCTION_TRACER
> > > select HAVE_FUNCTION_GRAPH_TRACER
> > > select ARCH_WANT_OPTIONAL_GPIOLIB
> > > + select ARCH_HAS_NORMAL_FRAME_POINTERS
> > > select HAVE_IDE
> > > select HAVE_IOREMAP_PROT
> > > select HAVE_EFFICIENT_UNALIGNED_ACCESS
> > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > > index 4b63b6b..fc8cd1f 100644
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -661,20 +661,24 @@ config DEBUG_NOTIFIERS
> > > This is a relatively cheap check but if you care about maximum
> > > performance, say N.
> > >
> > > -#
> > > -# Select this config option from the architecture Kconfig, if it
> > > -# it is preferred to always offer frame pointers as a config
> > > -# option on the architecture (regardless of KERNEL_DEBUG):
> > > -#
> > > config ARCH_WANT_FRAME_POINTERS
> > > bool
> > > help
> > > + Select this config option from the architecture Kconfig, if it
> > > + it is preferred to always offer frame pointers as a config
> > > + option on the architecture (regardless of KERNEL_DEBUG).
> > > +
> > > +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 && \
> > > (CRIS || M68K || M68KNOMMU || FRV || UML || S390 || \
> > > - AVR32 || SUPERH || BLACKFIN || MN10300) || \
> > > + AVR32 || SUPERH || BLACKFIN || MN10300 || PPC) || \
> > > ARCH_WANT_FRAME_POINTERS
> > > default y if (DEBUG_INFO && UML) || ARCH_WANT_FRAME_POINTERS
> > > help
> >
>
On Mon, Mar 30, 2009 at 09:54:50AM +1100, Benjamin Herrenschmidt wrote:
> On Sat, 2009-03-28 at 13:48 +0300, Anton Vorontsov wrote:
> > On Fri, Mar 20, 2009 at 11:49:29PM -0400, Steven Rostedt wrote:
> > > Ben,
> > >
> > > Can you ACK this patch? Or even take it in your tree?
> >
> > Benjamin, have you had a chance to look into this? Sam, could
> > you also take a look?
>
> Those patches look ok. I'm a little bit surprised that -mno-sched-epilog
> isn't needed, gcc weirdness never ceases to amaze me, but the
> Makefile/Kconfig churning isn't for me to judge. If Sam is happy, then
> let's merge it.
I will take a closer look tonight or tomorrow - as day-time job permits.
Sam
On Fri, Mar 20, 2009 at 07:44:29PM +0300, Anton Vorontsov wrote:
> This patch introduces ARCH_HAS_NORMAL_FRAME_POINTERS Kconfig symbol.
> When defined, the top level Makefile won't add -fno-omit-frame-pointer
> cflag (the flag is useless in PowerPC kernels, and also makes gcc
> generate wrong code).
>
> Also move ARCH_WANT_FRAME_POINTERS's help text.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
Hi Anton - sorry for the late feedback.
1) The preferred naming for variables that are supposed to be selcted
are "HAVE_xxxx".
So in this case it would be:
HAVE_NORMAL_FRAME_POINTER
2) I do not really understand the interpretation of normal in this case,
can it be more specific?
Looking at your patch then if specified we use "-fomit-frame-pointer"
in the case where it is set.
So I read it like:
If arch has normal framepointer then we omit them - strange?
3) Indent in top-level MAkefile for new stuff is generally 8 spaces.
Do NOT use tabs as this would confuse make and rener assignmnet non-functional.
4) The individual "HAVE_* members should be sorted alphabetically in the arch
Kconfig file (which they seldomly are :-( )
Sam
> ---
> Makefile | 7 +++++--
> arch/powerpc/Kconfig | 1 +
> lib/Kconfig.debug | 16 ++++++++++------
> 3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 46c04c5..bf41b05 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -538,9 +538,12 @@ KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
> endif
>
> ifdef CONFIG_FRAME_POINTER
> -KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> + KBUILD_CFLAGS += -fno-optimize-sibling-calls
> + ifndef ARCH_HAS_NORMAL_FRAME_POINTERS
> + KBUILD_CFLAGS += -fno-omit-frame-pointer
> + endif
> else
> -KBUILD_CFLAGS += -fomit-frame-pointer
> + KBUILD_CFLAGS += -fomit-frame-pointer
> endif
>
> ifdef CONFIG_DEBUG_INFO
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 97f9a64..4587e66 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -113,6 +113,7 @@ config PPC
> select HAVE_FUNCTION_TRACER
> select HAVE_FUNCTION_GRAPH_TRACER
> select ARCH_WANT_OPTIONAL_GPIOLIB
> + select ARCH_HAS_NORMAL_FRAME_POINTERS
> select HAVE_IDE
> select HAVE_IOREMAP_PROT
> select HAVE_EFFICIENT_UNALIGNED_ACCESS
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 4b63b6b..fc8cd1f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -661,20 +661,24 @@ config DEBUG_NOTIFIERS
> This is a relatively cheap check but if you care about maximum
> performance, say N.
>
> -#
> -# Select this config option from the architecture Kconfig, if it
> -# it is preferred to always offer frame pointers as a config
> -# option on the architecture (regardless of KERNEL_DEBUG):
> -#
> config ARCH_WANT_FRAME_POINTERS
> bool
> help
> + Select this config option from the architecture Kconfig, if it
> + it is preferred to always offer frame pointers as a config
> + option on the architecture (regardless of KERNEL_DEBUG).
> +
> +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 && \
> (CRIS || M68K || M68KNOMMU || FRV || UML || S390 || \
> - AVR32 || SUPERH || BLACKFIN || MN10300) || \
> + AVR32 || SUPERH || BLACKFIN || MN10300 || PPC) || \
> ARCH_WANT_FRAME_POINTERS
> default y if (DEBUG_INFO && UML) || ARCH_WANT_FRAME_POINTERS
> help
> --
> 1.5.6.5
>
On Sun, 2009-04-05 at 23:07 +0200, Sam Ravnborg wrote:
> 4) The individual "HAVE_* members should be sorted alphabetically in the arch
> Kconfig file (which they seldomly are :-( )
Note, my first port of Ftrace to PPC sorted the Kconfig HAVE_* macros,
which ended up being reverted :-/
http://www.mail-archive.com/[email protected]/msg21222.html
As stated, I did the sort without reporting the change in the
change-log, which I should have done.
-- Steve