[PATCH 0/3] ftrace: updates for tip

[PATCH 0/3] ftrace: updates for tip

[Steven Rostedt]

Ingo,

The first patch here is to disable the branch tracer on ALPHA.
There has been several reports that the branch tracer breaks the
compile on ALPHA. Alpha uses ifs extern inlines, and the injecting
of static elements breaks the build.

The next patch fixes the selecting of a tracer for bootup.
Now you can select the default tracer from the kernel command line.

i.e.

  ftrace=function

Will start the function tracer as soon as it is registered.

Now that we have the kernel command line tracer selection working
we can use it for he boot "initcall" tracer. Instead of having the
initcall tracer disable selftests, it now needs to be selected
in the kernel command line as the default tracer to be implemented.
This means we can keep both selftest and boot initcall tracer configured
at the same time.

  ftrace=initcall

Will now enable the boot initcall tracer. No need to recompile.

-- Steve


The following patches are in:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

    branch: tip/devel


Steven Rostedt (3):
      trace: disable branch tracer on alpha
      trace: fix default boot up tracer
      trace: let boot trace be chosen by command line

----
 kernel/trace/Kconfig      |    9 +++---
 kernel/trace/trace.c      |   65 ++++++++++++++++++++++++++++++++++++++-------
 kernel/trace/trace_boot.c |   11 +++++---
 3 files changed, 67 insertions(+), 18 deletions(-)
-- 

[PATCH 1/3] trace: disable branch tracer on alpha

[Steven Rostedt]

