From mboxrd@z Thu Jan 1 00:00:00 1970 From: Namhyung Kim Subject: Re: [PATCH v11 10/15] tracing: Add alternative synthetic event trace action syntax Date: Mon, 14 Jan 2019 13:59:35 +0900 Message-ID: <20190114045935.GA20802@sejong> References: <20190111060752.GC625@sejong> <1547223940.7869.8.camel@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: rostedt@goodmis.org, tglx@linutronix.de, mhiramat@kernel.org, vedang.patel@intel.com, bigeasy@linutronix.de, joel@joelfernandes.org, mathieu.desnoyers@efficios.com, julia@ni.com, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, kernel-team@lge.com To: Tom Zanussi Return-path: Content-Disposition: inline In-Reply-To: <1547223940.7869.8.camel@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-rt-users.vger.kernel.org Hi Tom, On Fri, Jan 11, 2019 at 10:25:40AM -0600, Tom Zanussi wrote: > Hi Namhyung, > > On Fri, 2019-01-11 at 15:07 +0900, Namhyung Kim wrote: > > On Wed, Jan 09, 2019 at 01:49:17PM -0600, Tom Zanussi wrote: > > > From: Tom Zanussi > > > > > > Add a 'trace(synthetic_event_name, params)' alternative to > > > synthetic_event_name(params). > > > > > > Currently, the syntax used for generating synthetic events is to > > > invoke synthetic_event_name(params) i.e. use the synthetic event > > > name > > > as a function call. > > > > > > Users requested a new form that more explicitly shows that the > > > synthetic event is in effect being traced. In this version, a new > > > 'trace()' keyword is used, and the synthetic event name is passed > > > in > > > as the first argument. > > > > > > Signed-off-by: Tom Zanussi > > > --- > > > Documentation/trace/histogram.rst | 21 ++++++++++++++++++++ > > > kernel/trace/trace.c | 1 + > > > kernel/trace/trace_events_hist.c | 42 > > > +++++++++++++++++++++++++++++++++++---- > > > 3 files changed, 60 insertions(+), 4 deletions(-) > > > > > > diff --git a/Documentation/trace/histogram.rst > > > b/Documentation/trace/histogram.rst > > > index 79476c906b1a..4939bad1c1cd 100644 > > > --- a/Documentation/trace/histogram.rst > > > +++ b/Documentation/trace/histogram.rst > > > @@ -1874,6 +1874,7 @@ The available handlers are: > > > The available actions are: > > > > > > - (param list) - generate > > > synthetic event > > > + - trace(,(param list)) - generate > > > synthetic event > > > > Shouldn't it be > > > > "trace(,param list)" > > > > ? Otherwise it looks like we need two parentheses. > > Good point, I'll remove the params. > > > > > IMHO, it seems better for consistency using this new syntax only. > > Of course it should support the old syntax as well for compatibility > > (and maybe make it undocumented?). But I won't insist strongly.. > > > > OK, yeah, I really hate when things are undocumented, so I think > removing the documentation would be a step backward, but maybe changing > the emphasis to the trace() form would suffice. How about the below?: Looks good to me! Thanks, Namhyung > > > diff --git a/Documentation/trace/histogram.rst b/Documentation/trace/histogram.rst > index e5bcef360997..0ea59d45aef1 100644 > --- a/Documentation/trace/histogram.rst > +++ b/Documentation/trace/histogram.rst > @@ -1873,46 +1873,45 @@ The available handlers are: > > The available actions are: > > - - (param list) - generate synthetic event > - trace(,param list) - generate synthetic event > - save(field,...) - save current event fields > - snapshot() - snapshot the trace buffer > > The following commonly-used handler.action pairs are available: > > - - onmatch(matching.event).(param list) > - > - or > - > - onmatch(matching.event).trace(,param list) > > - The 'onmatch(matching.event).(params)' hist > - trigger action is invoked whenever an event matches and the > - histogram entry would be added or updated. It causes the named > - synthetic event to be generated with the values given in the > + The 'onmatch(matching.event).trace(,param > + list)' hist trigger action is invoked whenever an event matches > + and the histogram entry would be added or updated. It causes the > + named synthetic event to be generated with the values given in the > 'param list'. The result is the generation of a synthetic event > that consists of the values contained in those variables at the > - time the invoking event was hit. > - > - There are two equivalent forms available for generating synthetic > - events. In the first form, the synthetic event name is used as if > - it were a function name. For example, if the synthetic event name > - is 'wakeup_latency', the wakeup_latency event would be generated > - by invoking it as if it were a function call, with the event field > - values passed in as arguments: wakeup_latency(arg1,arg2). The > - second form simply uses the 'trace' keyword as the function name > - and passes in the synthetic event name as the first argument, > - followed by the field values: trace(wakeup_latency,arg1,arg2). > - > - The 'param list' consists of one or more parameters which may be > - either variables or fields defined on either the 'matching.event' > - or the target event. The variables or fields specified in the > - param list may be either fully-qualified or unqualified. If a > - variable is specified as unqualified, it must be unique between > - the two events. A field name used as a param can be unqualified > - if it refers to the target event, but must be fully qualified if > - it refers to the matching event. A fully-qualified name is of the > - form 'system.event_name.$var_name' or 'system.event_name.field'. > + time the invoking event was hit. For example, if the synthetic > + event name is 'wakeup_latency', a wakeup_latency event is > + generated using onmatch(event).trace(wakeup_latency,arg1,arg2). > + > + There is also an equivalent alternative form available for > + generating synthetic events. In this form, the synthetic event > + name is used as if it were a function name. For example, using > + the 'wakeup_latency' synthetic event name again, the > + wakeup_latency event would be generated by invoking it as if it > + were a function call, with the event field values passed in as > + arguments: onmatch(event).wakeup_latency(arg1,arg2). The syntax > + for this form is: > + > + onmatch(matching.event).(param list) > + > + In either case, the 'param list' consists of one or more > + parameters which may be either variables or fields defined on > + either the 'matching.event' or the target event. The variables or > + fields specified in the param list may be either fully-qualified > + or unqualified. If a variable is specified as unqualified, it > + must be unique between the two events. A field name used as a > + param can be unqualified if it refers to the target event, but > + must be fully qualified if it refers to the matching event. A > + fully-qualified name is of the form 'system.event_name.$var_name' > + or 'system.event_name.field'. > > The 'matching.event' specification is simply the fully qualified > event name of the event that matches the target event for the > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 84981b61d45f..8c220d97c214 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -4899,7 +4899,6 @@ static const char readme_msg[] = > "\t onmax(var) - invoke if var exceeds current max\n" > "\t onchange(var) - invoke action if var changes\n\n" > "\t The available actions are:\n\n" > - "\t (param list) - generate synthetic event\n" > "\t trace(,param list) - generate synthetic event\n" > "\t save(field,...) - save current event fields\n" > "\t snapshot() - snapshot the trace buffer\n"