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.zanussi@xxxxxxxxxxxxxxx>
> >
> > 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.zanussi@xxxxxxxxxxxxxxx>
> > ---
> > 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?:
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"