[-- Attachment #1: 0001-trace-disable-branch-tracer-on-alpha.patch --]
[-- Type: text/plain, Size: 886 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Impact: fix alpha compiler builds

The profile all branches tracer causes errors on alpha, due to
the use of extern inlines. This patch disables it on alpha.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/trace/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index dde1d46..2780665 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -183,9 +183,11 @@ config TRACE_BRANCH_PROFILING
 
 	  Say N if unsure.
 
+# PROFILE_ALL_BRANCHES fails on alph due to extern inlines.
 config PROFILE_ALL_BRANCHES
 	bool "Profile all if conditionals"
 	depends on TRACE_BRANCH_PROFILING
+	depends on !ALPHA
 	help
 	  This tracer profiles all branch conditions. Every if ()
 	  taken in the kernel is recorded whether it hit or miss.
-- 
1.5.6.5

-- 

[PATCH 2/3] trace: fix default boot up tracer

[Steven Rostedt]

[-- Attachment #1: 0002-trace-fix-default-boot-up-tracer.patch --]
[-- Type: text/plain, Size: 3924 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Peter Zijlstra started the functionality to start up a default
tracing at bootup. This patch finishes the work.

Now if you add 'ftrace=<tracer>' to the command line, when that tracer
is registered on bootup, that tracer is selected and starts tracing.

Note, all selftests for tracers that are registered after this tracer
is disabled. This prevents the selftests from disturbing the running
tracer, or the running tracer from disturbing the selftest.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/trace/trace.c |   60 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2f8ac1f..2c720c7 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -53,6 +53,11 @@ unsigned long __read_mostly	tracing_thresh;
  */
 static bool __read_mostly tracing_selftest_running;
 
+/*
+ * If a tracer is running, we do not want to run SELFTEST.
+ */
+static bool __read_mostly tracing_selftest_disabled;
+
 /* For tracers that don't implement custom flags */
 static struct tracer_opt dummy_tracer_opt[] = {
 	{ }
@@ -110,14 +115,19 @@ static cpumask_var_t __read_mostly	tracing_buffer_mask;
  */
 int ftrace_dump_on_oops;
 
-static int tracing_set_tracer(char *buf);
+static int tracing_set_tracer(const char *buf);
+
+#define BOOTUP_TRACER_SIZE		100
+static char bootup_tracer_buf[BOOTUP_TRACER_SIZE] __initdata;
+static char *default_bootup_tracer;
 
 static int __init set_ftrace(char *str)
 {
-	tracing_set_tracer(str);
+	strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE);
+	default_bootup_tracer = bootup_tracer_buf;
 	return 1;
 }
-__setup("ftrace", set_ftrace);
+__setup("ftrace=", set_ftrace);
 
 static int __init set_ftrace_dump_on_oops(char *str)
 {
@@ -468,7 +478,7 @@ int register_tracer(struct tracer *type)
 			type->flags->opts = dummy_tracer_opt;
 
 #ifdef CONFIG_FTRACE_STARTUP_TEST
-	if (type->selftest) {
+	if (type->selftest && !tracing_selftest_disabled) {
 		struct tracer *saved_tracer = current_trace;
 		struct trace_array *tr = &global_trace;
 		int i;
@@ -510,8 +520,25 @@ int register_tracer(struct tracer *type)
  out:
 	tracing_selftest_running = false;
 	mutex_unlock(&trace_types_lock);
-	lock_kernel();
 
+	if (!ret && default_bootup_tracer) {
+		if (!strncmp(default_bootup_tracer, type->name,
+			     BOOTUP_TRACER_SIZE)) {
+			printk(KERN_INFO "Starting tracer '%s'\n",
+			       type->name);
+			/* Do we want this tracer to start on bootup? */
+			tracing_set_tracer(type->name);
+			default_bootup_tracer = NULL;
+			/* disable other selftests, since this will break it. */
+			tracing_selftest_disabled = 1;
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+			printk(KERN_INFO "Disabling FTRACE selftests due"
+			       " to running tracer '%s'\n", type->name);
+#endif
+		}
+	}
+
+	lock_kernel();
 	return ret;
 }
 
@@ -2245,7 +2272,7 @@ tracing_set_trace_read(struct file *filp, char __user *ubuf,
 	return simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
 }
 
-static int tracing_set_tracer(char *buf)
+static int tracing_set_tracer(const char *buf)
 {
 	struct trace_array *tr = &global_trace;
 	struct tracer *t;
@@ -3163,5 +3190,26 @@ out_free_buffer_mask:
 out:
 	return ret;
 }
+
+__init static int clear_boot_tracer(void)
+{
+	/*
+	 * The default tracer at boot buffer is an init section.
+	 * This function is called in lateinit. If we did not
+	 * find the boot tracer, then clear it out, to prevent
+	 * later registration from accessing the buffer that is
+	 * about to be freed.
+	 */
+	if (!default_bootup_tracer)
+		return 0;
+
+	printk(KERN_INFO "ftrace bootup tracer '%s' not registered.\n",
+	       default_bootup_tracer);
+	default_bootup_tracer = NULL;
+
+	return 0;
+}
+
 early_initcall(tracer_alloc_buffers);
 fs_initcall(tracer_init_debugfs);
+late_initcall(clear_boot_tracer);
-- 
1.5.6.5

-- 

[PATCH 3/3] trace: let boot trace be chosen by command line

[Steven Rostedt]

[-- Attachment #1: 0003-trace-let-boot-trace-be-chosen-by-command-line.patch --]
[-- Type: text/plain, Size: 3328 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Now that we have a working ftrace=<tracer> function, make the boot
tracer get activated by it. This way we can turn it on or off without
recompiling the kernel, as well as keeping the selftests on. The
selftests are disabled whenever a default tracer starts running.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/trace/Kconfig      |    7 +++----
 kernel/trace/trace.c      |    5 +----
 kernel/trace/trace_boot.c |   11 +++++++----
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 2780665..8115daf 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -164,9 +164,8 @@ config BOOT_TRACER
 	  representation of the delays during initcalls - but the raw
 	  /debug/tracing/trace text output is readable too.
 
-	  ( Note that tracing self tests can't be enabled if this tracer is
-	    selected, because the self-tests are an initcall as well and that
-	    would invalidate the boot trace. )
+	  You must pass in ftrace=initcall to the kernel command line
+	  to enable this on bootup.
 
 config TRACE_BRANCH_PROFILING
 	bool "Trace likely/unlikely profiler"
@@ -328,7 +327,7 @@ config FTRACE_SELFTEST
 
 config FTRACE_STARTUP_TEST
 	bool "Perform a startup test on ftrace"
-	depends on TRACING && DEBUG_KERNEL && !BOOT_TRACER
+	depends on TRACING && DEBUG_KERNEL
 	select FTRACE_SELFTEST
 	help
 	  This option performs a series of startup tests on ftrace. On bootup
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2c720c7..40edef4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3167,12 +3167,9 @@ __init static int tracer_alloc_buffers(void)
 	trace_init_cmdlines();
 
 	register_tracer(&nop_trace);
+	current_trace = &nop_trace;
 #ifdef CONFIG_BOOT_TRACER
 	register_tracer(&boot_tracer);
-	current_trace = &boot_tracer;
-	current_trace->init(&global_trace);
-#else
-	current_trace = &nop_trace;
 #endif
 	/* All seems OK, enable tracing */
 	tracing_disabled = 0;
diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index 0e94b3d..1f07895 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -28,13 +28,13 @@ void start_boot_trace(void)
 
 void enable_boot_trace(void)
 {
-	if (pre_initcalls_finished)
+	if (boot_trace && pre_initcalls_finished)
 		tracing_start_sched_switch_record();
 }
 
 void disable_boot_trace(void)
 {
-	if (pre_initcalls_finished)
+	if (boot_trace && pre_initcalls_finished)
 		tracing_stop_sched_switch_record();
 }
 
@@ -43,6 +43,9 @@ static int boot_trace_init(struct trace_array *tr)
 	int cpu;
 	boot_trace = tr;
 
+	if (!tr)
+		return 0;
+
 	for_each_cpu(cpu, cpu_possible_mask)
 		tracing_reset(tr, cpu);
 
@@ -132,7 +135,7 @@ void trace_boot_call(struct boot_trace_call *bt, initcall_t fn)
 	unsigned long irq_flags;
 	struct trace_array *tr = boot_trace;
 
-	if (!pre_initcalls_finished)
+	if (!tr || !pre_initcalls_finished)
 		return;
 
 	/* Get its name now since this function could
@@ -164,7 +167,7 @@ void trace_boot_ret(struct boot_trace_ret *bt, initcall_t fn)
 	unsigned long irq_flags;
 	struct trace_array *tr = boot_trace;
 
-	if (!pre_initcalls_finished)
+	if (!tr || !pre_initcalls_finished)
 		return;
 
 	sprint_symbol(bt->func, (unsigned long)fn);
-- 
1.5.6.5

-- 

Re: [PATCH 2/3] trace: fix default boot up tracer

[Andrew Morton]

On Mon, 02 Feb 2009 21:38:32 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:

> @@ -510,8 +520,25 @@ int register_tracer(struct tracer *type)
>   out:
>  	tracing_selftest_running = false;
>  	mutex_unlock(&trace_types_lock);
> -	lock_kernel();
>  
> +	if (!ret && default_bootup_tracer) {
> +		if (!strncmp(default_bootup_tracer, type->name,
> +			     BOOTUP_TRACER_SIZE)) {
> +			printk(KERN_INFO "Starting tracer '%s'\n",
> +			       type->name);
> +			/* Do we want this tracer to start on bootup? */
> +			tracing_set_tracer(type->name);
> +			default_bootup_tracer = NULL;
> +			/* disable other selftests, since this will break it. */
> +			tracing_selftest_disabled = 1;
> +#ifdef CONFIG_FTRACE_STARTUP_TEST
> +			printk(KERN_INFO "Disabling FTRACE selftests due"
> +			       " to running tracer '%s'\n", type->name);
> +#endif
> +		}
> +	}
> +
> +	lock_kernel();
>  	return ret;
>  }

The fun and games which register_tracer() plays with lock_kernel() tell
us that this function is only called at bootup time and hence could be
__init.  A quick whizz through callers confirms that.

And if register_tracer() is also only callable at bootup, one suspects
that unregister_tracer() isn't useful.  And lo, it has no callers.

This leads one to further surmise that trace_types_lock a) could be
__initdata and b) could be removed (the list is only altered when we're
running single-threaded).  This appears to be the case.

Re: [PATCH 2/3] trace: fix default boot up tracer

[Steven Rostedt]

On Mon, 2 Feb 2009, Andrew Morton wrote:

> On Mon, 02 Feb 2009 21:38:32 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > @@ -510,8 +520,25 @@ int register_tracer(struct tracer *type)
> >   out:
> >  	tracing_selftest_running = false;
> >  	mutex_unlock(&trace_types_lock);
> > -	lock_kernel();
> >  
> > +	if (!ret && default_bootup_tracer) {
> > +		if (!strncmp(default_bootup_tracer, type->name,
> > +			     BOOTUP_TRACER_SIZE)) {
> > +			printk(KERN_INFO "Starting tracer '%s'\n",
> > +			       type->name);
> > +			/* Do we want this tracer to start on bootup? */
> > +			tracing_set_tracer(type->name);
> > +			default_bootup_tracer = NULL;
> > +			/* disable other selftests, since this will break it. */
> > +			tracing_selftest_disabled = 1;
> > +#ifdef CONFIG_FTRACE_STARTUP_TEST
> > +			printk(KERN_INFO "Disabling FTRACE selftests due"
> > +			       " to running tracer '%s'\n", type->name);
> > +#endif
> > +		}
> > +	}
> > +
> > +	lock_kernel();
> >  	return ret;
> >  }
> 
> The fun and games which register_tracer() plays with lock_kernel() tell
> us that this function is only called at bootup time and hence could be
> __init.  A quick whizz through callers confirms that.
> 
> And if register_tracer() is also only callable at bootup, one suspects
> that unregister_tracer() isn't useful.  And lo, it has no callers.
> 
> This leads one to further surmise that trace_types_lock a) could be
> __initdata and b) could be removed (the list is only altered when we're
> running single-threaded).  This appears to be the case.

