From: Steven Rostedt <[email protected]>
The function graph has a test to check if the frame pointer is
corrupted, which can happen with various options of gcc with mcount.
But this is not an issue with -mfentry as -mfentry does not need nor use
frame pointers for function graph tracing.
Cc: Andi Kleen <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace_functions_graph.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index ce27c8b..99b4378 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -143,7 +143,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
return;
}
-#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
+#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
/*
* The arch may choose to record the frame pointer used
* and check it here to make sure that it is what we expect it
@@ -154,6 +154,9 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
*
* Currently, x86_32 with optimize for size (-Os) makes the latest
* gcc do the above.
+ *
+ * Note, -mfentry does not use frame pointers, and this test
+ * is not needed if CC_USING_FENTRY is set.
*/
if (unlikely(current->ret_stack[index].fp != frame_pointer)) {
ftrace_graph_stop();
--
1.7.10.4
(2012/08/08 4:38), Steven Rostedt wrote:
> From: Steven Rostedt <[email protected]>
>
> The function graph has a test to check if the frame pointer is
> corrupted, which can happen with various options of gcc with mcount.
> But this is not an issue with -mfentry as -mfentry does not need nor use
> frame pointers for function graph tracing.
>
> Cc: Andi Kleen <[email protected]>
> Signed-off-by: Steven Rostedt <[email protected]>
> ---
> kernel/trace/trace_functions_graph.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index ce27c8b..99b4378 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -143,7 +143,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
> return;
> }
>
> -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
> +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
I think CONFIG_HAVE_FENTRY would better unselect
CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST in arch/x86/Kconfig explicitly.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Wed, 2012-08-08 at 13:34 +0900, Masami Hiramatsu wrote:
>
> > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
> > +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
>
> I think CONFIG_HAVE_FENTRY would better unselect
> CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST in arch/x86/Kconfig explicitly.
No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
that it is being used. It only gets used if CC_USING_FENTRY is set,
which is set by the Makefile at time of compile.
If CONFIG_HAVE_FENTRY is defined, a test is done to see if the gcc
compiling the kernel supports -mfentry. If it does, then it defines the
CC_USING_FENTRY macro, if not, the macro is not defined and the old way
is performed.
If the old way is performed, even if CONFIG_HAVE_FENTRY is defined, then
we still need the above test. We can not have CONFIG_HAVE_FENTRY
unselect CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST.
-- Steve
(2012/08/08 21:49), Steven Rostedt wrote:
> On Wed, 2012-08-08 at 13:34 +0900, Masami Hiramatsu wrote:
>>
>>> -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
>>> +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
>>
>> I think CONFIG_HAVE_FENTRY would better unselect
>> CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST in arch/x86/Kconfig explicitly.
>
> No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> that it is being used. It only gets used if CC_USING_FENTRY is set,
> which is set by the Makefile at time of compile.
>
> If CONFIG_HAVE_FENTRY is defined, a test is done to see if the gcc
> compiling the kernel supports -mfentry. If it does, then it defines the
> CC_USING_FENTRY macro, if not, the macro is not defined and the old way
> is performed.
>
> If the old way is performed, even if CONFIG_HAVE_FENTRY is defined, then
> we still need the above test. We can not have CONFIG_HAVE_FENTRY
> unselect CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST.
Ah, I see. OK, I don't see any other issue.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Wed, Aug 8, 2012 at 3:49 PM, Steven Rostedt <[email protected]> wrote:
>
> No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> that it is being used. It only gets used if CC_USING_FENTRY is set,
> which is set by the Makefile at time of compile.
Btw, it might be lovely to consider the concept of "Kconfig variables
set by shell-scripts".
We currently have a metric sh*t-ton of Makefile magic for testing
various things like this, and integrating it into Kconfig might be a
good idea. That way you would be able to use the Kconfig logic on
these things.
Kconfig already has the "option env=XYZ" syntax for importing values
from the environment variables. Extending it to some kind of "option
shell=xyz" would allow for more complex interactions like this
(imagine testing compiler options and version dependencies in the
Kconfig files instead of the Makefiles)?
On Thu, 2012-08-09 at 06:45 +0300, Linus Torvalds wrote:
> On Wed, Aug 8, 2012 at 3:49 PM, Steven Rostedt <[email protected]> wrote:
> >
> > No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> > that it is being used. It only gets used if CC_USING_FENTRY is set,
> > which is set by the Makefile at time of compile.
>
> Btw, it might be lovely to consider the concept of "Kconfig variables
> set by shell-scripts".
>
> We currently have a metric sh*t-ton of Makefile magic for testing
> various things like this, and integrating it into Kconfig might be a
> good idea. That way you would be able to use the Kconfig logic on
> these things.
>
> Kconfig already has the "option env=XYZ" syntax for importing values
> from the environment variables. Extending it to some kind of "option
> shell=xyz" would allow for more complex interactions like this
> (imagine testing compiler options and version dependencies in the
> Kconfig files instead of the Makefiles)?
I understand your pain. I think this 'test the environment in the
makefile' is a bit of a hack.
I've worked a little with the Kconfig infrastructure, but I wouldn't
consider myself an expert. Would Kconfig be able to handle a change in
environment? That is, if you change your compiler, would a normal make
kick off Kconfig to update the configs for the new environment. Or would
the user have to do some kind of 'make oldconfig' before that.
Or is make 'oldconfig' performed at all compiles regardless now?
One nice thing of having all these tests represented in the .config
file, is that when a user produces a bug, when they send their config,
we would be able to see much more information about their environment.
-- Steve
On 08/08/2012 08:45 PM, Linus Torvalds wrote:
>
> Btw, it might be lovely to consider the concept of "Kconfig variables
> set by shell-scripts".
>
> We currently have a metric sh*t-ton of Makefile magic for testing
> various things like this, and integrating it into Kconfig might be a
> good idea. That way you would be able to use the Kconfig logic on
> these things.
>
> Kconfig already has the "option env=XYZ" syntax for importing values
> from the environment variables. Extending it to some kind of "option
> shell=xyz" would allow for more complex interactions like this
> (imagine testing compiler options and version dependencies in the
> Kconfig files instead of the Makefiles)?
>
This is a Frequently Requested Feature over on the kbuild list... and it
almost certainly would speed up the build given how many times the tests
are currently run. Furthermore, it would let us base rules on the
support existing, which we currently can't.
-hpa
On Thu, Aug 09, 2012 at 06:45:37AM +0300, Linus Torvalds wrote:
> On Wed, Aug 8, 2012 at 3:49 PM, Steven Rostedt <[email protected]> wrote:
> >
> > No, CONFIG_HAVE_FENTRY just means fentry is supported, it does not mean
> > that it is being used. It only gets used if CC_USING_FENTRY is set,
> > which is set by the Makefile at time of compile.
>
> Btw, it might be lovely to consider the concept of "Kconfig variables
> set by shell-scripts".
I looked into it some time ago because I needed it for something else.
But no mergeable patch produced. Will try again.
One issue was that it will break some odd cross compiling or ccache
setups where the compiler is not set correctly at config time.
But I think it would be worth it.
> We currently have a metric sh*t-ton of Makefile magic for testing
> various things like this, and integrating it into Kconfig might be a
> good idea. That way you would be able to use the Kconfig logic on
> these things.
Also another big win would be to make the empty do nothing build faster,
because the decisions would be cached.
I measured recently and we execute 30+ gccs on a empty build.
-Andi
Commit-ID: 781d06248234e221edb560a18461d65808a8a942
Gitweb: http://git.kernel.org/tip/781d06248234e221edb560a18461d65808a8a942
Author: Steven Rostedt <[email protected]>
AuthorDate: Wed, 9 Feb 2011 13:27:22 -0500
Committer: Steven Rostedt <[email protected]>
CommitDate: Thu, 23 Aug 2012 11:25:29 -0400
ftrace: Do not test frame pointers if -mfentry is used
The function graph has a test to check if the frame pointer is
corrupted, which can happen with various options of gcc with mcount.
But this is not an issue with -mfentry as -mfentry does not need nor use
frame pointers for function graph tracing.
Link: http://lkml.kernel.org/r/[email protected]
Acked-by: H. Peter Anvin <[email protected]>
Acked-by: Ingo Molnar <[email protected]>
Cc: Andi Kleen <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace_functions_graph.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index ce27c8b..99b4378 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -143,7 +143,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
return;
}
-#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
+#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
/*
* The arch may choose to record the frame pointer used
* and check it here to make sure that it is what we expect it
@@ -154,6 +154,9 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
*
* Currently, x86_32 with optimize for size (-Os) makes the latest
* gcc do the above.
+ *
+ * Note, -mfentry does not use frame pointers, and this test
+ * is not needed if CC_USING_FENTRY is set.
*/
if (unlikely(current->ret_stack[index].fp != frame_pointer)) {
ftrace_graph_stop();