[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

-- 

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

[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

-- 

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 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 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...

[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 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 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 ...

[PATCH 0/3] ftrace: updates for tip

[Steven Rostedt]

Ingo,

Arnaldo was nice enough to do something that was on my todo list
for quite some time.

I also included the change you asked for.

The following patches are in:

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

    branch: tip/devel


Arnaldo Carvalho de Melo (2):
      trace_branch: Remove unused function
      trace: Remove unused trace_array_cpu parameter

Steven Rostedt (1):
      trace: code style clean up

----
 block/blktrace.c                  |    2 +-
 kernel/trace/trace.c              |   76 ++++++++++++++++---------------------
 kernel/trace/trace.h              |    4 --
 kernel/trace/trace_branch.c       |   17 --------
 kernel/trace/trace_functions.c    |    8 ++--
 kernel/trace/trace_irqsoff.c      |   10 ++--
 kernel/trace/trace_sched_switch.c |    4 +-
 kernel/trace/trace_sched_wakeup.c |   12 ++---
 8 files changed, 50 insertions(+), 83 deletions(-)
-- 

Re: [PATCH 0/3] ftrace: updates for tip

[Ingo Molnar]

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

> Ingo,
> 
> Arnaldo was nice enough to do something that was on my todo list
> for quite some time.
> 
> I also included the change you asked for.
> 
> The following patches are in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> 
>     branch: tip/devel
> 
> 
> Arnaldo Carvalho de Melo (2):
>       trace_branch: Remove unused function
>       trace: Remove unused trace_array_cpu parameter
> 
> Steven Rostedt (1):
>       trace: code style clean up
> 
> ----
>  block/blktrace.c                  |    2 +-
>  kernel/trace/trace.c              |   76 ++++++++++++++++---------------------
>  kernel/trace/trace.h              |    4 --
>  kernel/trace/trace_branch.c       |   17 --------
>  kernel/trace/trace_functions.c    |    8 ++--
>  kernel/trace/trace_irqsoff.c      |   10 ++--
>  kernel/trace/trace_sched_switch.c |    4 +-
>  kernel/trace/trace_sched_wakeup.c |   12 ++---
>  8 files changed, 50 insertions(+), 83 deletions(-)

Applied to tip:tracing/ftrace, thanks guys!

	Ingo

[PATCH 0/3] ftrace: updates for tip

[Steven Rostedt]

This series restructures the output functions of trace.c.

Events are now registered and maintaining an event output is
simplified by keeping the output close together.

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):
      ftrace: remove obsolete print continue functionality
      ftrace: set up trace event hash infrastructure
      ftrace: change trace.c to use registered events

----
 kernel/trace/Makefile                |    1 +
 kernel/trace/trace.c                 |  738 ++----------------------------
 kernel/trace/trace.h                 |   15 +-
 kernel/trace/trace_boot.c            |    1 +
 kernel/trace/trace_branch.c          |   53 +++
 kernel/trace/trace_functions_graph.c |    4 +-
 kernel/trace/trace_hw_branches.c     |    1 +
 kernel/trace/trace_mmiotrace.c       |    4 +-
 kernel/trace/trace_output.c          |  832 ++++++++++++++++++++++++++++++++++
 kernel/trace/trace_output.h          |   59 +++
 kernel/trace/trace_power.c           |    1 +
 11 files changed, 990 insertions(+), 719 deletions(-)
-- 

Re: [PATCH 0/3] ftrace: updates for tip

[Frederic Weisbecker]

Steven Rostedt wrote:
> This series restructures the output functions of trace.c.
> 
> Events are now registered and maintaining an event output is
> simplified by keeping the output close together.
> 
> 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):
>       ftrace: remove obsolete print continue functionality
>       ftrace: set up trace event hash infrastructure
>       ftrace: change trace.c to use registered events
> 

Which does mean that a tracer will now be able to build as a module?
That's all good news!

I'm testing a bit these patches...

Re: [PATCH 0/3] ftrace: updates for tip

[Frederic Weisbecker]

Steven Rostedt wrote:
> This series restructures the output functions of trace.c.
> 
> Events are now registered and maintaining an event output is
> simplified by keeping the output close together.
> 
> The following patches are in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> 
>     branch: tip/devel

BTW it seems to be more likely on devel than tip/devel ...

Re: [PATCH 0/3] ftrace: updates for tip

[Ingo Molnar]

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

> This series restructures the output functions of trace.c.
> 
> Events are now registered and maintaining an event output is
> simplified by keeping the output close together.
> 
> 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):
>       ftrace: remove obsolete print continue functionality
>       ftrace: set up trace event hash infrastructure
>       ftrace: change trace.c to use registered events

