| Message ID | 20230302020810.559462599@goodmis.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e0213434fe3e4a0d118923dc98d31e7ff1cd9e45 |
| Headers | show |
| Series | tracing: Fix adding some modifiers to histogram values | expand |
On Wed, Mar 01, 2023 at 08:00:52PM -0500, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > Histogram values can not be strings, stacktraces, graphs, symbols, > syscalls, or grouped in buckets or log. Give an error if a value is set to > do so. > > Note, the histogram code was not prepared to handle these modifiers for > histograms and caused a bug. > Cc: stable@vger.kernel.org > Fixes: c6afad49d127f ("tracing: Add hist trigger 'sym' and 'sym-offset' modifiers") > Reported-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> Tested-by: Mark Rutland <mark.rutland@arm.com> I gave this a spin, and I see that the buckets modifier gerts rejected for hitcount, but is usable for other values as it should be: | # echo 'p:copy_to_user __arch_copy_to_user n=$arg3' >> /sys/kernel/tracing/kprobe_events | # echo 'hist:keys=n:vals=hitcount.buckets=8:sort=hitcount' > /sys/kernel/tracing/events/kprobes/copy_to_user/trigger | sh: write error: Invalid argument | # echo 'hist:keys=n.buckets=8:vals=hitcount:sort=hitcount' > /sys/kernel/tracing/events/kprobes/copy_to_user/trigger | # cat /sys/kernel/tracing/events/kprobes/copy_to_user/hist | # event histogram | # | # trigger info: hist:keys=n.buckets=8:vals=hitcount:sort=hitcount:size=2048 [active] | # | | { n: ~ 336-343 } hitcount: 1 | { n: ~ 16-23 } hitcount: 2 | { n: ~ 32-39 } hitcount: 2 | { n: ~ 832-839 } hitcount: 3 | { n: ~ 8-15 } hitcount: 3 | { n: ~ 128-135 } hitcount: 5 | { n: ~ 0-7 } hitcount: 57 | | Totals: | Hits: 73 | Entries: 7 | Dropped: 0 Thanks, Mark. > --- > kernel/trace/trace_events_hist.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c > index 89877a18f933..6e8ab726a7b5 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -4235,6 +4235,15 @@ static int __create_val_field(struct hist_trigger_data *hist_data, > goto out; > } > > + /* Some types cannot be a value */ > + if (hist_field->flags & (HIST_FIELD_FL_GRAPH | HIST_FIELD_FL_PERCENT | > + HIST_FIELD_FL_BUCKET | HIST_FIELD_FL_LOG2 | > + HIST_FIELD_FL_SYM | HIST_FIELD_FL_SYM_OFFSET | > + HIST_FIELD_FL_SYSCALL | HIST_FIELD_FL_STACKTRACE)) { > + hist_err(file->tr, HIST_ERR_BAD_FIELD_MODIFIER, errpos(field_str)); > + ret = -EINVAL; > + } > + > hist_data->fields[val_idx] = hist_field; > > ++hist_data->n_vals; > -- > 2.39.1
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 89877a18f933..6e8ab726a7b5 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -4235,6 +4235,15 @@ static int __create_val_field(struct hist_trigger_data *hist_data, goto out; } + /* Some types cannot be a value */ + if (hist_field->flags & (HIST_FIELD_FL_GRAPH | HIST_FIELD_FL_PERCENT | + HIST_FIELD_FL_BUCKET | HIST_FIELD_FL_LOG2 | + HIST_FIELD_FL_SYM | HIST_FIELD_FL_SYM_OFFSET | + HIST_FIELD_FL_SYSCALL | HIST_FIELD_FL_STACKTRACE)) { + hist_err(file->tr, HIST_ERR_BAD_FIELD_MODIFIER, errpos(field_str)); + ret = -EINVAL; + } + hist_data->fields[val_idx] = hist_field; ++hist_data->n_vals;