From: Namhyung Kim <namhyung@kernel.org>
To: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: rostedt@goodmis.org, tglx@linutronix.de, mhiramat@kernel.org,
vedang.patel@intel.com, bigeasy@linutronix.de,
joel.opensrc@gmail.com, joelaf@google.com,
mathieu.desnoyers@efficios.com, baohong.liu@intel.com,
rajvi.jingar@intel.com, linux-kernel@vger.kernel.org,
linux-rt-users@vger.kernel.org, kernel-team@lge.com
Subject: Re: [PATCH v3 19/33] tracing: Add variable reference handling to hist triggers
Date: Mon, 9 Oct 2017 21:46:35 +0900 [thread overview]
Message-ID: <20171009124635.GA7378@danjae.aot.lge.com> (raw)
In-Reply-To: <d92ae1de648410d7488f8787d9a2ec876d40aba6.1506105131.git.tom.zanussi@linux.intel.com>
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@linux.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?
> + }
> +
> + return found;
> +}
> +
> +static struct hist_field *find_any_var_ref(struct hist_trigger_data *hist_data,
> + unsigned int var_idx)
> +{
> + struct trace_array *tr = hist_data->event_file->tr;
> + struct hist_field *found = NULL;
> + struct hist_var_data *var_data;
> +
> + list_for_each_entry(var_data, &tr->hist_vars, list) {
> + found = find_var_ref(var_data->hist_data, hist_data, var_idx);
> + if (found)
> + break;
Shouldn't it check self-references - i.e. ref from the same hist_data?
> + }
> +
> + return found;
> +}
> +
> +static bool check_var_refs(struct hist_trigger_data *hist_data)
> +{
> + struct hist_field *field;
> + bool found = false;
> + int i;
> +
> + for_each_hist_field(i, hist_data) {
> + field = hist_data->fields[i];
> + if (field && field->flags & HIST_FIELD_FL_VAR) {
> + if (find_any_var_ref(hist_data, field->var.idx)) {
> + found = true;
> + break;
> + }
> + }
> + }
> +
> + return found;
> +}
> +
> +static struct hist_var_data *find_hist_vars(struct hist_trigger_data *hist_data)
> +{
> + struct trace_array *tr = hist_data->event_file->tr;
> + struct hist_var_data *var_data, *found = NULL;
> +
> + list_for_each_entry(var_data, &tr->hist_vars, list) {
> + if (var_data->hist_data == hist_data) {
> + found = var_data;
> + break;
> + }
> + }
> +
> + return found;
> +}
> +
> +static bool has_hist_vars(struct hist_trigger_data *hist_data)
> +{
> + struct hist_field *hist_field;
> + int i, j;
> +
> + for_each_hist_field(i, hist_data) {
> + hist_field = hist_data->fields[i];
> + if (hist_field &&
> + (hist_field->flags & HIST_FIELD_FL_VAR ||
> + hist_field->flags & HIST_FIELD_FL_VAR_REF))
> + return true;
> +
> + for (j = 0; j < HIST_FIELD_OPERANDS_MAX; j++) {
> + struct hist_field *operand;
> +
> + operand = hist_field->operands[j];
> + if (operand &&
> + (operand->flags & HIST_FIELD_FL_VAR ||
> + operand->flags & HIST_FIELD_FL_VAR_REF))
> + return true;
Same with the above.
Thanks,
Namhyung
> + }
> + }
> +
> + return false;
> +}
> +
next prev parent reply other threads:[~2017-10-09 12:49 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-22 19:59 [PATCH v3 00/33] tracing: Inter-event (e.g. latency) support Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 01/33] tracing: Add support to detect and avoid duplicates Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 02/33] tracing: Remove code which merges duplicates Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 03/33] ring-buffer: Add interface for setting absolute time stamps Tom Zanussi
2017-10-04 17:14 ` Steven Rostedt
2017-10-05 11:34 ` Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 04/33] ring-buffer: Redefine the unimplemented RINGBUF_TIME_TIME_STAMP Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 05/33] tracing: Give event triggers access to ring_buffer_event Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 06/33] tracing: Add ring buffer event param to hist field functions Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 07/33] tracing: Break out hist trigger assignment parsing Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 08/33] tracing: Add hist trigger timestamp support Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 09/33] tracing: Add per-element variable support to tracing_map Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 10/33] tracing: Add hist_data member to hist_field Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 11/33] tracing: Add usecs modifier for hist trigger timestamps Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 12/33] tracing: Add variable support to hist triggers Tom Zanussi
2017-10-09 3:27 ` Namhyung Kim
2017-10-11 13:33 ` Tom Zanussi
2017-10-11 23:23 ` Namhyung Kim
2017-10-12 15:51 ` Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 13/33] tracing: Account for variables in named trigger compatibility Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 14/33] tracing: Move get_hist_field_flags() Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 15/33] tracing: Add simple expression support to hist triggers Tom Zanussi
2017-10-09 12:54 ` Namhyung Kim
2017-10-11 14:08 ` Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 16/33] tracing: Generalize per-element hist trigger data Tom Zanussi
2017-10-04 17:34 ` Steven Rostedt
2017-10-04 19:22 ` Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 17/33] tracing: Pass tracing_map_elt to hist_field accessor functions Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 18/33] tracing: Add hist_field 'type' field Tom Zanussi
2017-09-22 19:59 ` [PATCH v3 19/33] tracing: Add variable reference handling to hist triggers Tom Zanussi
2017-10-09 12:46 ` Namhyung Kim [this message]
2017-10-11 14:07 ` Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 20/33] tracing: Add hist trigger action hook Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 21/33] tracing: Add support for 'synthetic' events Tom Zanussi
2017-10-04 18:08 ` Steven Rostedt
2017-10-04 19:50 ` Tom Zanussi
2017-10-12 0:53 ` Namhyung Kim
2017-09-22 20:00 ` [PATCH v3 22/33] tracing: Add support for 'field variables' Tom Zanussi
2017-10-12 2:52 ` Namhyung Kim
2017-10-12 21:35 ` Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 23/33] tracing: Add 'onmatch' hist trigger action support Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 24/33] tracing: Add 'onmax' " Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 25/33] tracing: Allow whitespace to surround hist trigger filter Tom Zanussi
2017-10-04 18:11 ` Steven Rostedt
2017-10-04 19:05 ` Tom Zanussi
2017-10-04 19:28 ` Steven Rostedt
2017-10-18 3:00 ` Namhyung Kim
2017-10-18 14:11 ` Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 26/33] tracing: Add cpu field for hist triggers Tom Zanussi
2017-10-04 18:12 ` Steven Rostedt
2017-10-04 19:21 ` Tom Zanussi
2017-10-04 19:30 ` Steven Rostedt
2017-09-22 20:00 ` [PATCH v3 27/33] tracing: Add hist trigger support for variable reference aliases Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 28/33] tracing: Add 'last error' error facility for hist triggers Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 29/33] tracing: Add inter-event hist trigger Documentation Tom Zanussi
2017-10-18 1:36 ` Steven Rostedt
2017-10-18 14:07 ` Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 30/33] tracing: Make tracing_set_clock() non-static Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 31/33] tracing: Add a clock attribute for hist triggers Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 32/33] tracing: Increase trace_recursive_lock() limit for synthetic events Tom Zanussi
2017-09-22 20:00 ` [PATCH v3 33/33] tracing: Add inter-event blurb to HIST_TRIGGERS config option Tom Zanussi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171009124635.GA7378@danjae.aot.lge.com \
--to=namhyung@kernel.org \
--cc=baohong.liu@intel.com \
--cc=bigeasy@linutronix.de \
--cc=joel.opensrc@gmail.com \
--cc=joelaf@google.com \
--cc=kernel-team@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=rajvi.jingar@intel.com \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=tom.zanussi@linux.intel.com \
--cc=vedang.patel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome