Commit d380dcde authored by Tom Zanussi's avatar Tom Zanussi Committed by Steven Rostedt (VMware)

tracing: Fix now invalid var_ref_vals assumption in trace action

The patch 'tracing: Fix histogram code when expression has same var as
value' added code to return an existing variable reference when
creating a new variable reference, which resulted in var_ref_vals
slots being reused instead of being duplicated.

The implementation of the trace action assumes that the end of the
var_ref_vals array starting at action_data.var_ref_idx corresponds to
the values that will be assigned to the trace params. The patch
mentioned above invalidates that assumption, which means that each
param needs to explicitly specify its index into var_ref_vals.

This fix changes action_data.var_ref_idx to an array of var ref
indexes to account for that.

Link: https://lore.kernel.org/r/1580335695.6220.8.camel@kernel.org

Fixes: 8bcebc77 ("tracing: Fix histogram code when expression has same var as value")
Signed-off-by: default avatarTom Zanussi <zanussi@kernel.org>
Signed-off-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
parent fdeb1aca
...@@ -476,11 +476,12 @@ struct action_data { ...@@ -476,11 +476,12 @@ struct action_data {
* When a histogram trigger is hit, the values of any * When a histogram trigger is hit, the values of any
* references to variables, including variables being passed * references to variables, including variables being passed
* as parameters to synthetic events, are collected into a * as parameters to synthetic events, are collected into a
* var_ref_vals array. This var_ref_idx is the index of the * var_ref_vals array. This var_ref_idx array is an array of
* first param in the array to be passed to the synthetic * indices into the var_ref_vals array, one for each synthetic
* event invocation. * event param, and is passed to the synthetic event
* invocation.
*/ */
unsigned int var_ref_idx; unsigned int var_ref_idx[TRACING_MAP_VARS_MAX];
struct synth_event *synth_event; struct synth_event *synth_event;
bool use_trace_keyword; bool use_trace_keyword;
char *synth_event_name; char *synth_event_name;
...@@ -884,14 +885,14 @@ static struct trace_event_functions synth_event_funcs = { ...@@ -884,14 +885,14 @@ static struct trace_event_functions synth_event_funcs = {
static notrace void trace_event_raw_event_synth(void *__data, static notrace void trace_event_raw_event_synth(void *__data,
u64 *var_ref_vals, u64 *var_ref_vals,
unsigned int var_ref_idx) unsigned int *var_ref_idx)
{ {
struct trace_event_file *trace_file = __data; struct trace_event_file *trace_file = __data;
struct synth_trace_event *entry; struct synth_trace_event *entry;
struct trace_event_buffer fbuffer; struct trace_event_buffer fbuffer;
struct trace_buffer *buffer; struct trace_buffer *buffer;
struct synth_event *event; struct synth_event *event;
unsigned int i, n_u64; unsigned int i, n_u64, val_idx;
int fields_size = 0; int fields_size = 0;
event = trace_file->event_call->data; event = trace_file->event_call->data;
...@@ -914,15 +915,16 @@ static notrace void trace_event_raw_event_synth(void *__data, ...@@ -914,15 +915,16 @@ static notrace void trace_event_raw_event_synth(void *__data,
goto out; goto out;
for (i = 0, n_u64 = 0; i < event->n_fields; i++) { for (i = 0, n_u64 = 0; i < event->n_fields; i++) {
val_idx = var_ref_idx[i];
if (event->fields[i]->is_string) { if (event->fields[i]->is_string) {
char *str_val = (char *)(long)var_ref_vals[var_ref_idx + i]; char *str_val = (char *)(long)var_ref_vals[val_idx];
char *str_field = (char *)&entry->fields[n_u64]; char *str_field = (char *)&entry->fields[n_u64];
strscpy(str_field, str_val, STR_VAR_LEN_MAX); strscpy(str_field, str_val, STR_VAR_LEN_MAX);
n_u64 += STR_VAR_LEN_MAX / sizeof(u64); n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
} else { } else {
struct synth_field *field = event->fields[i]; struct synth_field *field = event->fields[i];
u64 val = var_ref_vals[var_ref_idx + i]; u64 val = var_ref_vals[val_idx];
switch (field->size) { switch (field->size) {
case 1: case 1:
...@@ -1122,10 +1124,10 @@ static struct tracepoint *alloc_synth_tracepoint(char *name) ...@@ -1122,10 +1124,10 @@ static struct tracepoint *alloc_synth_tracepoint(char *name)
} }
typedef void (*synth_probe_func_t) (void *__data, u64 *var_ref_vals, typedef void (*synth_probe_func_t) (void *__data, u64 *var_ref_vals,
unsigned int var_ref_idx); unsigned int *var_ref_idx);
static inline void trace_synth(struct synth_event *event, u64 *var_ref_vals, static inline void trace_synth(struct synth_event *event, u64 *var_ref_vals,
unsigned int var_ref_idx) unsigned int *var_ref_idx)
{ {
struct tracepoint *tp = event->tp; struct tracepoint *tp = event->tp;
...@@ -3506,6 +3508,22 @@ static int init_var_ref(struct hist_field *ref_field, ...@@ -3506,6 +3508,22 @@ static int init_var_ref(struct hist_field *ref_field,
goto out; goto out;
} }
static int find_var_ref_idx(struct hist_trigger_data *hist_data,
struct hist_field *var_field)
{
struct hist_field *ref_field;
int i;
for (i = 0; i < hist_data->n_var_refs; i++) {
ref_field = hist_data->var_refs[i];
if (ref_field->var.idx == var_field->var.idx &&
ref_field->var.hist_data == var_field->hist_data)
return i;
}
return -ENOENT;
}
/** /**
* create_var_ref - Create a variable reference and attach it to trigger * create_var_ref - Create a variable reference and attach it to trigger
* @hist_data: The trigger that will be referencing the variable * @hist_data: The trigger that will be referencing the variable
...@@ -5071,11 +5089,11 @@ static int trace_action_create(struct hist_trigger_data *hist_data, ...@@ -5071,11 +5089,11 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
struct trace_array *tr = hist_data->event_file->tr; struct trace_array *tr = hist_data->event_file->tr;
char *event_name, *param, *system = NULL; char *event_name, *param, *system = NULL;
struct hist_field *hist_field, *var_ref; struct hist_field *hist_field, *var_ref;
unsigned int i, var_ref_idx; unsigned int i;
unsigned int field_pos = 0; unsigned int field_pos = 0;
struct synth_event *event; struct synth_event *event;
char *synth_event_name; char *synth_event_name;
int ret = 0; int var_ref_idx, ret = 0;
lockdep_assert_held(&event_mutex); lockdep_assert_held(&event_mutex);
...@@ -5092,8 +5110,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data, ...@@ -5092,8 +5110,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
event->ref++; event->ref++;
var_ref_idx = hist_data->n_var_refs;
for (i = 0; i < data->n_params; i++) { for (i = 0; i < data->n_params; i++) {
char *p; char *p;
...@@ -5142,6 +5158,14 @@ static int trace_action_create(struct hist_trigger_data *hist_data, ...@@ -5142,6 +5158,14 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
goto err; goto err;
} }
var_ref_idx = find_var_ref_idx(hist_data, var_ref);
if (WARN_ON(var_ref_idx < 0)) {
ret = var_ref_idx;
goto err;
}
data->var_ref_idx[i] = var_ref_idx;
field_pos++; field_pos++;
kfree(p); kfree(p);
continue; continue;
...@@ -5160,7 +5184,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data, ...@@ -5160,7 +5184,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
} }
data->synth_event = event; data->synth_event = event;
data->var_ref_idx = var_ref_idx;
out: out:
return ret; return ret;
err: err:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment