tracing/histogram: Fix sorting on old "cpu" value

BugLink: https://bugs.launchpad.net/bugs/1964361
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>
Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 918f969dffcfe22288b9c43ba478310fb6185759..ea168d42c8a2abcfcb85738c9e10f56c39c08190 100644 (file)
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2049,9 +2049,9 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
                        /*
                         * 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,
@@ -4478,7 +4478,7 @@ static int create_tracing_map_fields(struct hist_trigger_data *hist_data)
 
                        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))