pulled, thanks Steve!

	Ingo

[PATCH 0/3] ftrace: updates for tip

[Steven Rostedt]

Ingo,

This series has three patches.

The first patch adds a new feature that I've been wanting to have for some
time and Arjan even requested. That is to pick a function and only
trace that function and its children. Dynamic ftrace and function
graph needs to be enabled for this.

To do the above, I added a "trace" flags field in the task structure.
The second patch uses this for the ftrace pid code. It searches for
the task based on the pid and sets the trace flag, then in the
ftrace function caller it only needs to check this flag.

This means we can now trace more than one pid without any more overhead.
It also means that we should be able to use the name space code that
the container guys want us to. But since I'm not very up on the
namespace code, I'm still using just the normal 'pid'. I've Cc'd the
container folks so perhaps they could write up a patch for me ;-)

Note: When writing to the set_ftrace_pid two things happen.
 - The task with the matching pid gets the trace flag set.
 - Any other task has its trace flag cleared.
 #2 needs to be addressed when converting to pid name spaces.
 Just because it is not enough to simply find the matching task.
 It may be good enough to just clear all tasks and then find the
 one that matches.

The last patch makes the function graph tracer honor the set_ftrace_pid.

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):
      ftrace: graph of a single function
      ftrace: use task struct trace flag to filter on pid
      ftrace: trace single pid for function graph tracer

----
 include/linux/ftrace.h |   46 +++++++++
 include/linux/sched.h  |    4 +
 kernel/trace/ftrace.c  |  257 +++++++++++++++++++++++++++++++++++++++++++++++-
 kernel/trace/trace.c   |   11 ++
 kernel/trace/trace.h   |   40 +++++++-
 5 files changed, 353 insertions(+), 5 deletions(-)
-- 

Re: [PATCH 0/3] ftrace: updates for tip

[Ingo Molnar]

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

> Ingo,
> 
> This series has three patches.
> 
> The first patch adds a new feature that I've been wanting to have for some
> time and Arjan even requested. That is to pick a function and only
> trace that function and its children. Dynamic ftrace and function
> graph needs to be enabled for this.
> 
> To do the above, I added a "trace" flags field in the task structure.
> The second patch uses this for the ftrace pid code. It searches for
> the task based on the pid and sets the trace flag, then in the
> ftrace function caller it only needs to check this flag.
> 
> This means we can now trace more than one pid without any more overhead.
> It also means that we should be able to use the name space code that
> the container guys want us to. But since I'm not very up on the
> namespace code, I'm still using just the normal 'pid'. I've Cc'd the
> container folks so perhaps they could write up a patch for me ;-)
> 
> Note: When writing to the set_ftrace_pid two things happen.
>  - The task with the matching pid gets the trace flag set.
>  - Any other task has its trace flag cleared.
>  #2 needs to be addressed when converting to pid name spaces.
>  Just because it is not enough to simply find the matching task.
>  It may be good enough to just clear all tasks and then find the
>  one that matches.
> 
> The last patch makes the function graph tracer honor the set_ftrace_pid.
> 
> 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):
>       ftrace: graph of a single function
>       ftrace: use task struct trace flag to filter on pid
>       ftrace: trace single pid for function graph tracer
> 
> ----
>  include/linux/ftrace.h |   46 +++++++++
>  include/linux/sched.h  |    4 +
>  kernel/trace/ftrace.c  |  257 +++++++++++++++++++++++++++++++++++++++++++++++-
>  kernel/trace/trace.c   |   11 ++
>  kernel/trace/trace.h   |   40 +++++++-
>  5 files changed, 353 insertions(+), 5 deletions(-)
> -- 

pulled, thanks Steve!

These are some very nice changes!

	Ingo

Re: [PATCH 0/3] ftrace: updates for tip

[Ingo Molnar]

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

> Ingo,
> 
> This series has three patches.
> 
> The first patch adds a new feature that I've been wanting to have for 
> some time and Arjan even requested. That is to pick a function and only 
> trace that function and its children. Dynamic ftrace and function graph 
> needs to be enabled for this.
> 
> To do the above, I added a "trace" flags field in the task structure. 
> The second patch uses this for the ftrace pid code. It searches for the 
> task based on the pid and sets the trace flag, then in the ftrace 
> function caller it only needs to check this flag.

