diff mbox

[v7,04/10] ppc64 ftrace_with_regs configuration variables

Message ID 20160208152306.GF30328@pathway.suse.cz (mailing list archive)
State Not Applicable
Headers show

Commit Message

Petr Mladek Feb. 8, 2016, 3:23 p.m. UTC 
On Mon 2016-02-08 13:12:45, Torsten Duwe wrote:
> On Mon, Feb 08, 2016 at 11:34:06AM +0100, Petr Mladek wrote:
> > On Sat 2016-02-06 11:32:43, Torsten Duwe wrote:
> > > On Fri, Feb 05, 2016 at 05:18:34PM +0100, Petr Mladek wrote:
> > > [...]
> > > > more complicated. Whem I think about it, the change below does similar
> > > > job and looks more strightforwad:
> > > 
> > > Had I only looked closer. That's exactly how I thought it would work
> > > in the first place. I'd call that a fix. Full ACK from my side.
> > 
> > Feel free to merge this into your patch. Or do you want to do
> > this in a separate one, please?
> 
> My Kconfig/Makefile changes depend on it, but OTOH this change (Fix!)
> is independent.
> 
> IMHO the right thing would be you resend your second mail from Feb-05,
> with your sign-off, my ack, FWIW, and Steven checks it in ;-)

Please, find it below. I guess that it should be applied before
the check causing the build error. It will help to keep
the tree bisectable.


From 2b0fcb678d7720d03f9c9f233b61ed9ed4d420b3 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Mon, 8 Feb 2016 16:03:03 +0100
Subject: [PATCH] ftrace: Allow to explicitly disable the build of the dynamic
 ftrace with regs

This patch allows to explicitly disable
CONFIG_DYNAMIC_FTRACE_WITH_REGS. We will need to do so on
PPC with a broken gcc. This situation will be detected at
buildtime and could not be handled by Kbuild automatically.

Also it fixes the prompt of DYNAMIC_FTRACE. The uppercase
better fits the style of the other menu entries.

This patch does not change the default value.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Torsten Duwe <duwe@lst.de>
---
 kernel/trace/Kconfig | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Steven Rostedt Feb. 8, 2016, 3:49 p.m. UTC | #1 
On Mon, 8 Feb 2016 16:23:06 +0100
Petr Mladek <pmladek@suse.com> wrote:

> >From 2b0fcb678d7720d03f9c9f233b61ed9ed4d420b3 Mon Sep 17 00:00:00 2001  
> From: Petr Mladek <pmladek@suse.com>
> Date: Mon, 8 Feb 2016 16:03:03 +0100
> Subject: [PATCH] ftrace: Allow to explicitly disable the build of the dynamic
>  ftrace with regs
> 
> This patch allows to explicitly disable
> CONFIG_DYNAMIC_FTRACE_WITH_REGS. We will need to do so on
> PPC with a broken gcc. This situation will be detected at
> buildtime and could not be handled by Kbuild automatically.

Wait. Can it be detected at build time? That is, does it cause a build
error? If so, then you can have Kbuild automatically detect this and
set the proper value. We do this with 'asm goto'. There's tricks in the
build system that can change the configs based on if a compiler is
broken or not.


> 
> Also it fixes the prompt of DYNAMIC_FTRACE. The uppercase
> better fits the style of the other menu entries.

s/fixes/updates/

-- Steve

> 
> This patch does not change the default value.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Acked-by: Torsten Duwe <duwe@lst.de>
> ---
Petr Mladek Feb. 8, 2016, 4:32 p.m. UTC | #2 
On Mon 2016-02-08 10:49:28, Steven Rostedt wrote:
> On Mon, 8 Feb 2016 16:23:06 +0100
> Petr Mladek <pmladek@suse.com> wrote:
> 
> > >From 2b0fcb678d7720d03f9c9f233b61ed9ed4d420b3 Mon Sep 17 00:00:00 2001  
> > From: Petr Mladek <pmladek@suse.com>
> > Date: Mon, 8 Feb 2016 16:03:03 +0100
> > Subject: [PATCH] ftrace: Allow to explicitly disable the build of the dynamic
> >  ftrace with regs
> > 
> > This patch allows to explicitly disable
> > CONFIG_DYNAMIC_FTRACE_WITH_REGS. We will need to do so on
> > PPC with a broken gcc. This situation will be detected at
> > buildtime and could not be handled by Kbuild automatically.
> 
> Wait. Can it be detected at build time? That is, does it cause a build
> error? If so, then you can have Kbuild automatically detect this and
> set the proper value. We do this with 'asm goto'. There's tricks in the
> build system that can change the configs based on if a compiler is
> broken or not.

Just to be sure.

Do you suggest to define CONFIG_DYNAMIC_FTRACE_WITH_REGS
via a check in Makefile and rename it to e.g.
CC_FTRACE_WITH_REGS.

Or should we define another variable, e.g.
CC_BROKEN_DYNAMC_FTRACE_WITH_REGS? And then replace all occurences in
the code by something like:

-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#if defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS) && \
+    !defined(CC_BROKEN_DYNAMC_FTRACE_WITH_REGS)

?

Best Regards,
Petr
Torsten Duwe Feb. 9, 2016, 9:02 a.m. UTC | #3 
On Mon, Feb 08, 2016 at 10:49:28AM -0500, Steven Rostedt wrote:
> On Mon, 8 Feb 2016 16:23:06 +0100
> Petr Mladek <pmladek@suse.com> wrote:
> 
> > >From 2b0fcb678d7720d03f9c9f233b61ed9ed4d420b3 Mon Sep 17 00:00:00 2001  
> > From: Petr Mladek <pmladek@suse.com>
> > Date: Mon, 8 Feb 2016 16:03:03 +0100
> > Subject: [PATCH] ftrace: Allow to explicitly disable the build of the dynamic
> >  ftrace with regs
> > 
> > This patch allows to explicitly disable
> > CONFIG_DYNAMIC_FTRACE_WITH_REGS. We will need to do so on
> > PPC with a broken gcc. This situation will be detected at
> > buildtime and could not be handled by Kbuild automatically.
> 
> Wait. Can it be detected at build time? That is, does it cause a build

Yes, I wrote a test to detect it at build time. It is similar to "asm goto"
and part of the v7 patch set.

> error? If so, then you can have Kbuild automatically detect this and
> set the proper value. We do this with 'asm goto'. There's tricks in the
> build system that can change the configs based on if a compiler is
> broken or not.

Please clarify. All I could find is Makefile magic that does it. AFAICS
This runs _after_ Kconfig.

But what I'd like to see is to offer the user the full choice, where possible,
e.g.

Kernel Tracing ...
0) none
1) static FTRACE
2) DYNAMIC_FTRACE
3) DYNAMIC_FTRACE_WITH_REGS

Can such a test be used to simply reduce these options?
With Petr's patch, it comes quite close to the above, and if you select "3"
and your compiler is broken, compilation will fail. For "2", it will just do
the right thing ( fall back to plain "-pg" ).

Without Petr's patch you have *no* choice between "2" and "3".
(That's what I'd call a bug :)

So, the question is, can such a test be used to provide _input_ to
"make config" ? I can see the "env=" mechanism, but it seems not to be used
very heavily. That would then be a prerequisite to all "make *config".
Even if it can provide this input, you can still not choose between 2 and 3
where both are available.

	Torsten
diff mbox

Patch

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index a138f6d866ae..de6dab0f74f2 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -449,7 +449,7 @@  config PROBE_EVENTS
 	def_bool n
 
 config DYNAMIC_FTRACE
-	bool "enable/disable function tracing dynamically"
+	bool "Enable/Disable function tracing dynamically"
 	depends on FUNCTION_TRACER
 	depends on HAVE_DYNAMIC_FTRACE
 	default y
@@ -472,9 +472,17 @@  config DYNAMIC_FTRACE
 	  otherwise has native performance as long as no tracing is active.
 
 config DYNAMIC_FTRACE_WITH_REGS
-	def_bool y
+	bool "Pass registers to function tracer"
 	depends on DYNAMIC_FTRACE
 	depends on HAVE_DYNAMIC_FTRACE_WITH_REGS
+	default y
+	help
+	  This option enables passing the current state of processor
+	  registers to the function tracer. It allows to do a more
+	  detailed analyze and print more information.
+
+	  Say Y here if you are unsure. The only exception is if
+	  you want to pass a build error caused by a broken compiler.
 
 config FUNCTION_PROFILER
 	bool "Kernel function profiler"