[Patch 3/3] Tracing/ftrace: Replace none tracer by nop tracer

[Patch 3/3] Tracing/ftrace: Replace none tracer by nop tracer

[Frédéric Weisbecker]

Replace "none" tracer by the recently created "nop" tracer. Both are pretty similar except that nop accepts TRACE_PRINT or TRACE_SPECIAL entries.
And as a consequence, changing the size of the ring buffer now requires that tracing has already been disabled.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
diff -rup linux-2.6-tip (2)/kernel/trace/trace.c linux-2.6-tip/kernel/trace/trace.c
--- linux-2.6-tip (2)/kernel/trace/trace.c	2008-09-18 18:40:00.000000000 +0200
+++ linux-2.6-tip/kernel/trace/trace.c	2008-09-20 17:13:08.000000000 +0200
@@ -142,24 +142,6 @@ static DECLARE_WAIT_QUEUE_HEAD(trace_wai
 /* trace_flags holds iter_ctrl options */

 unsigned long trace_flags = TRACE_ITER_PRINT_PARENT;

 

-static notrace void no_trace_init(struct trace_array *tr)

-{

-	int cpu;

-

-	ftrace_function_enabled = 0;

-	if(tr->ctrl)

-		for_each_online_cpu(cpu)

-			tracing_reset(tr->data[cpu]);

-	tracer_enabled = 0;

-}

-

-/* dummy trace to disable tracing */

-static struct tracer no_tracer __read_mostly = {

-	.name		= "none",

-	.init		= no_trace_init

-};

-

-

 /**

  * trace_wake_up - wake up tasks waiting for trace input

  *

@@ -962,7 +944,7 @@ ftrace_special(unsigned long arg1, unsig
 	long disabled;

 	int cpu;

 

-	if (tracing_disabled || current_trace == &no_tracer || !tr->ctrl)

+	if (tracing_disabled || !tr->ctrl)

 		return;

 

 	local_irq_save(flags);

@@ -2794,7 +2776,8 @@ tracing_entries_write(struct file *filp,
 {

 	unsigned long val;

 	char buf[64];

-	int i, ret;

+	int i, ret;
+	struct trace_array *tr = filp->private_data;

 

 	if (cnt >= sizeof(buf))

 		return -EINVAL;

@@ -2814,9 +2797,9 @@ tracing_entries_write(struct file *filp,
 

 	mutex_lock(&trace_types_lock);

 

-	if (current_trace != &no_tracer) {

+	if (tr->ctrl) {

 		cnt = -EBUSY;

-		pr_info("ftrace: set current_tracer to none"

+		pr_info("ftrace: please disable tracing"

 			" before modifying buffer size\n");

 		goto out;

 	}

@@ -2879,20 +2862,6 @@ tracing_entries_write(struct file *filp,
 	return cnt;

 }

 

-static int tracing_open_mark(struct inode *inode, struct file *filp)

-{

-	int ret;

-

-	ret = tracing_open_generic(inode, filp);

-	if (ret)

-		return ret;

-

-	if (current_trace == &no_tracer)

-		return -ENODEV;

-

-	return 0;

-}

-

 static int mark_printk(const char *fmt, ...)

 {

 	int ret;

@@ -2911,7 +2880,7 @@ tracing_mark_write(struct file *filp, co
 	char *end;

 	struct trace_array *tr = &global_trace;

 

-	if (current_trace == &no_tracer || !tr->ctrl || tracing_disabled)

+	if (!tr->ctrl || tracing_disabled)

 		return -EINVAL;

 

 	if (cnt > TRACE_BUF_SIZE)

@@ -2971,7 +2940,7 @@ static struct file_operations tracing_en
 };

 

 static struct file_operations tracing_mark_fops = {

-	.open		= tracing_open_mark,

+	.open		= tracing_open_generic,

 	.write		= tracing_mark_write,

 };

 

@@ -3123,7 +3092,7 @@ int trace_vprintk(unsigned long ip, cons
 	long disabled;

 	int cpu, len = 0, write, written = 0;

 

-	if (current_trace == &no_tracer || !tr->ctrl || tracing_disabled)

+	if (!tr->ctrl || tracing_disabled)

 		return 0;

 

 	local_irq_save(flags);

@@ -3539,8 +3508,8 @@ __init static int tracer_alloc_buffers(v
 

 	trace_init_cmdlines();

 

-	register_tracer(&no_tracer);

-	current_trace = &no_tracer;

+	register_tracer(&nop_trace);

+	current_trace = &nop_trace;

 

 	/* All seems OK, enable tracing */

 	global_trace.ctrl = tracer_enabled;

diff -rup linux-2.6-tip (2)/kernel/trace/trace.h linux-2.6-tip/kernel/trace/trace.h
--- linux-2.6-tip (2)/kernel/trace/trace.h	2008-09-19 18:01:26.000000000 +0200
+++ linux-2.6-tip/kernel/trace/trace.h	2008-09-20 17:23:36.000000000 +0200
@@ -384,5 +384,7 @@ enum trace_iterator_flags {

 	TRACE_ITER_SCHED_TREE		= 0x200,

 	TRACE_ITER_PRINTK		= 0x400,

 };

+
+extern struct tracer nop_trace;
 

 #endif /* _LINUX_KERNEL_TRACE_H */

diff -rup linux-2.6-tip (2)/kernel/trace/trace_nop.c linux-2.6-tip/kernel/trace/trace_nop.c
--- linux-2.6-tip (2)/kernel/trace/trace_nop.c	2008-09-19 18:01:26.000000000 +0200
+++ linux-2.6-tip/kernel/trace/trace_nop.c	2008-09-21 00:44:47.000000000 +0200

@@ -47,7 +51,7 @@ static void nop_trace_ctrl_update(struct
 		stop_nop_trace(tr);

 }

 

-static struct tracer nop_trace __read_mostly =

+struct tracer nop_trace __read_mostly =

 {

 	.name		= "nop",

 	.init		= nop_trace_init,

@@ -57,9 +61,4 @@ static struct tracer nop_trace __read_mo
 	.selftest	= trace_selftest_startup_nop,

 #endif

 };

-

-__init static int init_nop_trace(void)

-{

-	return register_tracer(&nop_trace);

-}

-device_initcall(init_nop_trace);

+

Re: [Patch 3/3] Tracing/ftrace: Replace none tracer by nop tracer

[Ingo Molnar]

* Frédéric Weisbecker <fweisbec@gmail.com> wrote:

> Replace "none" tracer by the recently created "nop" tracer. Both are 
> pretty similar except that nop accepts TRACE_PRINT or TRACE_SPECIAL 
> entries. And as a consequence, changing the size of the ring buffer 
> now requires that tracing has already been disabled.

btw., if people are interested in new tracers, it would be useful to 
have an initcall tracer which can capture boot delays and schedule 
events from bootup. -tip has the 'fastboot' feature topic which could 
make use of such a tracer.

Right now fastboot profiling works like this: it (abu-)uses the 
initcall_debug boot parameter to get such special printouts in 
init/main.c:

                printk("initcall %pF returned %d after %Ld msecs\n", fn,
                        result, (unsigned long long) delta.tv64 >> 20);

and relies on CONFIG_PRINTK_TIME to get event timestamps. Then, after 
bootup, scripts/bootgraph.pl can be used to draw a graph of bootup 
dependencies and timings.

the efficiency of this mechanism could be extended significantly via an 
ftrace/fastboot plugin: it would allow non-initcall delays to be traced, 
plus the _reason_ for the delays as well. Stackframe tracing could be 
enabled by default (see 'stacktrace' / TRACE_ITER_STACKTRACE / 
__trace_stack() in kernel/tracing/trace.c - not to be confused with the 
stack-footprint tracer ftrace/stacktrace), and context-switch events 
could be / should be traced as well.

the ideal workflow would be like this: developers optimizing fastboot 
details would enable a single kernel option: CONFIG_TRACE_BOOT, and that 
would be an 'all in one' switch. Tracing would occur by default (no 
extra boot parameters), and /debug/tracing/trace would have output 
suitable for scripts/bootgraph.pl parsing.

(at least) one architectural detail that needs to be solved here is the 
fact that ftrace is initialized via an initcall right now. It should 
probably be initialized via an early-initcall instead.

Hm?

	Ingo

Re: [Patch 3/3] Tracing/ftrace: Replace none tracer by nop tracer

[Frédéric Weisbecker]

2008/9/22 Ingo Molnar <mingo@elte.hu>:
> btw., if people are interested in new tracers, it would be useful to
> have an initcall tracer which can capture boot delays and schedule
> events from bootup. -tip has the 'fastboot' feature topic which could
> make use of such a tracer.
>
> Right now fastboot profiling works like this: it (abu-)uses the
> initcall_debug boot parameter to get such special printouts in
> init/main.c:
>
>                printk("initcall %pF returned %d after %Ld msecs\n", fn,
>                        result, (unsigned long long) delta.tv64 >> 20);
>
> and relies on CONFIG_PRINTK_TIME to get event timestamps. Then, after
> bootup, scripts/bootgraph.pl can be used to draw a graph of bootup
> dependencies and timings.
>
> the efficiency of this mechanism could be extended significantly via an
> ftrace/fastboot plugin: it would allow non-initcall delays to be traced,
> plus the _reason_ for the delays as well. Stackframe tracing could be
> enabled by default (see 'stacktrace' / TRACE_ITER_STACKTRACE /
> __trace_stack() in kernel/tracing/trace.c - not to be confused with the
> stack-footprint tracer ftrace/stacktrace), and context-switch events
> could be / should be traced as well.
>
> the ideal workflow would be like this: developers optimizing fastboot
> details would enable a single kernel option: CONFIG_TRACE_BOOT, and that
> would be an 'all in one' switch. Tracing would occur by default (no
> extra boot parameters), and /debug/tracing/trace would have output
> suitable for scripts/bootgraph.pl parsing.
>
> (at least) one architectural detail that needs to be solved here is the
> fact that ftrace is initialized via an initcall right now. It should
> probably be initialized via an early-initcall instead.
>
> Hm?
>
>        Ingo
>

Sounds really interesting. I will try to write this tracer in next days.
But first I will try to solve this initcall problem...

(Sorry Ingo, I forgot to cc LKML)

Frédéric.