On Fri, 22 Nov 2013 23:33:29 +0900
Namhyung Kim <[email protected]> wrote:
> 2013-11-21 (목), 12:01 +0100, Jiri Olsa:
> > Backporting hrtimer plugin.
> >
> > Backported from Steven Rostedt's trace-cmd repo (HEAD 0f2c2fb):
> > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git
> >
> > This plugin adds function field resolving for following
> > tracepoint events:
> > timer:hrtimer_expire_entry
> > timer:hrtimer_start
> >
> > The diff of 'perf script' output generated by old and new code:
> > (data was generated by 'perf record -e 'timer:hrtimer*' -a')
> >
> > --- script.hrtimer.old
> > +++ script.hrtimer.new
> > - swapper 0 [000] 27405.519092: timer:hrtimer_start: [FAILED TO
> > PARSE] hrtimer=0xffff88021e20e800 function=0xffffffff810c0e10
> > expires=27398383000000 softexpires=27398383000000
> > + swapper 0 [000] 27405.519103: timer:hrtimer_start:
> > hrtimer=0xffff88021e20e800 function=tick_sched_timer expires=27398383000000
> > softexpires=27398383000000
> > - swapper 0 [001] 27405.519544: timer:hrtimer_expire_entry:
> > [FAILED TO PARSE] hrtimer=0xffff880211334058 now=27398294182491
> > function=0xffffffff81086f20
> > + swapper 0 [001] 27405.519544: timer:hrtimer_expire_entry:
> > hrtimer=0xffff880211334058 now=27398294182491 function=posix_timer_fn/0x0
> >
> > Check the 'function' field is translated into the function name.
>
> [SNIP]
> > + if (!fn) {
> > + trace_seq_printf(s, " function=MISSING");
> > + } else {
> > + unsigned long long function;
> > + const char *func;
> > +
> > + if (pevent_read_number_field(fn, data, &function))
> > + trace_seq_printf(s, " function=INVALID");
> > +
> > + func = pevent_find_function(pevent, function);
> > +
> > + trace_seq_printf(s, " function=%s", func);
>
> Shouldn't it be like below?
>
> if (pevent_read_number_field(fn, data, &function))
> trace_seq_printf(s, " function=INVALID");
> else {
> func = pevent_find_function(pevent, function);
> trace_seq_printf(s, " function=%s", func);
> }
>
> Otherwise it'll print "function=XXX" twice if pevent_read_number_field()
> failed.
>
Nice catch, but lets keep the backport and fixes separate.
Actually, this was one of the first plugins I wrote, and since then, I
added a function tat does this for you. The proper change is to rip
this out and use:
pevent_print_func_field()
instead. That function does the checks properly.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