Btw., i'd love to see this done via the regular regexp interface though, 
if possible - instead of the add-on interface you added.

( Also perhaps enable to toggle tracing via the /proc/<PID>/ hierarchy - 
  a /proc/<PID>/tracing_enabled switch or so. )

Regarding the filter functions, the basic principle should be 
mathematical set operations, like we have it now: add and remove, union, 
wildcards, etc.

I'd suggest a natural and intuitive extension of the current syntax. 
(while keeping all the current bits)

I already suggested a 'inverse' filter in a previous mail:

  echo "-schedule*" >> set_ftrace_filter

This rule operates on the current set of filter functions: it strikes out 
all existing filter functions that match this pattern.

To handle PIDs, we could do something like:

  echo "sshd-312:schedule" > set_ftrace_filter

This would restrict tracing to the sshd-pid:312 task.

Note: the PID portion of the filter rules still stay separate from the 
function names - we dont want per task function filter rules.

A natural variation would be:

  echo "312:schedule" > set_ftrace_filter

to only specify the PID, or:

  echo "312,313:schedule" > set_ftrace_filter

to specify two PIDs, or:

  echo "sshd:schedule" > set_ftrace_filter

to only specify the 'comm' part, which expands to all PIDs where 
task->comm matches sshd. Another variation would be:

  echo "loop*:schedule" > set_ftrace_filter

that matches all PIDs where task->comm matches loop*.

To specify recursive tracing, we could use something like:

  echo "loop*+schedule" > set_ftrace_filter

the '+' would signal that the 'schedule' function is 'expanded' and all 
its child functions are traced as well.

btw., maybe it makes sense to separate the regexp rule-set from the set 
of functions that we are tracing right now. For example:

  $ echo "schedule*" >  set_ftrace_filter
  $ echo "time*"     >> set_ftrace_filter
  $ echo "sys_*"     >> set_ftrace_filter

  $ cat set_ftrace_filter
  schedule*
  time*
  sys_*

We'd also have a separate, current_ftrace_functions file as well which 
shows all traced functions. (on a global basis - with possible PID filter 
rules added where applicable)

I know this will be hellishly hard to implement, but it would be _very_ 
elegant, and _very_ usable.

What do you think?

	Ingo

Re: [PATCH 0/3] ftrace: updates for tip

[Steven Rostedt]

On Thu, 4 Dec 2008, Ingo Molnar wrote:

> 
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > Ingo,
> > 
> > This series has three patches.
> > 
> > The first patch adds a new feature that I've been wanting to have for 
> > some time and Arjan even requested. That is to pick a function and only 
> > trace that function and its children. Dynamic ftrace and function graph 
> > needs to be enabled for this.
> > 
> > To do the above, I added a "trace" flags field in the task structure. 
> > The second patch uses this for the ftrace pid code. It searches for the 
> > task based on the pid and sets the trace flag, then in the ftrace 
> > function caller it only needs to check this flag.
> 
> Btw., i'd love to see this done via the regular regexp interface though, 
> if possible - instead of the add-on interface you added.

So would I. Unfortunately the regex is tightly coupled to turning on or 
off the function. This needs all functions enabled because we do not
know which functions the flagged one will call.

I could reuse the regex code if I add a call back to handle what to do on 
a match.  This is a bit more work, and will take some time to do.
If someone else has the time to do it, I would offer suggestions and 
review the code. Right now I do not have the time myself.

> 
> ( Also perhaps enable to toggle tracing via the /proc/<PID>/ hierarchy - 
>   a /proc/<PID>/tracing_enabled switch or so. )
> 
> Regarding the filter functions, the basic principle should be 
> mathematical set operations, like we have it now: add and remove, union, 
> wildcards, etc.
> 
> I'd suggest a natural and intuitive extension of the current syntax. 
> (while keeping all the current bits)
> 
> I already suggested a 'inverse' filter in a previous mail:
> 
>   echo "-schedule*" >> set_ftrace_filter

Ah, I did not see the '>>' that might be easier to do. I think you first
suggested this with a "!sched*" > set_ftrace_filter where the '>' would
truncate. But doing it with append '>>', might work.

> 
> This rule operates on the current set of filter functions: it strikes out 
> all existing filter functions that match this pattern.
> 
> To handle PIDs, we could do something like:
> 
>   echo "sshd-312:schedule" > set_ftrace_filter
> 
> This would restrict tracing to the sshd-pid:312 task.
> 
> Note: the PID portion of the filter rules still stay separate from the 
> function names - we dont want per task function filter rules.