The lock_kernel addition was added when the BKL became a spinlock again. 
The selftests needed to be able to sleep, and this caused issues.

The register_tracer was initial written to be pluggable at any time. 
Perhaps in the future to allow modules. But this does not seem to have 
panned out.

Since we have the lock_kernel there anyway, if we ever need to handle 
modules, that will need a different interface anyway. I guess I can nuke 
the unregister tracer.

As for the trace_types_lock mutex, that is still needed. Not only did it 
guard against the tracer registry, it also guards the switching of tracers.

 # echo function > /debug/tracing/current_tracer

-- Steve

Re: [PATCH 2/3] trace: fix default boot up tracer

[Andrew Morton]

On Mon, 2 Feb 2009 23:12:37 -0500 (EST) Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> On Mon, 2 Feb 2009, Andrew Morton wrote:
> 
> > On Mon, 02 Feb 2009 21:38:32 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > @@ -510,8 +520,25 @@ int register_tracer(struct tracer *type)
> > >   out:
> > >  	tracing_selftest_running = false;
> > >  	mutex_unlock(&trace_types_lock);
> > > -	lock_kernel();
> > >  
> > > +	if (!ret && default_bootup_tracer) {
> > > +		if (!strncmp(default_bootup_tracer, type->name,
> > > +			     BOOTUP_TRACER_SIZE)) {
> > > +			printk(KERN_INFO "Starting tracer '%s'\n",
> > > +			       type->name);
> > > +			/* Do we want this tracer to start on bootup? */
> > > +			tracing_set_tracer(type->name);
> > > +			default_bootup_tracer = NULL;
> > > +			/* disable other selftests, since this will break it. */
> > > +			tracing_selftest_disabled = 1;
> > > +#ifdef CONFIG_FTRACE_STARTUP_TEST
> > > +			printk(KERN_INFO "Disabling FTRACE selftests due"
> > > +			       " to running tracer '%s'\n", type->name);
> > > +#endif
> > > +		}
> > > +	}
> > > +
> > > +	lock_kernel();
> > >  	return ret;
> > >  }
> > 
> > The fun and games which register_tracer() plays with lock_kernel() tell
> > us that this function is only called at bootup time and hence could be
> > __init.  A quick whizz through callers confirms that.
> > 
> > And if register_tracer() is also only callable at bootup, one suspects
> > that unregister_tracer() isn't useful.  And lo, it has no callers.
> > 
> > This leads one to further surmise that trace_types_lock a) could be
> > __initdata and b) could be removed (the list is only altered when we're
> > running single-threaded).  This appears to be the case.
> 
> The lock_kernel addition was added when the BKL became a spinlock again. 
> The selftests needed to be able to sleep, and this caused issues.

