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 <tom.zanu...@linux.intel.com> > > > > > > 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 <tom.zanu...@linux.intel.com> > > > --- > > > 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: > > > > > > - <synthetic_event_name>(param list) - generate > > > synthetic event > > > + - trace(<synthetic_event_name>,(param list)) - generate > > > synthetic event > > > > Shouldn't it be > > > > "trace(<synthetic_event_name>,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: > > - - <synthetic_event_name>(param list) - generate synthetic event > - trace(<synthetic_event_name>,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).<synthetic_event_name>(param list) > - > - or > - > - onmatch(matching.event).trace(<synthetic_event_name>,param list) > > - The 'onmatch(matching.event).<synthetic_event_name>(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(<synthetic_event_name>,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).<synthetic_event_name>(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 <synthetic_event>(param list) - generate synthetic > event\n" > "\t trace(<synthetic_event>,param list) - generate synthetic > event\n" > "\t save(field,...) - save current event > fields\n" > "\t snapshot() - snapshot the trace > buffer\n"