Yep, agreed, A function is traced if the following conditions are true:

- function tracing is enabled
- the function is set to trace (not in set_ftrace_notrace)
- the pid filter is on and the current task has its trace bit set
 or the pid filter is off.
- the function filter is on and the function is in the trace array
 or the function filter is off

> 
> A natural variation would be:
> 
>   echo "312:schedule" > set_ftrace_filter
> 
> to only specify the PID, or:
> 
>   echo "312,313:schedule" > set_ftrace_filter
> 
> to specify two PIDs, or:
> 
>   echo "sshd:schedule" > set_ftrace_filter
> 
> to only specify the 'comm' part, which expands to all PIDs where 
> task->comm matches sshd. Another variation would be:
> 
>   echo "loop*:schedule" > set_ftrace_filter
> 
> that matches all PIDs where task->comm matches loop*.
> 
> To specify recursive tracing, we could use something like:
> 
>   echo "loop*+schedule" > set_ftrace_filter
> 
> the '+' would signal that the 'schedule' function is 'expanded' and all 
> its child functions are traced as well.
> 
> btw., maybe it makes sense to separate the regexp rule-set from the set 
> of functions that we are tracing right now. For example:
> 
>   $ echo "schedule*" >  set_ftrace_filter
>   $ echo "time*"     >> set_ftrace_filter
>   $ echo "sys_*"     >> set_ftrace_filter
> 
>   $ cat set_ftrace_filter
>   schedule*
>   time*
>   sys_*
> 
> We'd also have a separate, current_ftrace_functions file as well which 
> shows all traced functions. (on a global basis - with possible PID filter 
> rules added where applicable)
> 
> I know this will be hellishly hard to implement, but it would be _very_ 
> elegant, and _very_ usable.
> 
> What do you think?

Hmm, that is starting to get quite complex, just to use. This is something 
we need to experiment with to find the best solution. I'd like to know use 
cases first. Currently I have a simple program that forks, traces itself 
and execs code to trace. It is exectued like:

  ./trace-func ls -ltr

to trace "ls -lrt",  this code would become a little more complex with the 
above methods.  But I'm not set in stone in any of these options. I just 
do not want to spend the days coding this to find out no one uses any of 
it but what is already there.

-- Steve

[PATCH 0/3] ftrace: updates for tip

[Steven Rostedt]

Ingo,

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):
      ftrace: remove condition from ftrace_record_ip
      ftrace: disable ftrace on anomalies in trace start and stop
      ftrace: do not process freed records

----
 kernel/trace/ftrace.c |   91 +++++++++++++++++++++++++++---------------------
 1 files changed, 51 insertions(+), 40 deletions(-)

[PATCH 0/3] ftrace updates for tip

[Steven Rostedt]

[
  I added a bit more people to the Cc so that they are aware
  of the pending renames that are coming.

   Namely, trace_entries will be renamed to buffer_size
           iter_ctrl will be renamed to trace_options
]

Ingo,

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):
      ftrace: rename trace_entries to buffer_size
      ftrace: rename iter_ctrl to trace_options
      ftrace: CPU buffer start annotation clean ups

----
 Documentation/ftrace.txt |   32 ++++++++++++++++----------------
 kernel/trace/trace.c     |   38 ++++++++++++++++++++++++++------------
 kernel/trace/trace.h     |    1 +
 3 files changed, 43 insertions(+), 28 deletions(-)

Re: [PATCH 0/3] ftrace updates for tip

[Ingo Molnar]

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

> [
>   I added a bit more people to the Cc so that they are aware
>   of the pending renames that are coming.
> 
>    Namely, trace_entries will be renamed to buffer_size
>            iter_ctrl will be renamed to trace_options
> ]
> 
> Ingo,
> 
> 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):
>       ftrace: rename trace_entries to buffer_size
>       ftrace: rename iter_ctrl to trace_options
>       ftrace: CPU buffer start annotation clean ups

i've applied them to tip/tracing/ftrace, with some small changes:

 12ef7d4: ftrace: CPU buffer start annotation clean ups
 ee6bce5: ftrace: rename iter_ctrl to trace_options
 1696b2b: ftrace: show buffer size in kilobytes
 a94c80e: ftrace: rename trace_entries to buffer_size_kb

as per Arjan's suggestion i changed buffer_size to buffer_size_kb - 
and also removed the kilobytes string from its output.

thanks Steve!

	Ingo