Sleeping inside lock_kernel() is quite OK.  Confused.

What is the call path to this function?  Does it all happen under
ftrace_init()?  If not, do we risk breaking start_kernel()'s
thou-shalt-not-enable-interrupts-early rule which powerpc (at least)
imposes?

> The register_tracer was initial written to be pluggable at any time. 
> Perhaps in the future to allow modules. But this does not seem to have 
> panned out.
> 
> Since we have the lock_kernel there anyway, if we ever need to handle 
> modules, that will need a different interface anyway. I guess I can nuke 
> the unregister tracer.

And add some __init/__initdatas?

> As for the trace_types_lock mutex, that is still needed. Not only did it 
> guard against the tracer registry, it also guards the switching of tracers.
> 
>  # echo function > /debug/tracing/current_tracer
> 

OK, it's a dual-use lock, as the comment indicates.

Re: [PATCH 2/3] trace: fix default boot up tracer

[Steven Rostedt]

On Mon, 2 Feb 2009, Andrew Morton wrote:
> > 
> > The lock_kernel addition was added when the BKL became a spinlock again. 
> > The selftests needed to be able to sleep, and this caused issues.
> 
> Sleeping inside lock_kernel() is quite OK.  Confused.

I did not explain that quite well. I need to focus on the emails 
that I write, and not do it half concentrating on code that I'm
also writing :-/

