lkml.org 
[lkml]   [2020]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Unresolved reference for histogram variable
From
Date
Hi Steve,

On Thu, 2020-01-16 at 16:56 -0500, Steven Rostedt wrote:
> On Thu, 16 Jan 2020 15:42:16 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > in parse_expr():
> >
> > operand1->read_once = true;
> > operand2->read_once = true;
> >
> > Why is that?
> >
> > This means that any variable used in an expression can not be use
> > later
> > on.
> >
> > Or should the variable be detected that it is used multiple times
> > in
> > the expression, and have the parser detect this, and just reuse the
> > same variable multiple times?
>
> This patch seems to fix the problem, and lets us reuse the same
> variable multiple times.

It works for me too, and seems like it should be a good fix for the
problem. The size and is_signed might be the overkill you're referring
to, but I'd like to spend time tomorrow verifying that and doing some
testing to make sure.

Thanks,

Tom

>
> -- Steve
>
> diff --git a/kernel/trace/trace_events_hist.c
> b/kernel/trace/trace_events_hist.c
> index 117a1202a6b9..b7f944735a4a 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -116,6 +116,7 @@ struct hist_field {
> struct ftrace_event_field *field;
> unsigned long flags;
> hist_field_fn_t fn;
> + unsigned int ref;
> unsigned int size;
> unsigned int offset;
> unsigned int is_signed;
> @@ -2432,8 +2433,16 @@ static int contains_operator(char *str)
> return field_op;
> }
>
> +static void get_hist_field(struct hist_field *hist_field)
> +{
> + hist_field->ref++;
> +}
> +
> static void __destroy_hist_field(struct hist_field *hist_field)
> {
> + if (--hist_field->ref > 1)
> + return;
> +
> kfree(hist_field->var.name);
> kfree(hist_field->name);
> kfree(hist_field->type);
> @@ -2475,6 +2484,8 @@ static struct hist_field
> *create_hist_field(struct hist_trigger_data *hist_data,
> if (!hist_field)
> return NULL;
>
> + hist_field->ref = 1;
> +
> hist_field->hist_data = hist_data;
>
> if (flags & HIST_FIELD_FL_EXPR || flags &
> HIST_FIELD_FL_ALIAS)
> @@ -2670,6 +2681,19 @@ static struct hist_field
> *create_var_ref(struct hist_trigger_data *hist_data,
> {
> unsigned long flags = HIST_FIELD_FL_VAR_REF;
> struct hist_field *ref_field;
> + int i;
> +
> + for (i = 0; i < hist_data->n_var_refs; i++) {
> + ref_field = hist_data->var_refs[i];
> + /* Maybe this is overkill? */
> + if (ref_field->var.idx == var_field->var.idx &&
> + ref_field->var.hist_data == var_field->hist_data
> &&
> + ref_field->size == var_field->size &&
> + ref_field->is_signed == var_field->is_signed) {
> + get_hist_field(ref_field);
> + return ref_field;
> + }
> + }
>
> ref_field = create_hist_field(var_field->hist_data, NULL,
> flags, NULL);
> if (ref_field) {

\
 
 \ /
  Last update: 2020-01-17 05:27    [W:0.087 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site