[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1507730855.8849.18.camel@tzanussi-mobl.amr.corp.intel.com>
Date: Wed, 11 Oct 2017 09:07:35 -0500
From: Tom Zanussi <tom.zanussi@...ux.intel.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: rostedt@...dmis.org, tglx@...utronix.de, mhiramat@...nel.org,
vedang.patel@...el.com, bigeasy@...utronix.de,
joel.opensrc@...il.com, joelaf@...gle.com,
mathieu.desnoyers@...icios.com, baohong.liu@...el.com,
rajvi.jingar@...el.com, linux-kernel@...r.kernel.org,
linux-rt-users@...r.kernel.org, kernel-team@....com
Subject: Re: [PATCH v3 19/33] tracing: Add variable reference handling to
hist triggers
Hi Namhyung,
On Mon, 2017-10-09 at 21:46 +0900, Namhyung Kim wrote:
> On Fri, Sep 22, 2017 at 02:59:59PM -0500, Tom Zanussi wrote:
> > Add the necessary infrastructure to allow the variables defined on one
> > event to be referenced in another. This allows variables set by a
> > previous event to be referenced and used in expressions combining the
> > variable values saved by that previous event and the event fields of
> > the current event. For example, here's how a latency can be
> > calculated and saved into yet another variable named 'wakeup_lat':
> >
> > # echo 'hist:keys=pid,prio:ts0=common_timestamp ...
> > # echo 'hist:keys=next_pid:wakeup_lat=common_timestamp-$ts0 ...
> >
> > In the first event, the event's timetamp is saved into the variable
> > ts0. In the next line, ts0 is subtracted from the second event's
> > timestamp to produce the latency.
> >
> > Further users of variable references will be described in subsequent
> > patches, such as for instance how the 'wakeup_lat' variable above can
> > be displayed in a latency histogram.
> >
> > Signed-off-by: Tom Zanussi <tom.zanussi@...ux.intel.com>
> > ---
> > kernel/trace/trace.c | 2 +
> > kernel/trace/trace.h | 3 +
> > kernel/trace/trace_events_hist.c | 613 ++++++++++++++++++++++++++++++++----
> > kernel/trace/trace_events_trigger.c | 6 +
> > 4 files changed, 568 insertions(+), 56 deletions(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 19ffbe1..5eb313a 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -7753,6 +7753,7 @@ static int instance_mkdir(const char *name)
> >
> > INIT_LIST_HEAD(&tr->systems);
> > INIT_LIST_HEAD(&tr->events);
> > + INIT_LIST_HEAD(&tr->hist_vars);
> >
> > if (allocate_trace_buffers(tr, trace_buf_size) < 0)
> > goto out_free_tr;
> > @@ -8500,6 +8501,7 @@ __init static int tracer_alloc_buffers(void)
> >
> > INIT_LIST_HEAD(&global_trace.systems);
> > INIT_LIST_HEAD(&global_trace.events);
> > + INIT_LIST_HEAD(&global_trace.hist_vars);
> > list_add(&global_trace.list, &ftrace_trace_arrays);
> >
> > apply_trace_boot_options();
> > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > index 02bfd5c..7b78762 100644
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -273,6 +273,7 @@ struct trace_array {
> > int function_enabled;
> > #endif
> > int time_stamp_abs_ref;
> > + struct list_head hist_vars;
> > };
> >
> > enum {
> > @@ -1547,6 +1548,8 @@ extern int save_named_trigger(const char *name,
> > extern void unpause_named_trigger(struct event_trigger_data *data);
> > extern void set_named_trigger_data(struct event_trigger_data *data,
> > struct event_trigger_data *named_data);
> > +extern struct event_trigger_data *
> > +get_named_trigger_data(struct event_trigger_data *data);
> > extern int register_event_command(struct event_command *cmd);
> > extern int unregister_event_command(struct event_command *cmd);
> > extern int register_trigger_hist_enable_disable_cmds(void);
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index 02170df..6dcef22 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -60,6 +60,9 @@ struct hist_field {
> > struct hist_var var;
> > enum field_op_id operator;
> > char *name;
> > + unsigned int var_idx;
> > + unsigned int var_ref_idx;
> > + bool read_once;
> > };
> >
> > static u64 hist_field_none(struct hist_field *field,
> > @@ -215,6 +218,7 @@ enum hist_field_flags {
> > HIST_FIELD_FL_VAR = 1 << 12,
> > HIST_FIELD_FL_VAR_ONLY = 1 << 13,
> > HIST_FIELD_FL_EXPR = 1 << 14,
> > + HIST_FIELD_FL_VAR_REF = 1 << 15,
> > };
> >
> > struct var_defs {
> > @@ -255,6 +259,8 @@ struct hist_trigger_data {
> > struct tracing_map *map;
> > bool enable_timestamps;
> > bool remove;
> > + struct hist_field *var_refs[TRACING_MAP_VARS_MAX];
> > + unsigned int n_var_refs;
> > };
> >
> > static u64 hist_field_timestamp(struct hist_field *hist_field,
> > @@ -273,10 +279,344 @@ static u64 hist_field_timestamp(struct hist_field *hist_field,
> > return ts;
> > }
> >
> > +struct hist_var_data {
> > + struct list_head list;
> > + struct hist_trigger_data *hist_data;
> > +};
> > +
> > +static struct hist_field *check_var_ref(struct hist_field *hist_field,
> > + struct hist_trigger_data *var_data,
> > + unsigned int var_idx)
> > +{
> > + struct hist_field *found = NULL;
> > +
> > + if (hist_field && hist_field->flags & HIST_FIELD_FL_VAR_REF) {
> > + if (hist_field->var.idx == var_idx &&
> > + hist_field->var.hist_data == var_data) {
> > + found = hist_field;
> > + }
> > + }
> > +
> > + return found;
> > +}
> > +
> > +static struct hist_field *find_var_ref(struct hist_trigger_data *hist_data,
> > + struct hist_trigger_data *var_data,
> > + unsigned int var_idx)
> > +{
> > + struct hist_field *hist_field, *found = NULL;
> > + unsigned int i, j;
> > +
> > + for_each_hist_field(i, hist_data) {
> > + hist_field = hist_data->fields[i];
> > + found = check_var_ref(hist_field, var_data, var_idx);
> > + if (found)
> > + return found;
> > +
> > + for (j = 0; j < HIST_FIELD_OPERANDS_MAX; j++) {
> > + struct hist_field *operand;
> > +
> > + operand = hist_field->operands[j];
> > + found = check_var_ref(operand, var_data, var_idx);
> > + if (found)
> > + return found;
> > + }
>
> Hmm.. IIUC expression can be nested but it seems to miss var-ref at
> level 2 - something like "a + b + $c", no?
>
>
Yes, thanks for pointing these out, will fix, thanks.
Tom
Powered by blists - more mailing lists