| From 1d1898f65616c4601208963c3376c1d828cbf2c7 Mon Sep 17 00:00:00 2001 |
| From: "Steven Rostedt (Google)" <rostedt@goodmis.org> |
| Date: Tue, 1 Mar 2022 22:29:04 -0500 |
| Subject: tracing/histogram: Fix sorting on old "cpu" value |
| |
| From: Steven Rostedt (Google) <rostedt@goodmis.org> |
| |
| commit 1d1898f65616c4601208963c3376c1d828cbf2c7 upstream. |
| |
| When trying to add a histogram against an event with the "cpu" field, it |
| was impossible due to "cpu" being a keyword to key off of the running CPU. |
| So to fix this, it was changed to "common_cpu" to match the other generic |
| fields (like "common_pid"). But since some scripts used "cpu" for keying |
| off of the CPU (for events that did not have "cpu" as a field, which is |
| most of them), a backward compatibility trick was added such that if "cpu" |
| was used as a key, and the event did not have "cpu" as a field name, then |
| it would fallback and switch over to "common_cpu". |
| |
| This fix has a couple of subtle bugs. One was that when switching over to |
| "common_cpu", it did not change the field name, it just set a flag. But |
| the code still found a "cpu" field. The "cpu" field is used for filtering |
| and is returned when the event does not have a "cpu" field. |
| |
| This was found by: |
| |
| # cd /sys/kernel/tracing |
| # echo hist:key=cpu,pid:sort=cpu > events/sched/sched_wakeup/trigger |
| # cat events/sched/sched_wakeup/hist |
| |
| Which showed the histogram unsorted: |
| |
| { cpu: 19, pid: 1175 } hitcount: 1 |
| { cpu: 6, pid: 239 } hitcount: 2 |
| { cpu: 23, pid: 1186 } hitcount: 14 |
| { cpu: 12, pid: 249 } hitcount: 2 |
| { cpu: 3, pid: 994 } hitcount: 5 |
| |
| Instead of hard coding the "cpu" checks, take advantage of the fact that |
| trace_event_field_field() returns a special field for "cpu" and "CPU" if |
| the event does not have "cpu" as a field. This special field has the |
| "filter_type" of "FILTER_CPU". Check that to test if the returned field is |
| of the CPU type instead of doing the string compare. |
| |
| Also, fix the sorting bug by testing for the hist_field flag of |
| HIST_FIELD_FL_CPU when setting up the sort routine. Otherwise it will use |
| the special CPU field to know what compare routine to use, and since that |
| special field does not have a size, it returns tracing_map_cmp_none. |
| |
| Cc: stable@vger.kernel.org |
| Fixes: 1e3bac71c505 ("tracing/histogram: Rename "cpu" to "common_cpu"") |
| Reported-by: Daniel Bristot de Oliveira <bristot@kernel.org> |
| Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| kernel/trace/trace_events_hist.c | 6 +++--- |
| 1 file changed, 3 insertions(+), 3 deletions(-) |
| |
| --- a/kernel/trace/trace_events_hist.c |
| +++ b/kernel/trace/trace_events_hist.c |
| @@ -2891,9 +2891,9 @@ parse_field(struct hist_trigger_data *hi |
| /* |
| * For backward compatibility, if field_name |
| * was "cpu", then we treat this the same as |
| - * common_cpu. |
| + * common_cpu. This also works for "CPU". |
| */ |
| - if (strcmp(field_name, "cpu") == 0) { |
| + if (field && field->filter_type == FILTER_CPU) { |
| *flags |= HIST_FIELD_FL_CPU; |
| } else { |
| hist_err(tr, HIST_ERR_FIELD_NOT_FOUND, |
| @@ -5247,7 +5247,7 @@ static int create_tracing_map_fields(str |
| |
| if (hist_field->flags & HIST_FIELD_FL_STACKTRACE) |
| cmp_fn = tracing_map_cmp_none; |
| - else if (!field) |
| + else if (!field || hist_field->flags & HIST_FIELD_FL_CPU) |
| cmp_fn = tracing_map_cmp_num(hist_field->size, |
| hist_field->is_signed); |
| else if (is_string_field(field)) |