lkml.org 
[lkml]   [2012]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 -tip 2/4] tracing: replace static old_tracer with strcmp
Hi,

(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
>> tracers.
>
> 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
> descriptor
> 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 <hiraku.toyooka.gu@hitachi.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> 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 */
>> mutex_lock(&trace_types_lock);
>> - 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.


Thanks,
Hiraku Toyooka



\
 
 \ /
  Last update: 2012-12-21 12:21    [W:0.094 / U:1.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site