The preempt tracer expects preemption enabled when the self test is 
executed. Because the self test for preempt tracer is basically:

start_trace();
preempt_disable();
udelay(x);
preempt_enable();
stop_trace();

make sure we have a delay.

This failed, because lock_kernel now disables preemption. So that 
preempt_disable() never triggers the trace, and the test sees that nothing 
was recorded. This causes a failure to be flagged, and we disable the 
preempt tracer.

> 
> What is the call path to this function?  Does it all happen under
> ftrace_init()?  If not, do we risk breaking start_kernel()'s
> thou-shalt-not-enable-interrupts-early rule which powerpc (at least)
> imposes?

This function is always called via the initcall functions.

> 
> > The register_tracer was initial written to be pluggable at any time. 
> > Perhaps in the future to allow modules. But this does not seem to have 
> > panned out.
> > 
> > Since we have the lock_kernel there anyway, if we ever need to handle 
> > modules, that will need a different interface anyway. I guess I can nuke 
> > the unregister tracer.
> 
> And add some __init/__initdatas?

I'll take some time to analyze what can be annotated.

-- Steve
> 

Re: [PATCH 2/3] trace: fix default boot up tracer

[Andrew Morton]

On Mon, 2 Feb 2009 23:33:52 -0500 (EST) Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 2 Feb 2009, Andrew Morton wrote:
> > > 
> > > The lock_kernel addition was added when the BKL became a spinlock again. 
> > > The selftests needed to be able to sleep, and this caused issues.
> > 
> > Sleeping inside lock_kernel() is quite OK.  Confused.
> 
> I did not explain that quite well. I need to focus on the emails 
> that I write, and not do it half concentrating on code that I'm
> also writing :-/
> 
> The preempt tracer expects preemption enabled when the self test is 
> executed. Because the self test for preempt tracer is basically:
> 
> start_trace();
> preempt_disable();
> udelay(x);
> preempt_enable();
> stop_trace();
> 
> make sure we have a delay.
> 
> This failed, because lock_kernel now disables preemption. So that 
> preempt_disable() never triggers the trace, and the test sees that nothing 
> was recorded. This causes a failure to be flagged, and we disable the 
> preempt tracer.

OK.

It might be a bit cleaner to run all the selftests later, after
start_kernel() has done unlock_kernel().  That would make it even
harder to support modular tracers in the future though.

Perhaps the preempt tracer could itself do

	if (kernel_locked()) {
		kernel_was_locked = true;
		unlock_kernel();
	}
	...

Re: [PATCH 1/3] trace: disable branch tracer on alpha

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <srostedt@redhat.com>
> 
> Impact: fix alpha compiler builds
> 
> The profile all branches tracer causes errors on alpha, due to
> the use of extern inlines. This patch disables it on alpha.
> 
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  kernel/trace/Kconfig |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index dde1d46..2780665 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -183,9 +183,11 @@ config TRACE_BRANCH_PROFILING
>  
>  	  Say N if unsure.
>  
> +# PROFILE_ALL_BRANCHES fails on alph due to extern inlines.
>  config PROFILE_ALL_BRANCHES
>  	bool "Profile all if conditionals"
>  	depends on TRACE_BRANCH_PROFILING
> +	depends on !ALPHA
>  	help
>  	  This tracer profiles all branch conditions. Every if ()
>  	  taken in the kernel is recorded whether it hit or miss.

Alpha has 108 extern inlines in about a dozen include files:

earth4:~/tip> git grep 'extern inline' arch/alpha/include/asm/ | cut -d: -f1 
| uniq -c | sort -n
      1 arch/alpha/include/asm/core_apecs.h
      1 arch/alpha/include/asm/core_cia.h
      1 arch/alpha/include/asm/core_irongate.h
      1 arch/alpha/include/asm/core_lca.h
      1 arch/alpha/include/asm/core_marvel.h
      1 arch/alpha/include/asm/core_polaris.h
      1 arch/alpha/include/asm/core_titan.h
      1 arch/alpha/include/asm/core_tsunami.h
      1 arch/alpha/include/asm/core_wildfire.h
      1 arch/alpha/include/asm/jensen.h
      1 arch/alpha/include/asm/pci.h
      1 arch/alpha/include/asm/tlbflush.h
      2 arch/alpha/include/asm/core_mcpcia.h
      3 arch/alpha/include/asm/mmu_context.h
      3 arch/alpha/include/asm/processor.h
      5 arch/alpha/include/asm/system.h
      7 arch/alpha/include/asm/core_t2.h
      9 arch/alpha/include/asm/uaccess.h
     33 arch/alpha/include/asm/pgtable.h
     34 arch/alpha/include/asm/io.h

Wouldnt the right fix be to:

  sed -i 's/extern inline/static inline/' arch/alpha/include/asm/*.h
  git add arch/alpha/include/asm/*.h
  git commit -m "alpha: change extern inlines to static inlines"

?

	Ingo

Re: [PATCH 2/3] trace: fix default boot up tracer

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <srostedt@redhat.com>
> 
> Peter Zijlstra started the functionality to start up a default
> tracing at bootup. This patch finishes the work.
> 
> Now if you add 'ftrace=<tracer>' to the command line, when that tracer
> is registered on bootup, that tracer is selected and starts tracing.
> 
> Note, all selftests for tracers that are registered after this tracer
> is disabled. This prevents the selftests from disturbing the running
> tracer, or the running tracer from disturbing the selftest.
> 
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  kernel/trace/trace.c |   60 +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 54 insertions(+), 6 deletions(-)

applied to tip/tracing/ftrace, thanks Steve!

This portion is a bit ugly:

> -	lock_kernel();
>  
> +	if (!ret && default_bootup_tracer) {
> +		if (!strncmp(default_bootup_tracer, type->name,
> +			     BOOTUP_TRACER_SIZE)) {
> +			printk(KERN_INFO "Starting tracer '%s'\n",
> +			       type->name);
> +			/* Do we want this tracer to start on bootup? */
> +			tracing_set_tracer(type->name);
> +			default_bootup_tracer = NULL;
> +			/* disable other selftests, since this will break it. */
> +			tracing_selftest_disabled = 1;
> +#ifdef CONFIG_FTRACE_STARTUP_TEST
> +			printk(KERN_INFO "Disabling FTRACE selftests due"
> +			       " to running tracer '%s'\n", type->name);
> +#endif
> +		}
> +	}
> +
> +	lock_kernel();
>  	return ret;

This could be done as:

	if (ret || default_bootup_tracer)
		goto out_relock;

	if (strncmp(default_bootup_tracer, type->name, BOOTUP_TRACER_SIZE)) 
		goto out_relock;

	...

 out_relock:
	lock_kernel();
	return ret;

And we lose a lot of crappy linebreaks that way as well - beyond making the 
purpose of the BKL bits clearer. Hm?

	Ingo

Re: [PATCH 3/3] trace: let boot trace be chosen by command line

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <srostedt@redhat.com>
> 
> Now that we have a working ftrace=<tracer> function, make the boot
> tracer get activated by it. This way we can turn it on or off without
> recompiling the kernel, as well as keeping the selftests on. The
> selftests are disabled whenever a default tracer starts running.
> 
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  kernel/trace/Kconfig      |    7 +++----
>  kernel/trace/trace.c      |    5 +----
>  kernel/trace/trace_boot.c |   11 +++++++----
>  3 files changed, 11 insertions(+), 12 deletions(-)

Applied to tip/tracing/ftrace, thanks Steve!

	Ingo

Re: [PATCH 2/3] trace: fix default boot up tracer

[Frederic Weisbecker]

On Mon, Feb 02, 2009 at 09:38:32PM -0500, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Peter Zijlstra started the functionality to start up a default
> tracing at bootup. This patch finishes the work.
> 
> Now if you add 'ftrace=<tracer>' to the command line, when that tracer
> is registered on bootup, that tracer is selected and starts tracing.
> 
> Note, all selftests for tracers that are registered after this tracer
> is disabled. This prevents the selftests from disturbing the running
> tracer, or the running tracer from disturbing the selftest.
> 
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>

