| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190114045935.GA20802@sejong>
Date: Mon, 14 Jan 2019 13:59:35 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Tom Zanussi <zanussi@...nel.org>
Cc: rostedt@...dmis.org, tglx@...utronix.de, mhiramat@...nel.org,
vedang.patel@...el.com, bigeasy@...utronix.de,
joel@...lfernandes.org, mathieu.desnoyers@...icios.com,
julia@...com, linux-kernel@...r.kernel.org,
linux-rt-users@...r.kernel.org, kernel-team@....com
Subject: Re: [PATCH v11 10/15] tracing: Add alternative synthetic event trace
action syntax
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.zanussi@...ux.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.zanussi@...ux.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"
Powered by blists - more mailing lists