All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Tom Zanussi <tom.zanussi@linux.intel.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Namhyung Kim <namhyung@kernel.org>
Subject: [for-next][PATCH 20/30] tracing: Remove restriction on string position in hist trigger keys
Date: Wed, 20 Apr 2016 15:29:58 -0400	[thread overview]
Message-ID: <20160420193029.477719825@goodmis.org> (raw)
In-Reply-To: 20160420192938.515326341@goodmis.org

[-- Attachment #1: 0020-tracing-Remove-restriction-on-string-position-in-his.patch --]
[-- Type: text/plain, Size: 4377 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

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 | 63 ++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 418ff27cbbaf..4f4041d76926 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -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);
-- 
2.8.0.rc3

  parent reply	other threads:[~2016-04-20 19:30 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-20 19:29 [for-next][PATCH 00/30] tracing: Add features for event-fork and histograms Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 01/30] tracing: Rename check_ignore_pid() to ignore_this_task() Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 02/30] tracing: Use pid bitmap instead of a pid array for set_event_pid Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 03/30] tracing: Add infrastructure to allow set_event_pid to follow children Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 04/30] tracing: Update the documentation to describe "event-fork" option Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 05/30] tracing: Add lock-free tracing_map Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 06/30] tracing: Fix TRACING_MAP Kconfig Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 07/30] tracing: Update some tracing_map constants and comments Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 08/30] tracing: Add hist event trigger command Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 09/30] tracing: Add hist trigger support for multiple values (vals= param) Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 10/30] tracing: Add hist trigger support for compound keys Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 11/30] tracing: Add hist trigger support for user-defined sorting (sort= param) Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 12/30] tracing: Add hist trigger support for pausing and continuing a trace Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 13/30] tracing: Add hist trigger support for clearing " Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 14/30] tracing: Add hist trigger hex modifier for displaying numeric fields Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 15/30] tracing: Add hist trigger sym and sym-offset modifiers Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 16/30] tracing: Add hist trigger execname modifier Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 17/30] tracing: Add hist trigger syscall modifier Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 18/30] tracing: Add hist trigger support for stacktraces as keys Steven Rostedt
2016-04-20 19:29 ` [for-next][PATCH 19/30] tracing: Support string type key properly Steven Rostedt
2016-04-20 19:29 ` Steven Rostedt [this message]
2016-04-20 19:29 ` [for-next][PATCH 21/30] tracing: Add enable_hist/disable_hist triggers Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 22/30] tracing: Add hist trigger Documentation Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 23/30] tracing: Add support for multiple hist triggers per event Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 24/30] tracing: Add support for named triggers Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 25/30] tracing: Add support for named hist triggers Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 26/30] kselftests/ftrace : Add event trigger testcases Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 27/30] kselftests/ftrace: Add hist " Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 28/30] tracing: Add hist trigger log2 modifier Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 29/30] kselftests/ftrace: Add a test for log2 modifier of hist trigger Steven Rostedt
2016-04-20 19:30 ` [for-next][PATCH 30/30] tracing: Fix unsigned comparison to zero in hist trigger code Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160420193029.477719825@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=tom.zanussi@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.