tracing/histogram: Rename "cpu" to "common_cpu"
authorSteven Rostedt (VMware) <rostedt@goodmis.org>
Wed, 21 Jul 2021 15:00:53 +0000 (11:00 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 28 Jul 2021 12:37:37 +0000 (14:37 +0200)
commit 1e3bac71c5053c99d438771fc9fa5082ae5d90aa upstream.

Currently the histogram logic allows the user to write "cpu" in as an
event field, and it will record the CPU that the event happened on.

The problem with this is that there's a lot of events that have "cpu"
as a real field, and using "cpu" as the CPU it ran on, makes it
impossible to run histograms on the "cpu" field of events.

For example, if I want to have a histogram on the count of the
workqueue_queue_work event on its cpu field, running:

 ># echo 'hist:keys=cpu' > events/workqueue/workqueue_queue_work/trigger

Gives a misleading and wrong result.

Change the command to "common_cpu" as no event should have "common_*"
fields as that's a reserved name for fields used by all events. And
this makes sense here as common_cpu would be a field used by all events.

Now we can even do:

 ># echo 'hist:keys=common_cpu,cpu if cpu < 100' > events/workqueue/workqueue_queue_work/trigger
 ># cat events/workqueue/workqueue_queue_work/hist
 # event histogram
 #
 # trigger info: hist:keys=common_cpu,cpu:vals=hitcount:sort=hitcount:size=2048 if cpu < 100 [active]
 #

 { common_cpu:          0, cpu:          2 } hitcount:          1
 { common_cpu:          0, cpu:          4 } hitcount:          1
 { common_cpu:          7, cpu:          7 } hitcount:          1
 { common_cpu:          0, cpu:          7 } hitcount:          1
 { common_cpu:          0, cpu:          1 } hitcount:          1
 { common_cpu:          0, cpu:          6 } hitcount:          2
 { common_cpu:          0, cpu:          5 } hitcount:          2
 { common_cpu:          1, cpu:          1 } hitcount:          4
 { common_cpu:          6, cpu:          6 } hitcount:          4
 { common_cpu:          5, cpu:          5 } hitcount:         14
 { common_cpu:          4, cpu:          4 } hitcount:         26
 { common_cpu:          0, cpu:          0 } hitcount:         39
 { common_cpu:          2, cpu:          2 } hitcount:        184

Now for backward compatibility, I added a trick. If "cpu" is used, and
the field is not found, it will fall back to "common_cpu" and work as
it did before. This way, it will still work for old programs that use
"cpu" to get the actual CPU, but if the event has a "cpu" as a field, it
will get that event's "cpu" field, which is probably what it wants
anyway.

I updated the tracefs/README to include documentation about both the
common_timestamp and the common_cpu. This way, if that text is present in
the README, then an application can know that common_cpu is supported over
just plain "cpu".

Link: https://lkml.kernel.org/r/20210721110053.26b4f641@oasis.local.home
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: stable@vger.kernel.org
Fixes: 8b7622bf94a44 ("tracing: Add cpu field for hist triggers")
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Documentation/trace/histogram.rst
kernel/trace/trace.c
kernel/trace/trace_events_hist.c

index b71e09f745c3dae4511c36beb8ccf290f5f01c3e..f99be8062bc82861b1043bfddc31f91588838e02 100644 (file)
@@ -191,7 +191,7 @@ Documentation written by Tom Zanussi
                                 with the event, in nanoseconds.  May be
                                modified by .usecs to have timestamps
                                interpreted as microseconds.
-    cpu                    int  the cpu on which the event occurred.
+    common_cpu             int  the cpu on which the event occurred.
     ====================== ==== =======================================
 
 Extended error information
index 1afd3b57ae517a3212c06de7c3f3cb534177204e..20196380fc545000859693c5f80f2aa0da39e0f9 100644 (file)
@@ -5565,6 +5565,10 @@ static const char readme_msg[] =
        "\t            [:name=histname1]\n"
        "\t            [:<handler>.<action>]\n"
        "\t            [if <filter>]\n\n"
+       "\t    Note, special fields can be used as well:\n"
+       "\t            common_timestamp - to record current timestamp\n"
+       "\t            common_cpu - to record the CPU the event happened on\n"
+       "\n"
        "\t    When a matching event is hit, an entry is added to a hash\n"
        "\t    table using the key(s) and value(s) named, and the value of a\n"
        "\t    sum called 'hitcount' is incremented.  Keys and values\n"
index 53b4ab9ea305c63e21ae66b33eb9914eeafbe46e..023777a4a04d7584eed378177cc83bcd47963bd9 100644 (file)
@@ -1111,7 +1111,7 @@ static const char *hist_field_name(struct hist_field *field,
                 field->flags & HIST_FIELD_FL_ALIAS)
                field_name = hist_field_name(field->operands[0], ++level);
        else if (field->flags & HIST_FIELD_FL_CPU)
-               field_name = "cpu";
+               field_name = "common_cpu";
        else if (field->flags & HIST_FIELD_FL_EXPR ||
                 field->flags & HIST_FIELD_FL_VAR_REF) {
                if (field->system) {
@@ -1991,14 +1991,24 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
                hist_data->enable_timestamps = true;
                if (*flags & HIST_FIELD_FL_TIMESTAMP_USECS)
                        hist_data->attrs->ts_in_usecs = true;
-       } else if (strcmp(field_name, "cpu") == 0)
+       } else if (strcmp(field_name, "common_cpu") == 0)
                *flags |= HIST_FIELD_FL_CPU;
        else {
                field = trace_find_event_field(file->event_call, field_name);
                if (!field || !field->size) {
-                       hist_err(tr, HIST_ERR_FIELD_NOT_FOUND, errpos(field_name));
-                       field = ERR_PTR(-EINVAL);
-                       goto out;
+                       /*
+                        * For backward compatibility, if field_name
+                        * was "cpu", then we treat this the same as
+                        * common_cpu.
+                        */
+                       if (strcmp(field_name, "cpu") == 0) {
+                               *flags |= HIST_FIELD_FL_CPU;
+                       } else {
+                               hist_err(tr, HIST_ERR_FIELD_NOT_FOUND,
+                                        errpos(field_name));
+                               field = ERR_PTR(-EINVAL);
+                               goto out;
+                       }
                }
        }
  out:
@@ -5085,7 +5095,7 @@ static void hist_field_print(struct seq_file *m, struct hist_field *hist_field)
                seq_printf(m, "%s=", hist_field->var.name);
 
        if (hist_field->flags & HIST_FIELD_FL_CPU)
-               seq_puts(m, "cpu");
+               seq_puts(m, "common_cpu");
        else if (field_name) {
                if (hist_field->flags & HIST_FIELD_FL_VAR_REF ||
                    hist_field->flags & HIST_FIELD_FL_ALIAS)