| Message ID | 20090320164429.GA28037@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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 <avorontsov@ru.mvista.com> > --- > 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 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 <avorontsov@ru.mvista.com> > > --- > > 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 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 <avorontsov@ru.mvista.com> > > > --- > > > 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 <avorontsov@ru.mvista.com> 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/linuxppc-dev@ozlabs.org/msg21222.html As stated, I did the sort without reporting the change in the change-log, which I should have done. -- Steve
On Sun, Apr 05, 2009 at 11:07:56PM +0200, Sam Ravnborg wrote: > 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 <avorontsov@ru.mvista.com> > > 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 OK, will rename. > 2) I do not really understand the interpretation of normal in this case, > can it be more specific? Normal means that architecture's ABI implies having a framepointer, we don't need any gcc flags. I'm not sure if we can name it any better, that is what was suggested by Steven and I don't have any better variant. ;-) > Looking at your patch then if specified we use "-fomit-frame-pointer" > in the case where it is set. No. We specify neither -fomit-frame-pointer, nor we specify -fno-omit-frame-pointer (because that flag causes gcc to generate wrong code). > 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 Well, yes. So looking into the Kconfig, I'm quite unsure where to put it to make it alphabetically correct. Ok, let's put it after K. ;-) Thanks!
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
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 <avorontsov@ru.mvista.com> --- Makefile | 7 +++++-- arch/powerpc/Kconfig | 1 + lib/Kconfig.debug | 16 ++++++++++------ 3 files changed, 16 insertions(+), 8 deletions(-)