Re: [PATCH v3 -tip 2/4] tracing: replace static old_tracer with strcmp

From: Hiraku Toyooka
Date: Fri Dec 21 2012 - 05:56:10 EST


(12/21/2012 12:04 PM), Steven Rostedt wrote:
On Wed, 2012-12-19 at 16:02 +0900, Hiraku Toyooka wrote:
Currently, read functions for trace buffer use static "old_tracer"
for detecting changes of current tracer. This is because we can
assume that these functions are used from only one file ("trace").

But we are adding snapshot feature for ftrace, then those functions
are called from two files. So we remove all static "old_tracer", and
replace those with string comparison between current and previous

I reworded what you wrote:

Thank you very much.

Currently the trace buffer read functions use a static variable
"old_tracer" for detecting if the current tracer changes. This was
suitable for a single trace file ("trace"), but to add a snapshot
feature that will use the same function for its file, a check against
a static variable is not sufficient.

To use the output functions for two different files, instead of storing
the current tracer in a static variable, as the trace iterator
contains a pointer to the original current tracer's name, that pointer
can now be used to check if the current tracer has changed between
different reads of the trace file.

And I also realized a more efficient approach.

Signed-off-by: Hiraku Toyooka <>
Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: linux-kernel@xxxxxxxxxxxxxxx
kernel/trace/trace.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index a8ce008..8d05a44 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1948,7 +1948,6 @@ void tracing_iter_reset(struct trace_iterator *iter, int cpu)
static void *s_start(struct seq_file *m, loff_t *pos)
struct trace_iterator *iter = m->private;
- static struct tracer *old_tracer;
int cpu_file = iter->cpu_file;
void *p = NULL;
loff_t l = 0;
@@ -1956,10 +1955,9 @@ static void *s_start(struct seq_file *m, loff_t *pos)

/* copy the tracer to avoid using a global lock all around */
- if (unlikely(old_tracer != current_trace && current_trace)) {
- old_tracer = current_trace;
+ if (unlikely(current_trace &&
+ strcmp(iter->trace->name, current_trace->name) != 0))
*iter->trace = *current_trace;

As iter->trace is a copy of current_trace, it points to everything that
the current_trace pointed to. As the current_trace->name is a pointer to
a const char string that never changes, we don't need to do the strcmp()
you can simply do a direct comparison:

if (unlikely(current_trace && iter->trace->name != current_trace->name)) {

Yes. I'll update my patch to use this efficient way.

I would add a comment about that too:
* iter->trace is a copy of current_trace, the pointer to the
* name may be used instead of a strcmp(), as iter->trace->name
* will point to the same string as current_trace->name.

OK. I'll include this comment in my patch.

Hiraku Toyooka

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at