From: Jiri Olsa <jolsa@redhat.com>
Subject: perf tools: Fix modifier to be applied on correct events
Date: Tue Mar 20 19:15:40 2012 +0100
Git-commit: 5d7be90ed5cfb5dd3c9ab726d7daa91b86b81747
Patch-mainline: v3.4-rc1
References: bnc#806964
Signed-off-by: Tony Jones <tonyj@suse.de>

    perf tools: Fix modifier to be applied on correct events
    
    The event modifier needs to be applied only on the event definition it
    is attached to.
    
    The current state is that in case of multiple events definition (in
    single '-e' option, separated by ',') all will get modifier of the last
    one.
    
    Fixing this by adding separated list for each event definition, so the
    modifier is applied only to proper event(s). Added automated test to
    catch this, plus some other modifier tests.
    
    Signed-off-by: Jiri Olsa <jolsa@redhat.com>
    Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
    Cc: Frederic Weisbecker <fweisbec@gmail.com>
    Cc: Ingo Molnar <mingo@elte.hu>
    Cc: Paul Mackerras <paulus@samba.org>
    Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Link: http://lkml.kernel.org/r/1332267341-26338-3-git-send-email-jolsa@redhat.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

---
 tools/perf/builtin-test.c      |  116 ++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/parse-events.c |   27 +++++++--
 tools/perf/util/parse-events.h |    7 +-
 tools/perf/util/parse-events.y |   40 +++++++++-----
 4 files changed, 166 insertions(+), 24 deletions(-)

--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -878,6 +878,58 @@ static int test__checkevent_genhw_modifi
 	return test__checkevent_genhw(evlist);
 }
 
+static int test__checkevent_breakpoint_modifier(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel = list_entry(evlist->entries.next,
+					      struct perf_evsel, node);
+
+	TEST_ASSERT_VAL("wrong exclude_user", !evsel->attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
+	TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
+
+	return test__checkevent_breakpoint(evlist);
+}
+
+static int test__checkevent_breakpoint_x_modifier(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel = list_entry(evlist->entries.next,
+					      struct perf_evsel, node);
+
+	TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
+	TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
+
+	return test__checkevent_breakpoint_x(evlist);
+}
+
+static int test__checkevent_breakpoint_r_modifier(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel = list_entry(evlist->entries.next,
+					      struct perf_evsel, node);
+
+	TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->attr.exclude_hv);
+	TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+
+	return test__checkevent_breakpoint_r(evlist);
+}
+
+static int test__checkevent_breakpoint_w_modifier(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel = list_entry(evlist->entries.next,
+					      struct perf_evsel, node);
+
+	TEST_ASSERT_VAL("wrong exclude_user", !evsel->attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
+	TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+
+	return test__checkevent_breakpoint_w(evlist);
+}
+
 static int test__checkevent_pmu(struct perf_evlist *evlist)
 {
 
@@ -894,6 +946,47 @@ static int test__checkevent_pmu(struct p
 	return 0;
 }
 
+static int test__checkevent_list(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+
+	TEST_ASSERT_VAL("wrong number of entries", 3 == evlist->nr_entries);
+
+	/* r1 */
+	evsel = list_entry(evlist->entries.next, struct perf_evsel, node);
+	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->attr.type);
+	TEST_ASSERT_VAL("wrong config", 1 == evsel->attr.config);
+	TEST_ASSERT_VAL("wrong config1", 0 == evsel->attr.config1);
+	TEST_ASSERT_VAL("wrong config2", 0 == evsel->attr.config2);
+	TEST_ASSERT_VAL("wrong exclude_user", !evsel->attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->attr.exclude_hv);
+	TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
+
+	/* syscalls:sys_enter_open:k */
+	evsel = list_entry(evsel->node.next, struct perf_evsel, node);
+	TEST_ASSERT_VAL("wrong type", PERF_TYPE_TRACEPOINT == evsel->attr.type);
+	TEST_ASSERT_VAL("wrong sample_type",
+		(PERF_SAMPLE_RAW | PERF_SAMPLE_TIME | PERF_SAMPLE_CPU) ==
+		evsel->attr.sample_type);
+	TEST_ASSERT_VAL("wrong sample_period", 1 == evsel->attr.sample_period);
+	TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
+	TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
+
+	/* 1:1:hp */
+	evsel = list_entry(evsel->node.next, struct perf_evsel, node);
+	TEST_ASSERT_VAL("wrong type", 1 == evsel->attr.type);
+	TEST_ASSERT_VAL("wrong config", 1 == evsel->attr.config);
+	TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->attr.exclude_hv);
+	TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+
+	return 0;
+}
+
 static struct test__event_st {
 	const char *name;
 	__u32 type;
@@ -976,9 +1069,29 @@ static struct test__event_st {
 		.check = test__checkevent_genhw_modifier,
 	},
 	{
+		.name  = "mem:0:u",
+		.check = test__checkevent_breakpoint_modifier,
+	},
+	{
+		.name  = "mem:0:x:k",
+		.check = test__checkevent_breakpoint_x_modifier,
+	},
+	{
+		.name  = "mem:0:r:hp",
+		.check = test__checkevent_breakpoint_r_modifier,
+	},
+	{
+		.name  = "mem:0:w:up",
+		.check = test__checkevent_breakpoint_w_modifier,
+	},
+	{
 		.name  = "cpu/config=10,config1,config2=3,period=1000/u",
 		.check = test__checkevent_pmu,
 	},
+	{
+		.name  = "r1,syscalls:sys_enter_open:k,1:1:hp",
+		.check = test__checkevent_list,
+	},
 };
 
 #define TEST__EVENTS_CNT (sizeof(test__events) / sizeof(struct test__event_st))
@@ -1004,10 +1117,9 @@ static int test__parse_events(void)
 		}
 
 		ret = e->check(evlist);
+		perf_evlist__delete(evlist);
 		if (ret)
 			break;
-
-		perf_evlist__delete(evlist);
 	}
 
 	return ret;
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -23,7 +23,8 @@ struct event_symbol {
 	const char	*alias;
 };
 
-int parse_events_parse(struct list_head *list, int *idx);
+int parse_events_parse(struct list_head *list, struct list_head *list_tmp,
+		       int *idx);
 
 #define CHW(x) .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_##x
 #define CSW(x) .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_##x
@@ -671,6 +672,18 @@ int parse_events_add_pmu(struct list_hea
 	return add_event(list, idx, &attr, (char *) "pmu");
 }
 
+void parse_events_update_lists(struct list_head *list_event,
+			       struct list_head *list_all)
+{
+	/*
+	 * Called for single event definition. Update the
+	 * 'all event' list, and reinit the 'signle event'
+	 * list, for next event definition.
+	 */
+	list_splice_tail(list_event, list_all);
+	INIT_LIST_HEAD(list_event);
+}
+
 int parse_events_modifier(struct list_head *list, char *str)
 {
 	struct perf_evsel *evsel;
@@ -726,14 +739,14 @@ int parse_events_modifier(struct list_he
 
 int parse_events(struct perf_evlist *evlist, const char *str, int unset __used)
 {
-	struct perf_evsel *evsel, *h;
 	LIST_HEAD(list);
+	LIST_HEAD(list_tmp);
 	YY_BUFFER_STATE buffer;
 	int ret, idx = evlist->nr_entries;
 
 	buffer = parse_events__scan_string(str);
 
-	ret = parse_events_parse(&list, &idx);
+	ret = parse_events_parse(&list, &list_tmp, &idx);
 
 	parse_events__flush_buffer(buffer);
 	parse_events__delete_buffer(buffer);
@@ -744,9 +757,11 @@ int parse_events(struct perf_evlist *evl
 		return 0;
 	}
 
-	list_for_each_entry_safe(evsel, h, &list, node)
-		perf_evsel__delete(evsel);
-
+	/*
+	 * There are 2 users - builtin-record and builtin-test objects.
+	 * Both call perf_evlist__delete in case of error, so we dont
+	 * need to bother.
+	 */
 	fprintf(stderr, "invalid or unsupported event: '%s'\n", str);
 	fprintf(stderr, "Run 'perf list' for a list of valid events\n");
 	return ret;
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -76,8 +76,11 @@ int parse_events_add_breakpoint(struct l
 				void *ptr, char *type);
 int parse_events_add_pmu(struct list_head *list, int *idx,
 			 char *pmu , struct list_head *head_config);
-void parse_events_error(struct list_head *list, int *idx,
-			char const *msg);
+void parse_events_update_lists(struct list_head *list_event,
+			       struct list_head *list_all);
+void parse_events_error(struct list_head *list_all,
+			struct list_head *list_event,
+			int *idx, char const *msg);
 
 void print_events(const char *event_glob);
 void print_events_type(u8 type);
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -1,6 +1,7 @@
 
 %name-prefix "parse_events_"
-%parse-param {struct list_head *list}
+%parse-param {struct list_head *list_all}
+%parse-param {struct list_head *list_event}
 %parse-param {int *idx}
 
 %{
@@ -56,10 +57,19 @@ events ',' event | event
 event:
 event_def PE_MODIFIER_EVENT
 {
-	ABORT_ON(parse_events_modifier(list, $2));
+	/*
+	 * Apply modifier on all events added by single event definition
+	 * (there could be more events added for multiple tracepoint
+	 * definitions via '*?'.
+	 */
+	ABORT_ON(parse_events_modifier(list_event, $2));
+	parse_events_update_lists(list_event, list_all);
 }
 |
 event_def
+{
+	parse_events_update_lists(list_event, list_all);
+}
 
 event_def: event_pmu |
 	   event_legacy_symbol |
@@ -72,7 +82,7 @@ event_def: event_pmu |
 event_pmu:
 PE_NAME '/' event_config '/'
 {
-	ABORT_ON(parse_events_add_pmu(list, idx, $1, $3));
+	ABORT_ON(parse_events_add_pmu(list_event, idx, $1, $3));
 	parse_events__free_terms($3);
 }
 
@@ -82,7 +92,7 @@ PE_VALUE_SYM '/' event_config '/'
 	int type = $1 >> 16;
 	int config = $1 & 255;
 
-	ABORT_ON(parse_events_add_numeric(list, idx, type, config, $3));
+	ABORT_ON(parse_events_add_numeric(list_event, idx, type, config, $3));
 	parse_events__free_terms($3);
 }
 |
@@ -91,52 +101,52 @@ PE_VALUE_SYM sep_slash_dc
 	int type = $1 >> 16;
 	int config = $1 & 255;
 
-	ABORT_ON(parse_events_add_numeric(list, idx, type, config, NULL));
+	ABORT_ON(parse_events_add_numeric(list_event, idx, type, config, NULL));
 }
 
 event_legacy_cache:
 PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT
 {
-	ABORT_ON(parse_events_add_cache(list, idx, $1, $3, $5));
+	ABORT_ON(parse_events_add_cache(list_event, idx, $1, $3, $5));
 }
 |
 PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT
 {
-	ABORT_ON(parse_events_add_cache(list, idx, $1, $3, NULL));
+	ABORT_ON(parse_events_add_cache(list_event, idx, $1, $3, NULL));
 }
 |
 PE_NAME_CACHE_TYPE
 {
-	ABORT_ON(parse_events_add_cache(list, idx, $1, NULL, NULL));
+	ABORT_ON(parse_events_add_cache(list_event, idx, $1, NULL, NULL));
 }
 
 event_legacy_mem:
 PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
 {
-	ABORT_ON(parse_events_add_breakpoint(list, idx, (void *) $2, $4));
+	ABORT_ON(parse_events_add_breakpoint(list_event, idx, (void *) $2, $4));
 }
 |
 PE_PREFIX_MEM PE_VALUE sep_dc
 {
-	ABORT_ON(parse_events_add_breakpoint(list, idx, (void *) $2, NULL));
+	ABORT_ON(parse_events_add_breakpoint(list_event, idx, (void *) $2, NULL));
 }
 
 event_legacy_tracepoint:
 PE_NAME ':' PE_NAME
 {
-	ABORT_ON(parse_events_add_tracepoint(list, idx, $1, $3));
+	ABORT_ON(parse_events_add_tracepoint(list_event, idx, $1, $3));
 }
 
 event_legacy_numeric:
 PE_VALUE ':' PE_VALUE
 {
-	ABORT_ON(parse_events_add_numeric(list, idx, $1, $3, NULL));
+	ABORT_ON(parse_events_add_numeric(list_event, idx, $1, $3, NULL));
 }
 
 event_legacy_raw:
 PE_RAW
 {
-	ABORT_ON(parse_events_add_numeric(list, idx, PERF_TYPE_RAW, $1, NULL));
+	ABORT_ON(parse_events_add_numeric(list_event, idx, PERF_TYPE_RAW, $1, NULL));
 }
 
 event_config:
@@ -211,7 +221,9 @@ sep_slash_dc: '/' | ':' |
 
 %%
 
-void parse_events_error(struct list_head *list __used, int *idx __used,
+void parse_events_error(struct list_head *list_all __used,
+			struct list_head *list_event __used,
+			int *idx __used,
 			char const *msg __used)
 {
 }
