Messages in this thread Patch in this message | | | Date | Tue, 3 May 2016 07:29:36 -0400 (EDT) | From | Chunyu Hu <> | Subject | Re: [PATCH] tracing: Don't display trigger file for events that don't have register func |
| |
----- Original Message ----- > From: "Steven Rostedt" <rostedt@goodmis.org> > To: "Chunyu Hu" <chuhu@redhat.com> > Cc: linux-kernel@vger.kernel.org > Sent: Tuesday, May 3, 2016 10:42:44 AM > Subject: Re: [PATCH] tracing: Don't display trigger file for events that don't have register func > > On Mon, 2 May 2016 22:14:14 +0800 > Chunyu Hu <chuhu@redhat.com> wrote: > > > Currently register function of the event will be called > > through the 'reg' field of event class directly without > > any check when seting up triggers. > > > > Triggers for events that don't support register through > > debug fs (events under events/ftrace are for perf to > > Actually, they were created for trace-cmd. I'm not even sure if perf > uses the ftrace event formats.
Thanks for correcting me. I misunderstood it. > > > read event format, and most of them don't have regisgter > > function except events/ftrace/function.) can't be enabled > > at all, and an oops will be hit when setting up trigger > > for those events, so just not showing them is an easy way > > to avoid the oops. > > > > Signed-off-by: Chunyu Hu <chuhu@redhat.com> > > --- > > kernel/trace/trace_events.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c > > index da1eeb6..9fb99fd 100644 > > --- a/kernel/trace/trace_events.c > > +++ b/kernel/trace/trace_events.c > > @@ -2138,9 +2138,10 @@ event_create_dir(struct dentry *parent, struct > > trace_event_file *file) > > trace_create_file("filter", 0644, file->dir, file, > > &ftrace_event_filter_fops); > > > > - trace_create_file("trigger", 0644, file->dir, file, > > - &event_trigger_fops); > > - > > + if (call->class->reg) { > > + trace_create_file("trigger", 0644, file->dir, file, > > + &event_trigger_fops); > > + } > > As you stated, reg is there for function tracing, and is not a good > value to use as a check. Use the following check instead: > > if (!(call->flags & TRACE_EVENT_FL_IGNORE_ENABLE)) > trace_create_file("trigger", 0644, file->dir, file, > &event_trigger_fops);
Thanks. Agree, will prepare v2 for this.
It looks like that checking both is safest way, like other parts did in trace_events.c. but i didn't see other events using TRACE_EVENT_FL_IGNORE_ENABLE, ftrace event sub system is the only user now. and didn't see other events that don't have register func. so i think it's safe to use only this flag to check it.
> And add a comment that states that only event directories that can be > enabled should have triggers.
Ok.
> -- Steve > > > > > #ifdef CONFIG_HIST_TRIGGERS > > trace_create_file("hist", 0444, file->dir, file, > > &event_hist_fops); > >
-- Regards, Chunyu Hu
| |