Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1288833

[PATCH v13 21/29] tracing: Remove restriction on string position in hist trigger keys

From Tom Zanussi <tom.zanussi@linux.intel.com>
Newsgroups linux.kernel
Subject [PATCH v13 21/29] tracing: Remove restriction on string position in hist trigger keys
Date 2015-12-10 20:00 +0100
Message-ID <qEeBB-1xO-51@gated-at.bofh.it> (permalink)
References <qEeBz-1xO-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

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.

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>
---
 kernel/trace/trace_events_hist.c | 67 ++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 36309c1..ad6666c 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -58,14 +58,14 @@ static u64 hist_field_dynstring(struct hist_field *hist_field, void *event)
 	int str_loc = str_item & 0xffff;
 	char *addr = (char *)(event + str_loc);
 
-	return (u64)addr;
+	return (u64)(unsigned long)addr;
 }
 
 static u64 hist_field_pstring(struct hist_field *hist_field, void *event)
 {
 	char **addr = (char **)(event + hist_field->field->offset);
 
-	return (u64)*addr;
+	return (u64)(unsigned long)*addr;
 }
 
 #define DEFINE_HIST_FIELD_FN(type)					\
@@ -518,8 +518,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;
 	}
@@ -814,9 +814,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_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;
@@ -826,8 +851,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];
@@ -844,35 +868,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_STRING)
+			if (key_field->flags & HIST_FIELD_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_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);
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 00/29] tracing: 'hist' triggers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 21/29] tracing: Remove restriction on string position in hist trigger keys Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 27/29] kselftests/ftrace : Add event trigger testcases Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 24/29] tracing: Add support for multiple hist triggers per event Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 06/29] tracing: Add needs_rec flag to event triggers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 01/29] tracing: Make ftrace_event_field checking functions available Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 04/29] tracing: Add get_syscall_name() Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 17/29] tracing: Add hist trigger 'execname' modifier Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 12/29] tracing: Add hist trigger support for user-defined sorting ('sort=' param) Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 10/29] tracing: Add hist trigger support for multiple values ('vals=' param) Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 25/29] tracing: Add support for named triggers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:00 +0100
  [PATCH v13 15/29] tracing: Add hist trigger 'hex' modifier for displaying numeric fields Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:10 +0100
  [PATCH v13 03/29] tracing: Add event record param to trigger_ops.func() Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:10 +0100
  [PATCH v13 18/29] tracing: Add hist trigger 'syscall' modifier Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:10 +0100
  [PATCH v13 05/29] tracing: Add a per-event-trigger 'paused' field Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:10 +0100
  [PATCH v13 16/29] tracing: Add hist trigger 'sym' and 'sym-offset' modifiers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-12-10 20:10 +0100

csiph-web