Skip to content

Commit

Permalink
tracing: Fix now invalid var_ref_vals assumption in trace action
Browse files Browse the repository at this point in the history
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: 8bcebc7 ("tracing: Fix histogram code when expression has same var as value")
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
  • Loading branch information
Tom Zanussi authored and rostedt committed Jan 31, 2020
1 parent fdeb1ac commit d380dcd
Showing 1 changed file with 38 additions and 15 deletions.
53 changes: 38 additions & 15 deletions kernel/trace/trace_events_hist.c
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,12 @@ struct action_data {
* When a histogram trigger is hit, the values of any
* references to variables, including variables being passed
* as parameters to synthetic events, are collected into a
* var_ref_vals array. This var_ref_idx is the index of the
* first param in the array to be passed to the synthetic
* event invocation.
* var_ref_vals array. This var_ref_idx array is an array of
* indices into the var_ref_vals array, one for each synthetic
* 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;
bool use_trace_keyword;
char *synth_event_name;
Expand Down Expand Up @@ -884,14 +885,14 @@ static struct trace_event_functions synth_event_funcs = {

static notrace void trace_event_raw_event_synth(void *__data,
u64 *var_ref_vals,
unsigned int var_ref_idx)
unsigned int *var_ref_idx)
{
struct trace_event_file *trace_file = __data;
struct synth_trace_event *entry;
struct trace_event_buffer fbuffer;
struct trace_buffer *buffer;
struct synth_event *event;
unsigned int i, n_u64;
unsigned int i, n_u64, val_idx;
int fields_size = 0;

event = trace_file->event_call->data;
Expand All @@ -914,15 +915,16 @@ static notrace void trace_event_raw_event_synth(void *__data,
goto out;

for (i = 0, n_u64 = 0; i < event->n_fields; i++) {
val_idx = var_ref_idx[i];
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];

strscpy(str_field, str_val, STR_VAR_LEN_MAX);
n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
} else {
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) {
case 1:
Expand Down Expand Up @@ -1122,10 +1124,10 @@ static struct tracepoint *alloc_synth_tracepoint(char *name)
}

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,
unsigned int var_ref_idx)
unsigned int *var_ref_idx)
{
struct tracepoint *tp = event->tp;

Expand Down Expand Up @@ -3506,6 +3508,22 @@ static int init_var_ref(struct hist_field *ref_field,
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
* @hist_data: The trigger that will be referencing the variable
Expand Down Expand Up @@ -5071,11 +5089,11 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
struct trace_array *tr = hist_data->event_file->tr;
char *event_name, *param, *system = NULL;
struct hist_field *hist_field, *var_ref;
unsigned int i, var_ref_idx;
unsigned int i;
unsigned int field_pos = 0;
struct synth_event *event;
char *synth_event_name;
int ret = 0;
int var_ref_idx, ret = 0;

lockdep_assert_held(&event_mutex);

Expand All @@ -5092,8 +5110,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data,

event->ref++;

var_ref_idx = hist_data->n_var_refs;

for (i = 0; i < data->n_params; i++) {
char *p;

Expand Down Expand Up @@ -5142,6 +5158,14 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
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++;
kfree(p);
continue;
Expand All @@ -5160,7 +5184,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
}

data->synth_event = event;
data->var_ref_idx = var_ref_idx;
out:
return ret;
err:
Expand Down

0 comments on commit d380dcd

Please sign in to comment.