| Message ID | 20090204150835.GA30027@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
* Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > This patch gives arches more freedom on overwriting CFLAGS, specifically > on PowerPC we want to remove -fno-omit-frame-pointer flag. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > --- > Makefile | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 7715b2c..d1ba93f 100644 > --- a/Makefile > +++ b/Makefile > @@ -525,8 +525,6 @@ else > KBUILD_CFLAGS += -O2 > endif > > -include $(srctree)/arch/$(SRCARCH)/Makefile > - > ifneq (CONFIG_FRAME_WARN,0) > KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN}) > endif > @@ -555,6 +553,8 @@ ifdef CONFIG_DEBUG_SECTION_MISMATCH > KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once) > endif > > +include $(srctree)/arch/$(SRCARCH)/Makefile > + > # arch Makefile may override CC so keep this after arch Makefile is included > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) > CHECKFLAGS += $(NOSTDINC_FLAGS) > -- this patch is really for Sam to judge - Cc:-ed him. Ingo
On Wed, 2009-02-04 at 22:26 +0100, Ingo Molnar wrote: > > +include $(srctree)/arch/$(SRCARCH)/Makefile > > + > > # arch Makefile may override CC so keep this after arch Makefile is > included > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) > -print-file-name=include) > > CHECKFLAGS += $(NOSTDINC_FLAGS) > > -- > > this patch is really for Sam to judge - Cc:-ed him. Sam ? I have a few more powerpc patches depending on that one... so I'd like to know if it's totally doomed or not :-) Cheers, Ben.
* Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2009-02-04 at 22:26 +0100, Ingo Molnar wrote: > > > +include $(srctree)/arch/$(SRCARCH)/Makefile > > > + > > > # arch Makefile may override CC so keep this after arch Makefile is > > included > > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) > > -print-file-name=include) > > > CHECKFLAGS += $(NOSTDINC_FLAGS) > > > -- > > > > this patch is really for Sam to judge - Cc:-ed him. > > Sam ? I have a few more powerpc patches depending on that one... so I'd > like to know if it's totally doomed or not :-) Well, toplevel Makefile changes are not to be taken lightly :-) Looks good to me but it's handy to have Sam's Ack on it too. Ingo
On Wed, 2009-02-11 at 14:23 +0100, Ingo Molnar wrote: > * Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > > On Wed, 2009-02-04 at 22:26 +0100, Ingo Molnar wrote: > > > > +include $(srctree)/arch/$(SRCARCH)/Makefile > > > > + > > > > # arch Makefile may override CC so keep this after arch Makefile is > > > included > > > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) > > > -print-file-name=include) > > > > CHECKFLAGS += $(NOSTDINC_FLAGS) > > > > -- > > > > > > this patch is really for Sam to judge - Cc:-ed him. > > > > Sam ? I have a few more powerpc patches depending on that one... so I'd > > like to know if it's totally doomed or not :-) > > Well, toplevel Makefile changes are not to be taken lightly :-) > > Looks good to me but it's handy to have Sam's Ack on it too. > Someone on IRC said that Sam is on vacation. This could take a while for an ACK. -- Steve
On Wed, Feb 04, 2009 at 10:26:12PM +0100, Ingo Molnar wrote: > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > This patch gives arches more freedom on overwriting CFLAGS, specifically > > on PowerPC we want to remove -fno-omit-frame-pointer flag. > > > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > > --- > > Makefile | 4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 7715b2c..d1ba93f 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -525,8 +525,6 @@ else > > KBUILD_CFLAGS += -O2 > > endif > > > > -include $(srctree)/arch/$(SRCARCH)/Makefile > > - > > ifneq (CONFIG_FRAME_WARN,0) > > KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN}) > > endif > > @@ -555,6 +553,8 @@ ifdef CONFIG_DEBUG_SECTION_MISMATCH > > KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once) > > endif > > > > +include $(srctree)/arch/$(SRCARCH)/Makefile > > + > > # arch Makefile may override CC so keep this after arch Makefile is included > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) > > CHECKFLAGS += $(NOSTDINC_FLAGS) > > -- > > this patch is really for Sam to judge - Cc:-ed him. If we move the include further down then the following: # Force gcc to behave correct even for buggy distributions # Arch Makefiles may override this setting KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector) will most likely fail. If popwerpc needs to get rid of "-fno-omit-frame-pointer" then we need a way to express this at KConfig level and NOT by doing some tricks with CFLAGS. Sam
On Wed, Feb 11, 2009 at 02:51:34PM +1100, Benjamin Herrenschmidt wrote: > On Wed, 2009-02-04 at 22:26 +0100, Ingo Molnar wrote: > > > +include $(srctree)/arch/$(SRCARCH)/Makefile > > > + > > > # arch Makefile may override CC so keep this after arch Makefile is > > included > > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) > > -print-file-name=include) > > > CHECKFLAGS += $(NOSTDINC_FLAGS) > > > -- > > > > this patch is really for Sam to judge - Cc:-ed him. > > Sam ? I have a few more powerpc patches depending on that one... so I'd > like to know if it's totally doomed or not :-) See my answer to the mail from Ingo. We really need to do this on Kconfig level it at all possible. Sam
* Sam Ravnborg <sam@ravnborg.org> wrote: > On Wed, Feb 04, 2009 at 10:26:12PM +0100, Ingo Molnar wrote: > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > This patch gives arches more freedom on overwriting CFLAGS, specifically > > > on PowerPC we want to remove -fno-omit-frame-pointer flag. > > > > > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > > > --- > > > Makefile | 4 ++-- > > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index 7715b2c..d1ba93f 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -525,8 +525,6 @@ else > > > KBUILD_CFLAGS += -O2 > > > endif > > > > > > -include $(srctree)/arch/$(SRCARCH)/Makefile > > > - > > > ifneq (CONFIG_FRAME_WARN,0) > > > KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN}) > > > endif > > > @@ -555,6 +553,8 @@ ifdef CONFIG_DEBUG_SECTION_MISMATCH > > > KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once) > > > endif > > > > > > +include $(srctree)/arch/$(SRCARCH)/Makefile > > > + > > > # arch Makefile may override CC so keep this after arch Makefile is included > > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) > > > CHECKFLAGS += $(NOSTDINC_FLAGS) > > > -- > > > > this patch is really for Sam to judge - Cc:-ed him. > > If we move the include further down then the following: > > # Force gcc to behave correct even for buggy distributions > # Arch Makefiles may override this setting > KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector) > > will most likely fail. ah, ok. (I long ago made the mental note of "dont change the toplevel Makefile if you can avoid it" - this reinforces that.) > If popwerpc needs to get rid of "-fno-omit-frame-pointer" then > we need a way to express this at KConfig level and NOT by doing > some tricks with CFLAGS. Here is what we have in the toplevel Makefile at the moment: ifdef CONFIG_FRAME_POINTER KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls else KBUILD_CFLAGS += -fomit-frame-pointer endif My original suggestion (more than a week ago) was to make PPC always select FRAME_POINTERS. It was pointed out that -fno-omit-frame-pointers (i.e.: generate frame pointers) not only makes the code less optimal on PPC, but it can also be miscompiled. But instrumentation really needs to know whether __builtin_return_address(1) [etc] is reliable, whether stack tracing is fast - and other details - and PPC is the odd one out. So the question is: even with FRAME_POINTERS disabled on PPC, is __builtin_return_address(1)/(2) reliable, and is save_stack_trace() fast? (i.e. can it walk down the stack frame efficiently, or does it have to scan the full kernel stack) I.e. does PPC have all the material advantages of frame pointers? Ingo
On Sat, 2009-02-14 at 23:03 +0100, Ingo Molnar wrote: > > So the question is: even with FRAME_POINTERS disabled on PPC, is > __builtin_return_address(1)/(2) reliable, and is save_stack_trace() fast? (i.e. > can it walk down the stack frame efficiently, or does it have to scan the full > kernel stack) I.e. does PPC have all the material advantages of frame pointers? Yes, we do. We effectively have frame pointers in fact, they may only be omitted in leaf functions but then gcc __builtin_return_address() knows how to handle that afaik. Cheers, Ben.
* Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Sat, 2009-02-14 at 23:03 +0100, Ingo Molnar wrote: > > > > So the question is: even with FRAME_POINTERS disabled on PPC, is > > __builtin_return_address(1)/(2) reliable, and is save_stack_trace() fast? (i.e. > > can it walk down the stack frame efficiently, or does it have to scan the full > > kernel stack) I.e. does PPC have all the material advantages of frame pointers? > > Yes, we do. We effectively have frame pointers in fact, they may only be > omitted in leaf functions but then gcc __builtin_return_address() knows > how to handle that afaik. So basically we want to define FRAME_POINTERS on PPC, but do not want the -fno-omit-frame-pointers flag. Originally (many moons ago) FRAME_POINTER _was_ just the toplevel Makefile detail, but these days we've got a handful of secondary uses as well, expressing the reliability of backtraces in essence. We could split the whole option (affecting lots of files), or we could zap that compiler flag in the PPC case - it is only PPC that worries about this anyway. Ingo
diff --git a/Makefile b/Makefile index 7715b2c..d1ba93f 100644 --- a/Makefile +++ b/Makefile @@ -525,8 +525,6 @@ else KBUILD_CFLAGS += -O2 endif -include $(srctree)/arch/$(SRCARCH)/Makefile - ifneq (CONFIG_FRAME_WARN,0) KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN}) endif @@ -555,6 +553,8 @@ ifdef CONFIG_DEBUG_SECTION_MISMATCH KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once) endif +include $(srctree)/arch/$(SRCARCH)/Makefile + # arch Makefile may override CC so keep this after arch Makefile is included NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) CHECKFLAGS += $(NOSTDINC_FLAGS)
This patch gives arches more freedom on overwriting CFLAGS, specifically on PowerPC we want to remove -fno-omit-frame-pointer flag. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- Makefile | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)