Messages in this thread | | | Subject | Re: [PATCH 04/22] tools lib traceevent: Add jbd2 plugin | From | Namhyung Kim <> | Date | Fri, 22 Nov 2013 23:27:57 +0900 |
| |
2013-11-21 (목), 12:01 +0100, Jiri Olsa: > Backporting jbd2 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 field resolving functions for following > tracepoint events: > jbd2:jbd2_checkpoint_stats > jbd2:jbd2_run_stats > > The diff of 'perf script' output generated by old and new code: > (data was generated by 'perf record -e 'jbd2:jbd2_run_stats,jbd2:jbd2_checkpoint_stats' -a') > > --- script.jbd2.old > +++ script.jbd2.new > - jbd2/dm-3-8 576 [000] 2983.748423: jbd2:jbd2_checkpoint_stats: [FAILED TO PARSE] dev=265289731 tid=0x3f2bbb chp_time=0x0 forced_to_close=0 written=0 dropped=2 > + jbd2/dm-3-8 576 [000] 2983.748423: jbd2:jbd2_checkpoint_stats: dev 253,3 tid 4139963 chp_time 0 forced_to_close 0 written 0 dropped 2 > - jbd2/dm-3-8 576 [000] 2983.852789: jbd2:jbd2_run_stats: [FAILED TO PARSE] dev=265289731 tid=0x3f2bc0 wait=0x0 request_delay=0x0 running=0x138d locked=0x0 flushing=0x0 logging=0x68 handle_count=2995 blocks=17 blocks_logged=18 > + jbd2/dm-3-8 576 [000] 2983.852789: jbd2:jbd2_run_stats: dev 253,3 tid 4139968 wait 0 request_delay 0 running 5005 locked 0 flushing 0 logging 104 handle_count 2995 blocks 17 blocks_logged 18 > > This plugin also adds jbd2_dev_to_name function, which > was removed from jdb2 tracepoints recently. Keeping it > in for backward compatibility reasons.
[SNIP] > +#define MINORBITS 20 > +#define MINORMASK ((1U << MINORBITS) - 1) > + > +#define MAJOR(dev) ((unsigned int) ((dev) >> MINORBITS)) > +#define MINOR(dev) ((unsigned int) ((dev) & MINORMASK)) > + > +unsigned long long process_jbd2_dev_to_name(struct trace_seq *s, > + unsigned long long *args)
Isn't it better to make these functions static?
> +{ > + unsigned int dev = args[0]; > + > + trace_seq_printf(s, "%d:%d", MAJOR(dev), MINOR(dev)); > + return 0; > +} > + > +unsigned long long process_jiffies_to_msecs(struct trace_seq *s, > + unsigned long long *args) > +{ > + unsigned long long jiffies = args[0]; > + > + trace_seq_printf(s, "%lld", jiffies); > + return jiffies; > +} > + > +int PEVENT_PLUGIN_LOADER(struct pevent *pevent) > +{ > + pevent_register_print_function(pevent, > + process_jbd2_dev_to_name, > + PEVENT_FUNC_ARG_STRING,
Actually the function returns long long not string. But it seems the current code doesn't care about the return type.
> + "jbd2_dev_to_name", > + PEVENT_FUNC_ARG_INT, > + PEVENT_FUNC_ARG_VOID); > + > + pevent_register_print_function(pevent, > + process_jiffies_to_msecs, > + PEVENT_FUNC_ARG_LONG, > + "jiffies_to_msecs", > + PEVENT_FUNC_ARG_LONG, > + PEVENT_FUNC_ARG_VOID); > + return 0; > +}
Shouldn't it unregister these functions when unloaded?
Thanks, Namhyung
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |