----- Original Message ----- > From: "Steven Rostedt" <[email protected]> > To: "Chunyu Hu" <[email protected]> > Cc: [email protected] > 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 <[email protected]> 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 <[email protected]> > > --- > > 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
