tracing: Remove restriction on string position in hist trigger keys
authorTom Zanussi <tom.zanussi@linux.intel.com>
Thu, 3 Mar 2016 18:54:54 +0000 (12:54 -0600)
committerSteven Rostedt <rostedt@goodmis.org>
Tue, 19 Apr 2016 22:55:56 +0000 (18:55 -0400)
If we assume the maximum size for a string field, we don't have to
worry about its position.  Since we only allow two keys in a compound
key and having more than one string key in a given compound key
doesn't make much sense anyway, trading a bit of extra space instead
of introducing an arbitrary restriction makes more sense.

We also need to use the event field size for static strings when
copying the contents, otherwise we get random garbage in the key.

Also, cast string return values to avoid warnings on 32-bit compiles.

Finally, rearrange the code without changing any functionality by
moving the compound key updating code into a separate function.

Link: http://lkml.kernel.org/r/8976e1ab04b66bc2700ad1ed0768a2de85ac1983.1457029949.git.tom.zanussi@linux.intel.com
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
kernel/trace/trace_events_hist.c

index 418ff27cbbaf260db29ef8916be02133f2dc17a4..4f4041d769263d8fc587289203d3806a81418097 100644 (file)
@@ -530,8 +530,8 @@ static int create_key_field(struct hist_trigger_data *hist_data,
                        goto out;
                }
 
-               if (is_string_field(field)) /* should be last key field */
-                       key_size = HIST_KEY_SIZE_MAX - key_offset;
+               if (is_string_field(field))
+                       key_size = MAX_FILTER_STR_VAL;
                else
                        key_size = field->size;
        }
@@ -830,9 +830,34 @@ static void hist_trigger_elt_update(struct hist_trigger_data *hist_data,
        }
 }
 
+static inline void add_to_key(char *compound_key, void *key,
+                             struct hist_field *key_field, void *rec)
+{
+       size_t size = key_field->size;
+
+       if (key_field->flags & HIST_FIELD_FL_STRING) {
+               struct ftrace_event_field *field;
+
+               field = key_field->field;
+               if (field->filter_type == FILTER_DYN_STRING)
+                       size = *(u32 *)(rec + field->offset) >> 16;
+               else if (field->filter_type == FILTER_PTR_STRING)
+                       size = strlen(key);
+               else if (field->filter_type == FILTER_STATIC_STRING)
+                       size = field->size;
+
+               /* ensure NULL-termination */
+               if (size > key_field->size - 1)
+                       size = key_field->size - 1;
+       }
+
+       memcpy(compound_key + key_field->offset, key, size);
+}
+
 static void event_hist_trigger(struct event_trigger_data *data, void *rec)
 {
        struct hist_trigger_data *hist_data = data->private_data;
+       bool use_compound_key = (hist_data->n_keys > 1);
        unsigned long entries[HIST_STACKTRACE_DEPTH];
        char compound_key[HIST_KEY_SIZE_MAX];
        struct stack_trace stacktrace;
@@ -842,8 +867,7 @@ static void event_hist_trigger(struct event_trigger_data *data, void *rec)
        void *key = NULL;
        unsigned int i;
 
-       if (hist_data->n_keys > 1)
-               memset(compound_key, 0, hist_data->key_size);
+       memset(compound_key, 0, hist_data->key_size);
 
        for_each_hist_key_field(i, hist_data) {
                key_field = hist_data->fields[i];
@@ -860,35 +884,18 @@ static void event_hist_trigger(struct event_trigger_data *data, void *rec)
                        key = entries;
                } else {
                        field_contents = key_field->fn(key_field, rec);
-                       if (key_field->flags & HIST_FIELD_FL_STRING)
+                       if (key_field->flags & HIST_FIELD_FL_STRING) {
                                key = (void *)(unsigned long)field_contents;
-                       else
+                               use_compound_key = true;
+                       } else
                                key = (void *)&field_contents;
-
-                       if (hist_data->n_keys > 1) {
-                               /* ensure NULL-termination */
-                               size_t size = key_field->size - 1;
-
-                               if (key_field->flags & HIST_FIELD_FL_STRING) {
-                                       struct ftrace_event_field *field;
-
-                                       field = key_field->field;
-                                       if (field->filter_type == FILTER_DYN_STRING)
-                                               size = *(u32 *)(rec + field->offset) >> 16;
-                                       else if (field->filter_type == FILTER_PTR_STRING)
-                                               size = strlen(key);
-
-                                       if (size > key_field->size - 1)
-                                               size = key_field->size - 1;
-                               }
-
-                               memcpy(compound_key + key_field->offset, key,
-                                      size);
-                       }
                }
+
+               if (use_compound_key)
+                       add_to_key(compound_key, key, key_field, rec);
        }
 
-       if (hist_data->n_keys > 1)
+       if (use_compound_key)
                key = compound_key;
 
        elt = tracing_map_insert(hist_data->map, key);