From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755667AbcECL3l (ORCPT ); Tue, 3 May 2016 07:29:41 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:50740 "EHLO mx5-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755328AbcECL3k (ORCPT ); Tue, 3 May 2016 07:29:40 -0400 Date: Tue, 3 May 2016 07:29:36 -0400 (EDT) From: Chunyu Hu Reply-To: Chunyu Hu To: Steven Rostedt Cc: linux-kernel@vger.kernel.org Message-ID: <2087997675.42526586.1462274976826.JavaMail.zimbra@redhat.com> In-Reply-To: <20160502224244.1b17428c@grimm.local.home> References: <1462198454-17342-1-git-send-email-chuhu@redhat.com> <20160502224244.1b17428c@grimm.local.home> Subject: Re: [PATCH] tracing: Don't display trigger file for events that don't have register func MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.11.5.35] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF38 (Linux)/8.0.6_GA_5922) Thread-Topic: tracing: Don't display trigger file for events that don't have register func Thread-Index: SA2BTGsHAWueCl7NB0aI07xXrtH/Ig== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- Original Message ----- > From: "Steven Rostedt" > To: "Chunyu Hu" > 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 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 > > --- > > 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