Messages in this thread | | | Date | Mon, 25 Nov 2013 10:46:18 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH 04/22] tools lib traceevent: Add jbd2 plugin |
| |
On Sat, Nov 23, 2013 at 03:52:21AM -0500, Steven Rostedt wrote: > On Fri, 22 Nov 2013 23:27:57 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > > > [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? > > Yeah, but that was my mistake. Lets keep the fix separate from the > backport though. Thanks!
ok
> > > > > > > > +{ > > > + unsigned int dev = args[0]; > > > + > > > + trace_seq_printf(s, "%d:%d", MAJOR(dev), MINOR(dev));
SNIP
> > jbd2: remove jbd2_dev_to_name() from jbd2 tracepoints > > Using function calls in TP_printk causes perf heartburn, so print the > MAJOR/MINOR device numbers instead. > > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
yep, it's rather old change (v3.1-rc1) but I thought we want to keep that for backward compatibility
and it's small piece of code anyway.. ;-)
> > > > > > > > > > + "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? > > Probably ;-)
will add,
thanks, jirka
| |