lkml.org 
[lkml]   [2010]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH][RFC] tracing: Use class->reg() for all registering of events
    From
    Date
    On Tue, 2010-06-08 at 11:33 -0400, Steven Rostedt wrote:
    > I'm pushing this as an RFC first. This probably should be something that
    > makes it into 2.6.35.
    >
    > Acks and perhaps a little testing from the perf and kprobe angle?

    I'll have a look soon, but lets add Srikar to CC, he actually reported
    the problem :-)

    > Steven Rostedt (1):
    > tracing: Use class->reg() for all registering of events
    >
    > ----
    > include/linux/ftrace_event.h | 3 ++
    > include/trace/ftrace.h | 2 +
    > kernel/trace/trace_event_perf.c | 17 ++----------
    > kernel/trace/trace_events.c | 55 +++++++++++++++++++++++++-------------
    > 4 files changed, 44 insertions(+), 33 deletions(-)
    > ---------------------------
    > commit 0ab320dbe5894d7b4eec7d7da3a270abc68e4992
    > Author: Steven Rostedt <srostedt@redhat.com>
    > Date: Tue Jun 8 11:22:06 2010 -0400
    >
    > tracing: Use class->reg() for all registering of events
    >
    > Because kprobes and syscalls need special processing to register
    > events, the class->reg() method was created to handle the differences.
    >
    > But instead of creating a default ->reg for perf and ftrace events,
    > the code was scattered with:
    >
    > if (class->reg)
    > class->reg();
    > else
    > default_reg();
    >
    > This is messy and can also lead to bugs.
    >
    > This patch cleans up this code and creates a default reg() entry for
    > the events allowing for the code to directly call the class->reg()
    > without the condition.
    >
    > Reported-by: Peter Zijlstra <peterz@infradead.org>
    > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
    >
    > diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
    > index 3167f2d..a69bd32 100644
    > --- a/include/linux/ftrace_event.h
    > +++ b/include/linux/ftrace_event.h
    > @@ -146,6 +146,9 @@ struct ftrace_event_class {
    > int (*raw_init)(struct ftrace_event_call *);
    > };
    >
    > +extern int ftrace_event_reg(struct ftrace_event_call *event,
    > + enum trace_reg type);
    > +
    > enum {
    > TRACE_EVENT_FL_ENABLED_BIT,
    > TRACE_EVENT_FL_FILTERED_BIT,
    > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
    > index 5a64905..8815e27 100644
    > --- a/include/trace/ftrace.h
    > +++ b/include/trace/ftrace.h
    > @@ -439,6 +439,7 @@ static inline notrace int ftrace_get_offsets_##call( \
    > * .fields = LIST_HEAD_INIT(event_class_##call.fields),
    > * .raw_init = trace_event_raw_init,
    > * .probe = ftrace_raw_event_##call,
    > + * .reg = ftrace_event_reg,
    > * };
    > *
    > * static struct ftrace_event_call __used
    > @@ -567,6 +568,7 @@ static struct ftrace_event_class __used event_class_##call = { \
    > .fields = LIST_HEAD_INIT(event_class_##call.fields),\
    > .raw_init = trace_event_raw_init, \
    > .probe = ftrace_raw_event_##call, \
    > + .reg = ftrace_event_reg, \
    > _TRACE_PERF_INIT(call) \
    > };
    >
    > diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
    > index e6f6588..de2f170 100644
    > --- a/kernel/trace/trace_event_perf.c
    > +++ b/kernel/trace/trace_event_perf.c
    > @@ -56,13 +56,7 @@ static int perf_trace_event_init(struct ftrace_event_call *tp_event,
    > }
    > }
    >
    > - if (tp_event->class->reg)
    > - ret = tp_event->class->reg(tp_event, TRACE_REG_PERF_REGISTER);
    > - else
    > - ret = tracepoint_probe_register(tp_event->name,
    > - tp_event->class->perf_probe,
    > - tp_event);
    > -
    > + ret = tp_event->class->reg(tp_event, TRACE_REG_PERF_REGISTER);
    > if (ret)
    > goto fail;
    >
    > @@ -96,7 +90,7 @@ int perf_trace_init(struct perf_event *p_event)
    > mutex_lock(&event_mutex);
    > list_for_each_entry(tp_event, &ftrace_events, list) {
    > if (tp_event->event.type == event_id &&
    > - tp_event->class && tp_event->class->perf_probe &&
    > + tp_event->class && tp_event->class->reg &&
    > try_module_get(tp_event->mod)) {
    > ret = perf_trace_event_init(tp_event, p_event);
    > break;
    > @@ -136,12 +130,7 @@ void perf_trace_destroy(struct perf_event *p_event)
    > if (--tp_event->perf_refcount > 0)
    > goto out;
    >
    > - if (tp_event->class->reg)
    > - tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER);
    > - else
    > - tracepoint_probe_unregister(tp_event->name,
    > - tp_event->class->perf_probe,
    > - tp_event);
    > + tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER);
    >
    > /*
    > * Ensure our callback won't be called anymore. See
    > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
    > index 45a8968..b353639 100644
    > --- a/kernel/trace/trace_events.c
    > +++ b/kernel/trace/trace_events.c
    > @@ -132,6 +132,35 @@ int trace_event_raw_init(struct ftrace_event_call *call)
    > }
    > EXPORT_SYMBOL_GPL(trace_event_raw_init);
    >
    > +int ftrace_event_reg(struct ftrace_event_call *call, enum trace_reg type)
    > +{
    > + switch (type) {
    > + case TRACE_REG_REGISTER:
    > + return tracepoint_probe_register(call->name,
    > + call->class->probe,
    > + call);
    > + case TRACE_REG_UNREGISTER:
    > + tracepoint_probe_unregister(call->name,
    > + call->class->probe,
    > + call);
    > + return 0;
    > +
    > +#ifdef CONFIG_PERF_EVENTS
    > + case TRACE_REG_PERF_REGISTER:
    > + return tracepoint_probe_register(call->name,
    > + call->class->perf_probe,
    > + call);
    > + case TRACE_REG_PERF_UNREGISTER:
    > + tracepoint_probe_unregister(call->name,
    > + call->class->perf_probe,
    > + call);
    > + return 0;
    > +#endif
    > + }
    > + return 0;
    > +}
    > +EXPORT_SYMBOL_GPL(ftrace_event_reg);
    > +
    > static int ftrace_event_enable_disable(struct ftrace_event_call *call,
    > int enable)
    > {
    > @@ -142,23 +171,13 @@ static int ftrace_event_enable_disable(struct ftrace_event_call *call,
    > if (call->flags & TRACE_EVENT_FL_ENABLED) {
    > call->flags &= ~TRACE_EVENT_FL_ENABLED;
    > tracing_stop_cmdline_record();
    > - if (call->class->reg)
    > - call->class->reg(call, TRACE_REG_UNREGISTER);
    > - else
    > - tracepoint_probe_unregister(call->name,
    > - call->class->probe,
    > - call);
    > + call->class->reg(call, TRACE_REG_UNREGISTER);
    > }
    > break;
    > case 1:
    > if (!(call->flags & TRACE_EVENT_FL_ENABLED)) {
    > tracing_start_cmdline_record();
    > - if (call->class->reg)
    > - ret = call->class->reg(call, TRACE_REG_REGISTER);
    > - else
    > - ret = tracepoint_probe_register(call->name,
    > - call->class->probe,
    > - call);
    > + ret = call->class->reg(call, TRACE_REG_REGISTER);
    > if (ret) {
    > tracing_stop_cmdline_record();
    > pr_info("event trace: Could not enable event "
    > @@ -196,8 +215,7 @@ static int __ftrace_set_clr_event(const char *match, const char *sub,
    > mutex_lock(&event_mutex);
    > list_for_each_entry(call, &ftrace_events, list) {
    >
    > - if (!call->name || !call->class ||
    > - (!call->class->probe && !call->class->reg))
    > + if (!call->name || !call->class || !call->class->reg)
    > continue;
    >
    > if (match &&
    > @@ -323,7 +341,7 @@ t_next(struct seq_file *m, void *v, loff_t *pos)
    > * The ftrace subsystem is for showing formats only.
    > * They can not be enabled or disabled via the event files.
    > */
    > - if (call->class && (call->class->probe || call->class->reg))
    > + if (call->class && call->class->reg)
    > return call;
    > }
    >
    > @@ -476,8 +494,7 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
    >
    > mutex_lock(&event_mutex);
    > list_for_each_entry(call, &ftrace_events, list) {
    > - if (!call->name || !call->class ||
    > - (!call->class->probe && !call->class->reg))
    > + if (!call->name || !call->class || !call->class->reg)
    > continue;
    >
    > if (system && strcmp(call->class->system, system) != 0)
    > @@ -1037,12 +1054,12 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
    > return -1;
    > }
    >
    > - if (call->class->probe || call->class->reg)
    > + if (call->class->reg)
    > trace_create_file("enable", 0644, call->dir, call,
    > enable);
    >
    > #ifdef CONFIG_PERF_EVENTS
    > - if (call->event.type && (call->class->perf_probe || call->class->reg))
    > + if (call->event.type && call->class->reg)
    > trace_create_file("id", 0444, call->dir, call,
    > id);
    > #endif
    >
    >



    \
     
     \ /
      Last update: 2010-06-08 18:01    [W:0.035 / U:17.700 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site