Great!

> ---
>  kernel/trace/trace.c |   60 +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 2f8ac1f..2c720c7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -53,6 +53,11 @@ unsigned long __read_mostly	tracing_thresh;
>   */
>  static bool __read_mostly tracing_selftest_running;
>  
> +/*
> + * If a tracer is running, we do not want to run SELFTEST.
> + */
> +static bool __read_mostly tracing_selftest_disabled;
> +
>  /* For tracers that don't implement custom flags */
>  static struct tracer_opt dummy_tracer_opt[] = {
>  	{ }
> @@ -110,14 +115,19 @@ static cpumask_var_t __read_mostly	tracing_buffer_mask;
>   */
>  int ftrace_dump_on_oops;
>  
> -static int tracing_set_tracer(char *buf);
> +static int tracing_set_tracer(const char *buf);
> +
> +#define BOOTUP_TRACER_SIZE		100
> +static char bootup_tracer_buf[BOOTUP_TRACER_SIZE] __initdata;
> +static char *default_bootup_tracer;
>  
>  static int __init set_ftrace(char *str)
>  {
> -	tracing_set_tracer(str);
> +	strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE);
> +	default_bootup_tracer = bootup_tracer_buf;
>  	return 1;
>  }
> -__setup("ftrace", set_ftrace);
> +__setup("ftrace=", set_ftrace);
>  
>  static int __init set_ftrace_dump_on_oops(char *str)
>  {
> @@ -468,7 +478,7 @@ int register_tracer(struct tracer *type)
>  			type->flags->opts = dummy_tracer_opt;
>  
>  #ifdef CONFIG_FTRACE_STARTUP_TEST
> -	if (type->selftest) {
> +	if (type->selftest && !tracing_selftest_disabled) {
>  		struct tracer *saved_tracer = current_trace;
>  		struct trace_array *tr = &global_trace;
>  		int i;
> @@ -510,8 +520,25 @@ int register_tracer(struct tracer *type)
>   out:
>  	tracing_selftest_running = false;
>  	mutex_unlock(&trace_types_lock);
> -	lock_kernel();
>  
> +	if (!ret && default_bootup_tracer) {
> +		if (!strncmp(default_bootup_tracer, type->name,
> +			     BOOTUP_TRACER_SIZE)) {
> +			printk(KERN_INFO "Starting tracer '%s'\n",
> +			       type->name);
> +			/* Do we want this tracer to start on bootup? */
> +			tracing_set_tracer(type->name);
> +			default_bootup_tracer = NULL;
> +			/* disable other selftests, since this will break it. */
> +			tracing_selftest_disabled = 1;
> +#ifdef CONFIG_FTRACE_STARTUP_TEST
> +			printk(KERN_INFO "Disabling FTRACE selftests due"
> +			       " to running tracer '%s'\n", type->name);
> +#endif
> +		}
> +	}
> +
> +	lock_kernel();
>  	return ret;
>  }
>  
> @@ -2245,7 +2272,7 @@ tracing_set_trace_read(struct file *filp, char __user *ubuf,
>  	return simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
>  }
>  
> -static int tracing_set_tracer(char *buf)
> +static int tracing_set_tracer(const char *buf)
>  {
>  	struct trace_array *tr = &global_trace;
>  	struct tracer *t;
> @@ -3163,5 +3190,26 @@ out_free_buffer_mask:
>  out:
>  	return ret;
>  }
> +
> +__init static int clear_boot_tracer(void)
> +{
> +	/*
> +	 * The default tracer at boot buffer is an init section.
> +	 * This function is called in lateinit. If we did not
> +	 * find the boot tracer, then clear it out, to prevent
> +	 * later registration from accessing the buffer that is
> +	 * about to be freed.
> +	 */
> +	if (!default_bootup_tracer)
> +		return 0;
> +
> +	printk(KERN_INFO "ftrace bootup tracer '%s' not registered.\n",
> +	       default_bootup_tracer);
> +	default_bootup_tracer = NULL;
> +
> +	return 0;
> +}
> +
>  early_initcall(tracer_alloc_buffers);
>  fs_initcall(tracer_init_debugfs);
> +late_initcall(clear_boot_tracer);


That makes me think that perhaps a tracer would prefer to keep or not the tracing
when system_state looses the value SYSTEM_BOOTING.

Something similar to earlyprintk by adding a "keep" value in the ftrace parameter.

Perhaps it's better to wait for someone to request it...

Re: [PATCH 3/3] trace: let boot trace be chosen by command line

[Frederic Weisbecker]

On Mon, Feb 02, 2009 at 09:38:33PM -0500, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Now that we have a working ftrace=<tracer> function, make the boot
> tracer get activated by it. This way we can turn it on or off without
> recompiling the kernel, as well as keeping the selftests on. The
> selftests are disabled whenever a default tracer starts running.
> 
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  kernel/trace/Kconfig      |    7 +++----
>  kernel/trace/trace.c      |    5 +----
>  kernel/trace/trace_boot.c |   11 +++++++----
>  3 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 2780665..8115daf 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -164,9 +164,8 @@ config BOOT_TRACER
>  	  representation of the delays during initcalls - but the raw
>  	  /debug/tracing/trace text output is readable too.
>  
> -	  ( Note that tracing self tests can't be enabled if this tracer is
> -	    selected, because the self-tests are an initcall as well and that
> -	    would invalidate the boot trace. )
> +	  You must pass in ftrace=initcall to the kernel command line
> +	  to enable this on bootup.
>  
>  config TRACE_BRANCH_PROFILING
>  	bool "Trace likely/unlikely profiler"
> @@ -328,7 +327,7 @@ config FTRACE_SELFTEST
>  
>  config FTRACE_STARTUP_TEST
>  	bool "Perform a startup test on ftrace"
> -	depends on TRACING && DEBUG_KERNEL && !BOOT_TRACER
> +	depends on TRACING && DEBUG_KERNEL
>  	select FTRACE_SELFTEST
>  	help
>  	  This option performs a series of startup tests on ftrace. On bootup
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 2c720c7..40edef4 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3167,12 +3167,9 @@ __init static int tracer_alloc_buffers(void)
>  	trace_init_cmdlines();
>  
>  	register_tracer(&nop_trace);
> +	current_trace = &nop_trace;
>  #ifdef CONFIG_BOOT_TRACER
>  	register_tracer(&boot_tracer);
> -	current_trace = &boot_tracer;
> -	current_trace->init(&global_trace);
> -#else
> -	current_trace = &nop_trace;
>  #endif
>  	/* All seems OK, enable tracing */
>  	tracing_disabled = 0;
> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
> index 0e94b3d..1f07895 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -28,13 +28,13 @@ void start_boot_trace(void)
>  
>  void enable_boot_trace(void)
>  {
> -	if (pre_initcalls_finished)
> +	if (boot_trace && pre_initcalls_finished)
>  		tracing_start_sched_switch_record();
>  }
>  
>  void disable_boot_trace(void)
>  {
> -	if (pre_initcalls_finished)
> +	if (boot_trace && pre_initcalls_finished)
>  		tracing_stop_sched_switch_record();
>  }
>  
> @@ -43,6 +43,9 @@ static int boot_trace_init(struct trace_array *tr)
>  	int cpu;
>  	boot_trace = tr;
>  
> +	if (!tr)
> +		return 0;
> +
>  	for_each_cpu(cpu, cpu_possible_mask)
>  		tracing_reset(tr, cpu);
>  
> @@ -132,7 +135,7 @@ void trace_boot_call(struct boot_trace_call *bt, initcall_t fn)
>  	unsigned long irq_flags;
>  	struct trace_array *tr = boot_trace;
>  
> -	if (!pre_initcalls_finished)
> +	if (!tr || !pre_initcalls_finished)
>  		return;
>  
>  	/* Get its name now since this function could
> @@ -164,7 +167,7 @@ void trace_boot_ret(struct boot_trace_ret *bt, initcall_t fn)
>  	unsigned long irq_flags;
>  	struct trace_array *tr = boot_trace;
>  
> -	if (!pre_initcalls_finished)
> +	if (!tr || !pre_initcalls_finished)
>  		return;
>  
>  	sprint_symbol(bt->func, (unsigned long)fn);
> -- 
> 1.5.6.5
> 
> -- 

Thanks, now I should turn out the initcall_debug dependency for the boot tracer
to actually trace. I should even make it trace the async calls BTW ...