linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] perf tools: Report event parsing errors
@ 2015-04-22 19:10 Jiri Olsa
  2015-04-22 19:10 ` [PATCH 1/9] perf tools: Add parse_events_error interface Jiri Olsa
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

hi,
adding support to report error from event string parsing.

v1 changes (from RFC):
  - display list of allowed terms for pmu event error [Ingo]
  - changing 'invalid or unsupported event' string into
    'event syntax error' for cases we know the precise error

This patchset contains support for standard parsing errors
and more logic to recognize tracepoint and 'pmu//' terms,
like:

  $ sudo perf record -e 'sched:krava' ls
  event syntax error: 'sched:krava'
                       \___ unknown tracepoint
  ...

  $ perf record -e 'cpu/even=0x1/' ls
  event syntax error: 'cpu/even=0x1/'
                           \___ unknown term

  valid terms: pc,any,inv,edge,cmask,event,in_tx,ldlat,umask,in_tx_cp,offcore_rsp,config,config1,config2..
  ...

  $ perf record -e cycles,cache-mises ls
  event syntax error: '..es,cache-mises'
                                 \___ parser error
  ...

Changes are also reachable in here:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/event_parse_error

thanks
jirka


---
Jiri Olsa (9):
      perf tools: Add parse_events_error interface
      perf tools: Add flex support for parse_events_error
      perf tools: Always bail out when config_attr function fails
      perf tools: Change parse_events_add_pmu interface
      perf tools: Add location to pmu event terms
      perf tools: Add term support for parse_events_error
      perf tools: Add static terms support for parse_events_error
      perf tools: Add tracepoint support for parse_events_error
      perf tools: Add symbolic events support for parse_events_error

 tools/perf/builtin-stat.c               |   2 +-
 tools/perf/tests/code-reading.c         |   2 +-
 tools/perf/tests/evsel-roundtrip-name.c |   4 +-
 tools/perf/tests/hists_cumulate.c       |   2 +-
 tools/perf/tests/hists_filter.c         |   4 +-
 tools/perf/tests/hists_link.c           |   4 +-
 tools/perf/tests/hists_output.c         |   2 +-
 tools/perf/tests/keep-tracking.c        |   4 +-
 tools/perf/tests/parse-events.c         |   2 +-
 tools/perf/tests/perf-time-to-tsc.c     |   2 +-
 tools/perf/tests/pmu.c                  |   3 +-
 tools/perf/tests/switch-tracking.c      |   8 +--
 tools/perf/util/parse-events.c          | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 tools/perf/util/parse-events.h          |  36 ++++++++----
 tools/perf/util/parse-events.l          |  41 ++++++++++++--
 tools/perf/util/parse-events.y          |  48 ++++++++--------
 tools/perf/util/pmu.c                   |  57 +++++++++++++++++--
 tools/perf/util/pmu.h                   |   6 +-
 tools/perf/util/record.c                |   4 +-
 19 files changed, 325 insertions(+), 100 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/9] perf tools: Add parse_events_error interface
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:04   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 2/9] perf tools: Add flex support for parse_events_error Jiri Olsa
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Adding support to return error information from parse_events
function. Following struct will be populated by parse_events
function on return:

  struct parse_events_error {
    int   idx;
    char *str;
    char *help;
  };

where 'idx' is the position in the string where the parsing
failed, 'str' contains dynamically allocated error string
describing the error and 'help' is optional help string.

The change contains reporting function, which currently does
not display anything. The code changes to supply error data
for specific event types are coming in next patches. However
this is what the expected output is:

  $ sudo perf record -e 'sched:krava' ls
  event syntax error: 'sched:krava'
                       \___ unknown tracepoint
  ...

  $ perf record -e 'cpu/even=0x1/' ls
  event syntax error: 'cpu/even=0x1/'
                           \___ unknown term

  valid terms: pc,any,inv,edge,cmask,event,in_tx,ldlat,umask,in_tx_cp,offcore_rsp,config,config1,config2,name,period,branch_type
  ...

  $ perf record -e cycles,cache-mises ls
  event syntax error: '..es,cache-mises'
                                 \___ parser error
  ...

The output functions cut the beginning of the event string so the
error starts up to 10th character and cut the end of the string
of it crosses the terminal width.

Link: http://lkml.kernel.org/n/tip-rqil5ug9qe4b6odr90g19umt@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-stat.c               |   2 +-
 tools/perf/tests/code-reading.c         |   2 +-
 tools/perf/tests/evsel-roundtrip-name.c |   4 +-
 tools/perf/tests/hists_cumulate.c       |   2 +-
 tools/perf/tests/hists_filter.c         |   4 +-
 tools/perf/tests/hists_link.c           |   4 +-
 tools/perf/tests/hists_output.c         |   2 +-
 tools/perf/tests/keep-tracking.c        |   4 +-
 tools/perf/tests/parse-events.c         |   2 +-
 tools/perf/tests/perf-time-to-tsc.c     |   2 +-
 tools/perf/tests/switch-tracking.c      |   8 +--
 tools/perf/util/parse-events.c          | 100 +++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.h          |  19 ++++--
 tools/perf/util/record.c                |   4 +-
 14 files changed, 127 insertions(+), 32 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f7b8218785f6..3dbd8c59efc5 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1541,7 +1541,7 @@ static int setup_events(const char * const *attrs, unsigned len)
 	unsigned i;
 
 	for (i = 0; i < len; i++) {
-		if (parse_events(evsel_list, attrs[i]))
+		if (parse_events(evsel_list, attrs[i], NULL))
 			return -1;
 	}
 	return 0;
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index f671ec37a7c4..ca0e480e741b 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -482,7 +482,7 @@ static int do_test_code_reading(bool try_kcore)
 		else
 			str = "cycles";
 		pr_debug("Parsing event '%s'\n", str);
-		ret = parse_events(evlist, str);
+		ret = parse_events(evlist, str, NULL);
 		if (ret < 0) {
 			pr_debug("parse_events failed\n");
 			goto out_err;
diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
index b8d8341b383e..3fa715987a5e 100644
--- a/tools/perf/tests/evsel-roundtrip-name.c
+++ b/tools/perf/tests/evsel-roundtrip-name.c
@@ -23,7 +23,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
 			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
 				__perf_evsel__hw_cache_type_op_res_name(type, op, i,
 									name, sizeof(name));
-				err = parse_events(evlist, name);
+				err = parse_events(evlist, name, NULL);
 				if (err)
 					ret = err;
 			}
@@ -71,7 +71,7 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names)
                 return -ENOMEM;
 
 	for (i = 0; i < nr_names; ++i) {
-		err = parse_events(evlist, names[i]);
+		err = parse_events(evlist, names[i], NULL);
 		if (err) {
 			pr_debug("failed to parse event '%s', err %d\n",
 				 names[i], err);
diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index 18619966454c..b08a95a5ca1a 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -695,7 +695,7 @@ int test__hists_cumulate(void)
 
 	TEST_ASSERT_VAL("No memory", evlist);
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 59e53db7914c..108488cd71fa 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -108,10 +108,10 @@ int test__hists_filter(void)
 
 	TEST_ASSERT_VAL("No memory", evlist);
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
-	err = parse_events(evlist, "task-clock");
+	err = parse_events(evlist, "task-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 278ba8344c23..34c61e4d3352 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -282,10 +282,10 @@ int test__hists_link(void)
 	if (evlist == NULL)
                 return -ENOMEM;
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
-	err = parse_events(evlist, "task-clock");
+	err = parse_events(evlist, "task-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index b52c9faea224..d8a23db80094 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -590,7 +590,7 @@ int test__hists_output(void)
 
 	TEST_ASSERT_VAL("No memory", evlist);
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 7a5ab7b0b8f6..5b171d1e338b 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -78,8 +78,8 @@ int test__keep_tracking(void)
 
 	perf_evlist__set_maps(evlist, cpus, threads);
 
-	CHECK__(parse_events(evlist, "dummy:u"));
-	CHECK__(parse_events(evlist, "cycles:u"));
+	CHECK__(parse_events(evlist, "dummy:u", NULL));
+	CHECK__(parse_events(evlist, "cycles:u", NULL));
 
 	perf_evlist__config(evlist, &opts);
 
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 3de744961739..82d2a1636f7f 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1571,7 +1571,7 @@ static int test_event(struct evlist_test *e)
 	if (evlist == NULL)
 		return -ENOMEM;
 
-	ret = parse_events(evlist, e->name);
+	ret = parse_events(evlist, e->name, NULL);
 	if (ret) {
 		pr_debug("failed to parse event '%s', err %d\n",
 			 e->name, ret);
diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
index f238442b238a..5f49484f1abc 100644
--- a/tools/perf/tests/perf-time-to-tsc.c
+++ b/tools/perf/tests/perf-time-to-tsc.c
@@ -68,7 +68,7 @@ int test__perf_time_to_tsc(void)
 
 	perf_evlist__set_maps(evlist, cpus, threads);
 
-	CHECK__(parse_events(evlist, "cycles:u"));
+	CHECK__(parse_events(evlist, "cycles:u", NULL));
 
 	perf_evlist__config(evlist, &opts);
 
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index cc68648c7c55..0d31403ea593 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -347,7 +347,7 @@ int test__switch_tracking(void)
 	perf_evlist__set_maps(evlist, cpus, threads);
 
 	/* First event */
-	err = parse_events(evlist, "cpu-clock:u");
+	err = parse_events(evlist, "cpu-clock:u", NULL);
 	if (err) {
 		pr_debug("Failed to parse event dummy:u\n");
 		goto out_err;
@@ -356,7 +356,7 @@ int test__switch_tracking(void)
 	cpu_clocks_evsel = perf_evlist__last(evlist);
 
 	/* Second event */
-	err = parse_events(evlist, "cycles:u");
+	err = parse_events(evlist, "cycles:u", NULL);
 	if (err) {
 		pr_debug("Failed to parse event cycles:u\n");
 		goto out_err;
@@ -371,7 +371,7 @@ int test__switch_tracking(void)
 		goto out;
 	}
 
-	err = parse_events(evlist, sched_switch);
+	err = parse_events(evlist, sched_switch, NULL);
 	if (err) {
 		pr_debug("Failed to parse event %s\n", sched_switch);
 		goto out_err;
@@ -401,7 +401,7 @@ int test__switch_tracking(void)
 	perf_evsel__set_sample_bit(cycles_evsel, TIME);
 
 	/* Fourth event */
-	err = parse_events(evlist, "dummy:u");
+	err = parse_events(evlist, "dummy:u", NULL);
 	if (err) {
 		pr_debug("Failed to parse event dummy:u\n");
 		goto out_err;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index be0655388b38..f2b46f3bc11b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -17,6 +17,7 @@
 #include "parse-events-flex.h"
 #include "pmu.h"
 #include "thread_map.h"
+#include "asm/bug.h"
 
 #define MAX_NAME_LEN 100
 
@@ -1019,11 +1020,13 @@ int parse_events_terms(struct list_head *terms, const char *str)
 	return ret;
 }
 
-int parse_events(struct perf_evlist *evlist, const char *str)
+int parse_events(struct perf_evlist *evlist, const char *str,
+		 struct parse_events_error *error)
 {
 	struct parse_events_evlist data = {
-		.list = LIST_HEAD_INIT(data.list),
-		.idx  = evlist->nr_entries,
+		.list  = LIST_HEAD_INIT(data.list),
+		.idx   = evlist->nr_entries,
+		.error = error,
 	};
 	int ret;
 
@@ -1044,16 +1047,87 @@ int parse_events(struct perf_evlist *evlist, const char *str)
 	return ret;
 }
 
+#define MAX_WIDTH 1000
+static int get_term_width(void)
+{
+	struct winsize ws;
+
+	get_term_dimensions(&ws);
+	return ws.ws_col > MAX_WIDTH ? MAX_WIDTH : ws.ws_col;
+}
+
+static void parse_events_print_error(struct parse_events_error *error,
+				     const char *event)
+{
+	const char *str = "invalid or unsupported event: ";
+	char _buf[MAX_WIDTH];
+	char *buf = (char *) event;
+	int idx = 0;
+
+	if (error->str) {
+		/* -2 for extra '' in the final fprintf */
+		int width       = get_term_width() - 2;
+		int len_event   = strlen(event);
+		int len_str, max_len, cut = 0;
+
+		/*
+		 * Maximum error index indent, we will cut
+		 * the event string if it's bigger.
+		 */
+		int max_err_idx = 10;
+
+		/*
+		 * Let's be specific with the message when
+		 * we have the precise error.
+		 */
+		str     = "event syntax error: ";
+		len_str = strlen(str);
+		max_len = width - len_str;
+
+		buf = _buf;
+
+		/* We're cutting from the beggining. */
+		if (error->idx > max_err_idx)
+			cut = error->idx - max_err_idx;
+
+		strncpy(buf, event + cut, max_len);
+
+		/* Mark cut parts with '..' on both sides. */
+		if (cut)
+			buf[0] = buf[1] = '.';
+
+		if ((len_event - cut) > max_len) {
+			buf[max_len - 1] = buf[max_len - 2] = '.';
+			buf[max_len] = 0;
+		}
+
+		idx = len_str + error->idx - cut;
+	}
+
+	fprintf(stderr, "%s'%s'\n", str, buf);
+	if (idx) {
+		fprintf(stderr, "%*s\\___ %s\n", idx + 1, "", error->str);
+		if (error->help)
+			fprintf(stderr, "\n%s\n", error->help);
+		free(error->str);
+		free(error->help);
+	}
+
+	fprintf(stderr, "Run 'perf list' for a list of valid events\n");
+}
+
+#undef MAX_WIDTH
+
 int parse_events_option(const struct option *opt, const char *str,
 			int unset __maybe_unused)
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
-	int ret = parse_events(evlist, str);
+	struct parse_events_error error = { .idx = 0, };
+	int ret = parse_events(evlist, str, &error);
+
+	if (ret)
+		parse_events_print_error(&error, str);
 
-	if (ret) {
-		fprintf(stderr, "invalid or unsupported event: '%s'\n", str);
-		fprintf(stderr, "Run 'perf list' for a list of valid events\n");
-	}
 	return ret;
 }
 
@@ -1535,3 +1609,13 @@ void parse_events__free_terms(struct list_head *terms)
 	list_for_each_entry_safe(term, h, terms, list)
 		free(term);
 }
+
+void parse_events_evlist_error(struct parse_events_evlist *data,
+			       int idx, const char *str)
+{
+	struct parse_events_error *error = data->error;
+
+	error->idx = idx;
+	error->str = strdup(str);
+	WARN_ONCE(!error->str, "WARNING: failed to allocate error string");
+}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 52a2dda4f954..5ac2ffa0a145 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -12,6 +12,7 @@
 struct list_head;
 struct perf_evsel;
 struct perf_evlist;
+struct parse_events_error;
 
 struct option;
 
@@ -29,7 +30,8 @@ const char *event_type(int type);
 
 extern int parse_events_option(const struct option *opt, const char *str,
 			       int unset);
-extern int parse_events(struct perf_evlist *evlist, const char *str);
+extern int parse_events(struct perf_evlist *evlist, const char *str,
+			struct parse_events_error *error);
 extern int parse_events_terms(struct list_head *terms, const char *str);
 extern int parse_filter(const struct option *opt, const char *str, int unset);
 
@@ -74,10 +76,17 @@ struct parse_events_term {
 	bool used;
 };
 
+struct parse_events_error {
+	int   idx;	/* index in the parsed string */
+	char *str;      /* string to display at the index */
+	char *help;	/* optional help string */
+};
+
 struct parse_events_evlist {
-	struct list_head list;
-	int idx;
-	int nr_groups;
+	struct list_head	   list;
+	int			   idx;
+	int			   nr_groups;
+	struct parse_events_error *error;
 };
 
 struct parse_events_terms {
@@ -114,6 +123,8 @@ void parse_events__set_leader(char *name, struct list_head *list);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
 void parse_events_error(void *data, void *scanner, char const *msg);
+void parse_events_evlist_error(struct parse_events_evlist *data,
+			       int idx, const char *str);
 
 void print_events(const char *event_glob, bool name_only);
 
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index 0ccfa498f7b8..d457c523a33d 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -20,7 +20,7 @@ static int perf_do_probe_api(setup_probe_fn_t fn, int cpu, const char *str)
 	if (!evlist)
 		return -ENOMEM;
 
-	if (parse_events(evlist, str))
+	if (parse_events(evlist, str, NULL))
 		goto out_delete;
 
 	evsel = perf_evlist__first(evlist);
@@ -216,7 +216,7 @@ bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str)
 	if (!temp_evlist)
 		return false;
 
-	err = parse_events(temp_evlist, str);
+	err = parse_events(temp_evlist, str, NULL);
 	if (err)
 		goto out_delete;
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/9] perf tools: Add flex support for parse_events_error
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
  2015-04-22 19:10 ` [PATCH 1/9] perf tools: Add parse_events_error interface Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:04   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 3/9] perf tools: Always bail out when config_attr function fails Jiri Olsa
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Allowing flex parser to report back event parsing error, like:

  $ perf record -e cycles,cache-mises ls
  event syntax error: '..es,cache-mises'
                                 \___ parser error
  ...

Link: http://lkml.kernel.org/n/tip-j2ek52is8nt846pmyiib05ux@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.h |  1 -
 tools/perf/util/parse-events.l | 37 +++++++++++++++++++++++++++++++++----
 tools/perf/util/parse-events.y |  7 ++++---
 3 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 5ac2ffa0a145..eb12bcd12642 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -122,7 +122,6 @@ perf_pmu__parse_check(const char *name);
 void parse_events__set_leader(char *name, struct list_head *list);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
-void parse_events_error(void *data, void *scanner, char const *msg);
 void parse_events_evlist_error(struct parse_events_evlist *data,
 			       int idx, const char *str);
 
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 8895cf3132ab..330dd2d35f5a 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -3,6 +3,8 @@
 %option bison-bridge
 %option prefix="parse_events_"
 %option stack
+%option bison-locations
+%option yylineno
 
 %{
 #include <errno.h>
@@ -51,6 +53,18 @@ static int str(yyscan_t scanner, int token)
 	return token;
 }
 
+#define REWIND(__alloc)				\
+do {								\
+	YYSTYPE *__yylval = parse_events_get_lval(yyscanner);	\
+	char *text = parse_events_get_text(yyscanner);		\
+								\
+	if (__alloc)						\
+		__yylval->str = strdup(text);			\
+								\
+	yycolumn -= strlen(text);				\
+	yyless(0);						\
+} while (0)
+
 static int pmu_str_check(yyscan_t scanner)
 {
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
@@ -85,6 +99,13 @@ static int term(yyscan_t scanner, int type)
 	return PE_TERM;
 }
 
+#define YY_USER_ACTION					\
+do {							\
+	yylloc->last_column  = yylloc->first_column;	\
+	yylloc->first_column = yycolumn;		\
+	yycolumn += yyleng;				\
+} while (0);
+
 %}
 
 %x mem
@@ -119,6 +140,12 @@ modifier_bp	[rwx]{1,3}
 
 		if (start_token) {
 			parse_events_set_extra(NULL, yyscanner);
+			/*
+			 * The flex parser does not init locations variable
+			 * via the scan_string interface, so we need do the
+			 * init in here.
+			 */
+			yycolumn = 0;
 			return start_token;
 		}
          }
@@ -127,19 +154,21 @@ modifier_bp	[rwx]{1,3}
 <event>{
 
 {group}		{
-			BEGIN(INITIAL); yyless(0);
+			BEGIN(INITIAL);
+			REWIND(0);
 		}
 
 {event_pmu}	|
 {event}		{
-			str(yyscanner, PE_EVENT_NAME);
-			BEGIN(INITIAL); yyless(0);
+			BEGIN(INITIAL);
+			REWIND(1);
 			return PE_EVENT_NAME;
 		}
 
 .		|
 <<EOF>>		{
-			BEGIN(INITIAL); yyless(0);
+			BEGIN(INITIAL);
+			REWIND(0);
 		}
 
 }
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 72def077dbbf..14521ce534d9 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -2,6 +2,7 @@
 %parse-param {void *_data}
 %parse-param {void *scanner}
 %lex-param {void* scanner}
+%locations
 
 %{
 
@@ -14,8 +15,6 @@
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
-extern int parse_events_lex (YYSTYPE* lvalp, void* scanner);
-
 #define ABORT_ON(val) \
 do { \
 	if (val) \
@@ -520,7 +519,9 @@ sep_slash_dc: '/' | ':' |
 
 %%
 
-void parse_events_error(void *data __maybe_unused, void *scanner __maybe_unused,
+void parse_events_error(YYLTYPE *loc, void *data,
+			void *scanner __maybe_unused,
 			char const *msg __maybe_unused)
 {
+	parse_events_evlist_error(data, loc->last_column, "parser error");
 }
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/9] perf tools: Always bail out when config_attr function fails
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
  2015-04-22 19:10 ` [PATCH 1/9] perf tools: Add parse_events_error interface Jiri Olsa
  2015-04-22 19:10 ` [PATCH 2/9] perf tools: Add flex support for parse_events_error Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:04   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 4/9] perf tools: Change parse_events_add_pmu interface Jiri Olsa
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Not sure why we allowed the fail state, but it's wrong.
Wrong type for 'name' term can cause segfault, and there's
probably more fun hidden.

Link: http://lkml.kernel.org/n/tip-d9en7cw3sy06ezy0olvofaog@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f2b46f3bc11b..c52705634267 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -549,6 +549,12 @@ do {								\
 } while (0)
 
 	switch (term->type_term) {
+	case PARSE_EVENTS__TERM_TYPE_USER:
+		/*
+		 * Always succeed for sysfs terms, as we dont know
+		 * at this point what type they need to have.
+		 */
+		return 0;
 	case PARSE_EVENTS__TERM_TYPE_CONFIG:
 		CHECK_TYPE_VAL(NUM);
 		attr->config = term->val.num;
@@ -583,12 +589,12 @@ do {								\
 }
 
 static int config_attr(struct perf_event_attr *attr,
-		       struct list_head *head, int fail)
+		       struct list_head *head)
 {
 	struct parse_events_term *term;
 
 	list_for_each_entry(term, head, list)
-		if (config_term(attr, term) && fail)
+		if (config_term(attr, term))
 			return -EINVAL;
 
 	return 0;
@@ -605,7 +611,7 @@ int parse_events_add_numeric(struct list_head *list, int *idx,
 	attr.config = config;
 
 	if (head_config &&
-	    config_attr(&attr, head_config, 1))
+	    config_attr(&attr, head_config))
 		return -EINVAL;
 
 	return add_event(list, idx, &attr, NULL);
@@ -659,7 +665,8 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
 	 * Configure hardcoded terms first, no need to check
 	 * return value when called with fail == 0 ;)
 	 */
-	config_attr(&attr, head_config, 0);
+	if (config_attr(&attr, head_config))
+		return -EINVAL;
 
 	if (perf_pmu__config(pmu, &attr, head_config))
 		return -EINVAL;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/9] perf tools: Change parse_events_add_pmu interface
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
                   ` (2 preceding siblings ...)
  2015-04-22 19:10 ` [PATCH 3/9] perf tools: Always bail out when config_attr function fails Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 5/9] perf tools: Add location to pmu event terms Jiri Olsa
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Changing parse_events_add_pmu interface to allow
propagating of the parse_events_error info.

Link: http://lkml.kernel.org/n/tip-lhjgvd0h5e9i890nprnrnzew@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c | 11 ++++++-----
 tools/perf/util/parse-events.h |  5 +++--
 tools/perf/util/parse-events.y |  6 +++---
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c52705634267..0e8ccea9998b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -633,8 +633,9 @@ static char *pmu_event_name(struct list_head *head_terms)
 	return NULL;
 }
 
-int parse_events_add_pmu(struct list_head *list, int *idx,
-			 char *name, struct list_head *head_config)
+int parse_events_add_pmu(struct parse_events_evlist *data,
+			 struct list_head *list, char *name,
+			 struct list_head *head_config)
 {
 	struct perf_event_attr attr;
 	struct perf_pmu_info info;
@@ -654,7 +655,7 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
 
 	if (!head_config) {
 		attr.type = pmu->type;
-		evsel = __add_event(list, idx, &attr, NULL, pmu->cpus);
+		evsel = __add_event(list, &data->idx, &attr, NULL, pmu->cpus);
 		return evsel ? 0 : -ENOMEM;
 	}
 
@@ -671,8 +672,8 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
 	if (perf_pmu__config(pmu, &attr, head_config))
 		return -EINVAL;
 
-	evsel = __add_event(list, idx, &attr, pmu_event_name(head_config),
-			    pmu->cpus);
+	evsel = __add_event(list, &data->idx, &attr,
+			    pmu_event_name(head_config), pmu->cpus);
 	if (evsel) {
 		evsel->unit = info.unit;
 		evsel->scale = info.scale;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index eb12bcd12642..76ea3de288da 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -115,8 +115,9 @@ int parse_events_add_cache(struct list_head *list, int *idx,
 			   char *type, char *op_result1, char *op_result2);
 int parse_events_add_breakpoint(struct list_head *list, int *idx,
 				void *ptr, char *type, u64 len);
-int parse_events_add_pmu(struct list_head *list, int *idx,
-			 char *pmu , struct list_head *head_config);
+int parse_events_add_pmu(struct parse_events_evlist *data,
+			 struct list_head *list, char *name,
+			 struct list_head *head_config);
 enum perf_pmu_event_symbol_type
 perf_pmu__parse_check(const char *name);
 void parse_events__set_leader(char *name, struct list_head *list);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 14521ce534d9..84596617b355 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -207,7 +207,7 @@ PE_NAME '/' event_config '/'
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(list, &data->idx, $1, $3));
+	ABORT_ON(parse_events_add_pmu(data, list, $1, $3));
 	parse_events__free_terms($3);
 	$$ = list;
 }
@@ -218,7 +218,7 @@ PE_NAME '/' '/'
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(list, &data->idx, $1, NULL));
+	ABORT_ON(parse_events_add_pmu(data, list, $1, NULL));
 	$$ = list;
 }
 |
@@ -235,7 +235,7 @@ PE_KERNEL_PMU_EVENT sep_dc
 	list_add_tail(&term->list, head);
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(list, &data->idx, "cpu", head));
+	ABORT_ON(parse_events_add_pmu(data, list, "cpu", head));
 	parse_events__free_terms(head);
 	$$ = list;
 }
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/9] perf tools: Add location to pmu event terms
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
                   ` (3 preceding siblings ...)
  2015-04-22 19:10 ` [PATCH 4/9] perf tools: Change parse_events_add_pmu interface Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 6/9] perf tools: Add term support for parse_events_error Jiri Olsa
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Saving the terms location within term struct,
so it could be used later for report.

Link: http://lkml.kernel.org/n/tip-0usk1luyaeeguh8qmxwmmedr@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c | 32 ++++++++++++++++++++++++--------
 tools/perf/util/parse-events.h |  8 ++++----
 tools/perf/util/parse-events.y | 16 ++++++++--------
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 0e8ccea9998b..d3f4567bd622 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -25,6 +25,12 @@
 extern int parse_events_debug;
 #endif
 int parse_events_parse(void *data, void *scanner);
+int parse_events_term__num(struct parse_events_term **term,
+			   int type_term, char *config, u64 num,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val);
+int parse_events_term__str(struct parse_events_term **term,
+			   int type_term, char *config, char *str,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val);
 
 static struct perf_pmu_event_symbol *perf_pmu_events_list;
 /*
@@ -1542,7 +1548,7 @@ int parse_events__is_hardcoded_term(struct parse_events_term *term)
 
 static int new_term(struct parse_events_term **_term, int type_val,
 		    int type_term, char *config,
-		    char *str, u64 num)
+		    char *str, u64 num, int err_term, int err_val)
 {
 	struct parse_events_term *term;
 
@@ -1554,6 +1560,8 @@ static int new_term(struct parse_events_term **_term, int type_val,
 	term->type_val  = type_val;
 	term->type_term = type_term;
 	term->config = config;
+	term->err_term = err_term;
+	term->err_val  = err_val;
 
 	switch (type_val) {
 	case PARSE_EVENTS__TERM_TYPE_NUM:
@@ -1572,17 +1580,23 @@ static int new_term(struct parse_events_term **_term, int type_val,
 }
 
 int parse_events_term__num(struct parse_events_term **term,
-			   int type_term, char *config, u64 num)
+			   int type_term, char *config, u64 num,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val)
 {
 	return new_term(term, PARSE_EVENTS__TERM_TYPE_NUM, type_term,
-			config, NULL, num);
+			config, NULL, num,
+			loc_term ? loc_term->first_column : 0,
+			loc_val ? loc_val->first_column : 0);
 }
 
 int parse_events_term__str(struct parse_events_term **term,
-			   int type_term, char *config, char *str)
+			   int type_term, char *config, char *str,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val)
 {
 	return new_term(term, PARSE_EVENTS__TERM_TYPE_STR, type_term,
-			config, str, 0);
+			config, str, 0,
+			loc_term ? loc_term->first_column : 0,
+			loc_val ? loc_val->first_column : 0);
 }
 
 int parse_events_term__sym_hw(struct parse_events_term **term,
@@ -1596,18 +1610,20 @@ int parse_events_term__sym_hw(struct parse_events_term **term,
 	if (config)
 		return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
 				PARSE_EVENTS__TERM_TYPE_USER, config,
-				(char *) sym->symbol, 0);
+				(char *) sym->symbol, 0, 0, 0);
 	else
 		return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
 				PARSE_EVENTS__TERM_TYPE_USER,
-				(char *) "event", (char *) sym->symbol, 0);
+				(char *) "event", (char *) sym->symbol,
+				0, 0, 0);
 }
 
 int parse_events_term__clone(struct parse_events_term **new,
 			     struct parse_events_term *term)
 {
 	return new_term(new, term->type_val, term->type_term, term->config,
-			term->val.str, term->val.num);
+			term->val.str, term->val.num,
+			term->err_term, term->err_val);
 }
 
 void parse_events__free_terms(struct list_head *terms)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 76ea3de288da..6286ffdf2295 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -74,6 +74,10 @@ struct parse_events_term {
 	int type_term;
 	struct list_head list;
 	bool used;
+
+	/* error string indexes for within parsed string */
+	int err_term;
+	int err_val;
 };
 
 struct parse_events_error {
@@ -94,10 +98,6 @@ struct parse_events_terms {
 };
 
 int parse_events__is_hardcoded_term(struct parse_events_term *term);
-int parse_events_term__num(struct parse_events_term **_term,
-			   int type_term, char *config, u64 num);
-int parse_events_term__str(struct parse_events_term **_term,
-			   int type_term, char *config, char *str);
 int parse_events_term__sym_hw(struct parse_events_term **term,
 			      char *config, unsigned idx);
 int parse_events_term__clone(struct parse_events_term **new,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 84596617b355..486247739a39 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -231,7 +231,7 @@ PE_KERNEL_PMU_EVENT sep_dc
 
 	ALLOC_LIST(head);
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, 1));
+					$1, 1, &@1, NULL));
 	list_add_tail(&term->list, head);
 
 	ALLOC_LIST(list);
@@ -251,7 +251,7 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
 
 	ALLOC_LIST(head);
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					&pmu_name, 1));
+					&pmu_name, 1, &@1, NULL));
 	list_add_tail(&term->list, head);
 
 	ALLOC_LIST(list);
@@ -449,7 +449,7 @@ PE_NAME '=' PE_NAME
 	struct parse_events_term *term;
 
 	ABORT_ON(parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, $3));
+					$1, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -458,7 +458,7 @@ PE_NAME '=' PE_VALUE
 	struct parse_events_term *term;
 
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, $3));
+					$1, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -476,7 +476,7 @@ PE_NAME
 	struct parse_events_term *term;
 
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, 1));
+					$1, 1, &@1, NULL));
 	$$ = term;
 }
 |
@@ -493,7 +493,7 @@ PE_TERM '=' PE_NAME
 {
 	struct parse_events_term *term;
 
-	ABORT_ON(parse_events_term__str(&term, (int)$1, NULL, $3));
+	ABORT_ON(parse_events_term__str(&term, (int)$1, NULL, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -501,7 +501,7 @@ PE_TERM '=' PE_VALUE
 {
 	struct parse_events_term *term;
 
-	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3));
+	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -509,7 +509,7 @@ PE_TERM
 {
 	struct parse_events_term *term;
 
-	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1));
+	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, &@1, NULL));
 	$$ = term;
 }
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/9] perf tools: Add term support for parse_events_error
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
                   ` (4 preceding siblings ...)
  2015-04-22 19:10 ` [PATCH 5/9] perf tools: Add location to pmu event terms Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 7/9] perf tools: Add static terms " Jiri Olsa
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Allowing event's term processing to report back error, like:

  $ perf record -e 'cpu/even=0x1/' ls
  event syntax error: 'cpu/even=0x1/'
                           \___ unknown term

  valid terms: pc,any,inv,edge,cmask,event,in_tx,ldlat,umask,in_tx_cp,offcore_rsp,config,config1,config2,name,period,branch_type

Link: http://lkml.kernel.org/n/tip-scysppvzcayvg4iusztryrss@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/pmu.c         |  3 ++-
 tools/perf/util/parse-events.c |  2 +-
 tools/perf/util/parse-events.l |  4 +++
 tools/perf/util/pmu.c          | 57 +++++++++++++++++++++++++++++++++++++-----
 tools/perf/util/pmu.h          |  6 +++--
 5 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index eeb68bb1972d..faa04e9d5d5f 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -152,7 +152,8 @@ int test__pmu(void)
 		if (ret)
 			break;
 
-		ret = perf_pmu__config_terms(&formats, &attr, terms, false);
+		ret = perf_pmu__config_terms(&formats, &attr, terms,
+					     false, NULL);
 		if (ret)
 			break;
 
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d3f4567bd622..9c2e1aece477 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -675,7 +675,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 	if (config_attr(&attr, head_config))
 		return -EINVAL;
 
-	if (perf_pmu__config(pmu, &attr, head_config))
+	if (perf_pmu__config(pmu, &attr, head_config, data->error))
 		return -EINVAL;
 
 	evsel = __add_event(list, &data->idx, &attr,
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 330dd2d35f5a..09e738fe9ea2 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -174,6 +174,10 @@ modifier_bp	[rwx]{1,3}
 }
 
 <config>{
+	/*
+	 * Please update formats_error_string any time
+	 * new static term is added.
+	 */
 config			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG); }
 config1			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG1); }
 config2			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG2); }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 48411674da0f..4cd4f84caad7 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -579,6 +579,38 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
 	return -1;
 }
 
+static char *formats_error_string(struct list_head *formats)
+{
+	struct perf_pmu_format *format;
+	char *error, *str;
+	static const char *static_terms = "config,config1,config2,name,period,branch_type\n";
+	unsigned i = 0;
+
+	if (!asprintf(&str, "valid terms:"))
+		return NULL;
+
+	/* sysfs exported terms */
+	list_for_each_entry(format, formats, list) {
+		char c = i++ ? ',' : ' ';
+
+		error = str;
+		if (!asprintf(&str, "%s%c%s", error, c, format->name))
+			goto fail;
+		free(error);
+	}
+
+	/* static terms */
+	error = str;
+	if (!asprintf(&str, "%s,%s", error, static_terms))
+		goto fail;
+
+	free(error);
+	return str;
+fail:
+	free(error);
+	return NULL;
+}
+
 /*
  * Setup one of config[12] attr members based on the
  * user input data - term parameter.
@@ -587,7 +619,7 @@ static int pmu_config_term(struct list_head *formats,
 			   struct perf_event_attr *attr,
 			   struct parse_events_term *term,
 			   struct list_head *head_terms,
-			   bool zero)
+			   bool zero, struct parse_events_error *error)
 {
 	struct perf_pmu_format *format;
 	__u64 *vp;
@@ -611,6 +643,11 @@ static int pmu_config_term(struct list_head *formats,
 	if (!format) {
 		if (verbose)
 			printf("Invalid event/parameter '%s'\n", term->config);
+		if (error) {
+			error->idx  = term->err_term;
+			error->str  = strdup("unknown term");
+			error->help = formats_error_string(formats);
+		}
 		return -EINVAL;
 	}
 
@@ -636,9 +673,14 @@ static int pmu_config_term(struct list_head *formats,
 		val = term->val.num;
 	else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
 		if (strcmp(term->val.str, "?")) {
-			if (verbose)
+			if (verbose) {
 				pr_info("Invalid sysfs entry %s=%s\n",
 						term->config, term->val.str);
+			}
+			if (error) {
+				error->idx = term->err_val;
+				error->str = strdup("expected numeric value");
+			}
 			return -EINVAL;
 		}
 
@@ -654,12 +696,13 @@ static int pmu_config_term(struct list_head *formats,
 int perf_pmu__config_terms(struct list_head *formats,
 			   struct perf_event_attr *attr,
 			   struct list_head *head_terms,
-			   bool zero)
+			   bool zero, struct parse_events_error *error)
 {
 	struct parse_events_term *term;
 
 	list_for_each_entry(term, head_terms, list) {
-		if (pmu_config_term(formats, attr, term, head_terms, zero))
+		if (pmu_config_term(formats, attr, term, head_terms,
+				    zero, error))
 			return -EINVAL;
 	}
 
@@ -672,12 +715,14 @@ int perf_pmu__config_terms(struct list_head *formats,
  * 2) pmu format definitions - specified by pmu parameter
  */
 int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
-		     struct list_head *head_terms)
+		     struct list_head *head_terms,
+		     struct parse_events_error *error)
 {
 	bool zero = !!pmu->default_config;
 
 	attr->type = pmu->type;
-	return perf_pmu__config_terms(&pmu->format, attr, head_terms, zero);
+	return perf_pmu__config_terms(&pmu->format, attr, head_terms,
+				      zero, error);
 }
 
 static struct perf_pmu_alias *pmu_find_alias(struct perf_pmu *pmu,
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 6b1249fbdb5f..7b9c8cf8ae3e 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -4,6 +4,7 @@
 #include <linux/bitmap.h>
 #include <linux/perf_event.h>
 #include <stdbool.h>
+#include "parse-events.h"
 
 enum {
 	PERF_PMU_FORMAT_VALUE_CONFIG,
@@ -47,11 +48,12 @@ struct perf_pmu_alias {
 
 struct perf_pmu *perf_pmu__find(const char *name);
 int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
-		     struct list_head *head_terms);
+		     struct list_head *head_terms,
+		     struct parse_events_error *error);
 int perf_pmu__config_terms(struct list_head *formats,
 			   struct perf_event_attr *attr,
 			   struct list_head *head_terms,
-			   bool zero);
+			   bool zero, struct parse_events_error *error);
 int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
 			  struct perf_pmu_info *info);
 struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/9] perf tools: Add static terms support for parse_events_error
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
                   ` (5 preceding siblings ...)
  2015-04-22 19:10 ` [PATCH 6/9] perf tools: Add term support for parse_events_error Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 8/9] perf tools: Add tracepoint " Jiri Olsa
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Allowing static terms like 'name,period,config,config1..' processing
to report back error.

  $ perf record -e 'cpu/event=1,name=1/' ls
  event syntax error: '..=1,name=1/'
                                 \___ expected string value

  $ perf record -e 'cpu/event=1,period=krava/' ls
  event syntax error: '..,period=krava/'
                                 \___ expected numeric value

  $ perf record -e 'cpu/config=krava1/' ls
  event syntax error: '../config=krava1/'
                                 \___ expected numeric value

Link: http://lkml.kernel.org/n/tip-dvkdtxlkbmdbl1b22fic4jut@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9c2e1aece477..73f6bb65e310 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -545,13 +545,31 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 	return add_event(list, idx, &attr, NULL);
 }
 
+static int check_type_val(struct parse_events_term *term,
+			  struct parse_events_error *error,
+			  int type)
+{
+	if (type == term->type_val)
+		return 0;
+
+	if (error) {
+		error->idx = term->err_val;
+		if (type == PARSE_EVENTS__TERM_TYPE_NUM)
+			error->str = strdup("expected numeric value");
+		else
+			error->str = strdup("expected string value");
+	}
+	return -EINVAL;
+}
+
 static int config_term(struct perf_event_attr *attr,
-		       struct parse_events_term *term)
+		       struct parse_events_term *term,
+		       struct parse_events_error *error)
 {
-#define CHECK_TYPE_VAL(type)					\
-do {								\
-	if (PARSE_EVENTS__TERM_TYPE_ ## type != term->type_val)	\
-		return -EINVAL;					\
+#define CHECK_TYPE_VAL(type)						   \
+do {									   \
+	if (check_type_val(term, error, PARSE_EVENTS__TERM_TYPE_ ## type)) \
+		return -EINVAL;						   \
 } while (0)
 
 	switch (term->type_term) {
@@ -595,12 +613,13 @@ do {								\
 }
 
 static int config_attr(struct perf_event_attr *attr,
-		       struct list_head *head)
+		       struct list_head *head,
+		       struct parse_events_error *error)
 {
 	struct parse_events_term *term;
 
 	list_for_each_entry(term, head, list)
-		if (config_term(attr, term))
+		if (config_term(attr, term, error))
 			return -EINVAL;
 
 	return 0;
@@ -617,7 +636,7 @@ int parse_events_add_numeric(struct list_head *list, int *idx,
 	attr.config = config;
 
 	if (head_config &&
-	    config_attr(&attr, head_config))
+	    config_attr(&attr, head_config, NULL))
 		return -EINVAL;
 
 	return add_event(list, idx, &attr, NULL);
@@ -672,7 +691,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 	 * Configure hardcoded terms first, no need to check
 	 * return value when called with fail == 0 ;)
 	 */
-	if (config_attr(&attr, head_config))
+	if (config_attr(&attr, head_config, data->error))
 		return -EINVAL;
 
 	if (perf_pmu__config(pmu, &attr, head_config, data->error))
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 8/9] perf tools: Add tracepoint support for parse_events_error
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
                   ` (6 preceding siblings ...)
  2015-04-22 19:10 ` [PATCH 7/9] perf tools: Add static terms " Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:06   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-22 19:10 ` [PATCH 9/9] perf tools: Add symbolic events " Jiri Olsa
  2015-04-23 14:44 ` [PATCH 0/9] perf tools: Report event parsing errors Arnaldo Carvalho de Melo
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Allowing tracepoint events processing to report back error.

  $ perf record -e 'sched:krava' ls
  event syntax error: 'sched:krava'
                       \___ unknown tracepoint
  ...

Link: http://lkml.kernel.org/n/tip-xkbpn3j5of376ag7utcirt8w@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.y | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 486247739a39..38a0f21fc433 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -388,7 +388,13 @@ PE_NAME ':' PE_NAME
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_tracepoint(list, &data->idx, $1, $3));
+	if (parse_events_add_tracepoint(list, &data->idx, $1, $3)) {
+		struct parse_events_error *error = data->error;
+
+		error->idx = @1.first_column;
+		error->str = strdup("unknown tracepoint");
+		return -1;
+	}
 	$$ = list;
 }
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 9/9] perf tools: Add symbolic events support for parse_events_error
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
                   ` (7 preceding siblings ...)
  2015-04-22 19:10 ` [PATCH 8/9] perf tools: Add tracepoint " Jiri Olsa
@ 2015-04-22 19:10 ` Jiri Olsa
  2015-05-06  3:06   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2015-04-23 14:44 ` [PATCH 0/9] perf tools: Report event parsing errors Arnaldo Carvalho de Melo
  9 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-04-22 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Allowing symbolic events processing to report back error.

  $ perf record -e 'cycles/period=krava/' ls
  event syntax error: '../period=krava/'
                                 \___ expected numeric value

  $ perf record -e 'cycles/name=1/' ls
  event syntax error: '..es/name=1/'
                                 \___ expected string value

Link: http://lkml.kernel.org/n/tip-10kn89sm5yafl1a5t9mqef9z@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c |  7 ++++---
 tools/perf/util/parse-events.h |  3 ++-
 tools/perf/util/parse-events.y | 11 ++++-------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 73f6bb65e310..98a44b7ccd62 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -625,7 +625,8 @@ static int config_attr(struct perf_event_attr *attr,
 	return 0;
 }
 
-int parse_events_add_numeric(struct list_head *list, int *idx,
+int parse_events_add_numeric(struct parse_events_evlist *data,
+			     struct list_head *list,
 			     u32 type, u64 config,
 			     struct list_head *head_config)
 {
@@ -636,10 +637,10 @@ int parse_events_add_numeric(struct list_head *list, int *idx,
 	attr.config = config;
 
 	if (head_config &&
-	    config_attr(&attr, head_config, NULL))
+	    config_attr(&attr, head_config, data->error))
 		return -EINVAL;
 
-	return add_event(list, idx, &attr, NULL);
+	return add_event(list, &data->idx, &attr, NULL);
 }
 
 static int parse_events__is_name_term(struct parse_events_term *term)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 6286ffdf2295..e236f1b6ac6f 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -108,7 +108,8 @@ int parse_events__modifier_group(struct list_head *list, char *event_mod);
 int parse_events_name(struct list_head *list, char *name);
 int parse_events_add_tracepoint(struct list_head *list, int *idx,
 				char *sys, char *event);
-int parse_events_add_numeric(struct list_head *list, int *idx,
+int parse_events_add_numeric(struct parse_events_evlist *data,
+			     struct list_head *list,
 			     u32 type, u64 config,
 			     struct list_head *head_config);
 int parse_events_add_cache(struct list_head *list, int *idx,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 38a0f21fc433..3d11e00243e3 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -274,8 +274,7 @@ value_sym '/' event_config '/'
 	int config = $1 & 255;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx,
-					  type, config, $3));
+	ABORT_ON(parse_events_add_numeric(data, list, type, config, $3));
 	parse_events__free_terms($3);
 	$$ = list;
 }
@@ -288,8 +287,7 @@ value_sym sep_slash_dc
 	int config = $1 & 255;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx,
-					  type, config, NULL));
+	ABORT_ON(parse_events_add_numeric(data, list, type, config, NULL));
 	$$ = list;
 }
 
@@ -405,7 +403,7 @@ PE_VALUE ':' PE_VALUE
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx, (u32)$1, $3, NULL));
+	ABORT_ON(parse_events_add_numeric(data, list, (u32)$1, $3, NULL));
 	$$ = list;
 }
 
@@ -416,8 +414,7 @@ PE_RAW
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx,
-					  PERF_TYPE_RAW, $1, NULL));
+	ABORT_ON(parse_events_add_numeric(data, list, PERF_TYPE_RAW, $1, NULL));
 	$$ = list;
 }
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/9] perf tools: Report event parsing errors
  2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
                   ` (8 preceding siblings ...)
  2015-04-22 19:10 ` [PATCH 9/9] perf tools: Add symbolic events " Jiri Olsa
@ 2015-04-23 14:44 ` Arnaldo Carvalho de Melo
  9 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-04-23 14:44 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
	Ingo Molnar

Em Wed, Apr 22, 2015 at 09:10:15PM +0200, Jiri Olsa escreveu:
> hi,
> adding support to report error from event string parsing.
> 
> v1 changes (from RFC):
>   - display list of allowed terms for pmu event error [Ingo]
>   - changing 'invalid or unsupported event' string into
>     'event syntax error' for cases we know the precise error
> 
> This patchset contains support for standard parsing errors
> and more logic to recognize tracepoint and 'pmu//' terms,
> like:
> 
>   $ sudo perf record -e 'sched:krava' ls
>   event syntax error: 'sched:krava'
>                        \___ unknown tracepoint
>   ...

Thanks, applied and tested each of these patches.

- Arnaldo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Add parse_events_error interface
  2015-04-22 19:10 ` [PATCH 1/9] perf tools: Add parse_events_error interface Jiri Olsa
@ 2015-05-06  3:04   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, linux-kernel, dsahern, tglx, a.p.zijlstra, hpa, paulus,
	mingo, namhyung, acme

Commit-ID:  b39b839309ce8c5dd15cd95d26af153fa392c3e6
Gitweb:     http://git.kernel.org/tip/b39b839309ce8c5dd15cd95d26af153fa392c3e6
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:16 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:37:58 -0300

perf tools: Add parse_events_error interface

Adding support to return error information from parse_events function.
Following struct will be populated by parse_events function on return:

  struct parse_events_error {
    int   idx;
    char *str;
    char *help;
  };

where 'idx' is the position in the string where the parsing failed,
'str' contains dynamically allocated error string describing the error
and 'help' is optional help string.

The change contains reporting function, which currently does not display
anything. The code changes to supply error data for specific event types
are coming in next patches. However this is what the expected output is:

  $ sudo perf record -e 'sched:krava' ls
  event syntax error: 'sched:krava'
                       \___ unknown tracepoint
  ...

  $ perf record -e 'cpu/even=0x1/' ls
  event syntax error: 'cpu/even=0x1/'
                           \___ unknown term

  valid terms: pc,any,inv,edge,cmask,event,in_tx,ldlat,umask,in_tx_cp,offcore_rsp,config,config1,config2,name,period,branch_type
  ...

  $ perf record -e cycles,cache-mises ls
  event syntax error: '..es,cache-mises'
                                 \___ parser error
  ...

The output functions cut the beginning of the event string so the error
starts up to 10th character and cut the end of the string of it crosses
the terminal width.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-2-git-send-email-jolsa@kernel.org
[ Renamed 'error' variables to 'err', not to clash with util.h error() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c               |   2 +-
 tools/perf/tests/code-reading.c         |   2 +-
 tools/perf/tests/evsel-roundtrip-name.c |   4 +-
 tools/perf/tests/hists_cumulate.c       |   2 +-
 tools/perf/tests/hists_filter.c         |   4 +-
 tools/perf/tests/hists_link.c           |   4 +-
 tools/perf/tests/hists_output.c         |   2 +-
 tools/perf/tests/keep-tracking.c        |   4 +-
 tools/perf/tests/parse-events.c         |   2 +-
 tools/perf/tests/perf-time-to-tsc.c     |   2 +-
 tools/perf/tests/switch-tracking.c      |   8 +--
 tools/perf/util/parse-events.c          | 100 +++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.h          |  19 ++++--
 tools/perf/util/record.c                |   4 +-
 14 files changed, 127 insertions(+), 32 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f7b8218..3dbd8c5 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1541,7 +1541,7 @@ static int setup_events(const char * const *attrs, unsigned len)
 	unsigned i;
 
 	for (i = 0; i < len; i++) {
-		if (parse_events(evsel_list, attrs[i]))
+		if (parse_events(evsel_list, attrs[i], NULL))
 			return -1;
 	}
 	return 0;
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index f671ec3..ca0e480 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -482,7 +482,7 @@ static int do_test_code_reading(bool try_kcore)
 		else
 			str = "cycles";
 		pr_debug("Parsing event '%s'\n", str);
-		ret = parse_events(evlist, str);
+		ret = parse_events(evlist, str, NULL);
 		if (ret < 0) {
 			pr_debug("parse_events failed\n");
 			goto out_err;
diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
index b8d8341..3fa7159 100644
--- a/tools/perf/tests/evsel-roundtrip-name.c
+++ b/tools/perf/tests/evsel-roundtrip-name.c
@@ -23,7 +23,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
 			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
 				__perf_evsel__hw_cache_type_op_res_name(type, op, i,
 									name, sizeof(name));
-				err = parse_events(evlist, name);
+				err = parse_events(evlist, name, NULL);
 				if (err)
 					ret = err;
 			}
@@ -71,7 +71,7 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names)
                 return -ENOMEM;
 
 	for (i = 0; i < nr_names; ++i) {
-		err = parse_events(evlist, names[i]);
+		err = parse_events(evlist, names[i], NULL);
 		if (err) {
 			pr_debug("failed to parse event '%s', err %d\n",
 				 names[i], err);
diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index 1861996..b08a95a 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -695,7 +695,7 @@ int test__hists_cumulate(void)
 
 	TEST_ASSERT_VAL("No memory", evlist);
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 59e53db..108488c 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -108,10 +108,10 @@ int test__hists_filter(void)
 
 	TEST_ASSERT_VAL("No memory", evlist);
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
-	err = parse_events(evlist, "task-clock");
+	err = parse_events(evlist, "task-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 278ba834..34c61e4 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -282,10 +282,10 @@ int test__hists_link(void)
 	if (evlist == NULL)
                 return -ENOMEM;
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
-	err = parse_events(evlist, "task-clock");
+	err = parse_events(evlist, "task-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index b52c9fa..d8a23db 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -590,7 +590,7 @@ int test__hists_output(void)
 
 	TEST_ASSERT_VAL("No memory", evlist);
 
-	err = parse_events(evlist, "cpu-clock");
+	err = parse_events(evlist, "cpu-clock", NULL);
 	if (err)
 		goto out;
 
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 7a5ab7b..5b171d1 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -78,8 +78,8 @@ int test__keep_tracking(void)
 
 	perf_evlist__set_maps(evlist, cpus, threads);
 
-	CHECK__(parse_events(evlist, "dummy:u"));
-	CHECK__(parse_events(evlist, "cycles:u"));
+	CHECK__(parse_events(evlist, "dummy:u", NULL));
+	CHECK__(parse_events(evlist, "cycles:u", NULL));
 
 	perf_evlist__config(evlist, &opts);
 
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 3de7449..82d2a16 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1571,7 +1571,7 @@ static int test_event(struct evlist_test *e)
 	if (evlist == NULL)
 		return -ENOMEM;
 
-	ret = parse_events(evlist, e->name);
+	ret = parse_events(evlist, e->name, NULL);
 	if (ret) {
 		pr_debug("failed to parse event '%s', err %d\n",
 			 e->name, ret);
diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
index f238442..5f49484 100644
--- a/tools/perf/tests/perf-time-to-tsc.c
+++ b/tools/perf/tests/perf-time-to-tsc.c
@@ -68,7 +68,7 @@ int test__perf_time_to_tsc(void)
 
 	perf_evlist__set_maps(evlist, cpus, threads);
 
-	CHECK__(parse_events(evlist, "cycles:u"));
+	CHECK__(parse_events(evlist, "cycles:u", NULL));
 
 	perf_evlist__config(evlist, &opts);
 
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index cc68648..0d31403 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -347,7 +347,7 @@ int test__switch_tracking(void)
 	perf_evlist__set_maps(evlist, cpus, threads);
 
 	/* First event */
-	err = parse_events(evlist, "cpu-clock:u");
+	err = parse_events(evlist, "cpu-clock:u", NULL);
 	if (err) {
 		pr_debug("Failed to parse event dummy:u\n");
 		goto out_err;
@@ -356,7 +356,7 @@ int test__switch_tracking(void)
 	cpu_clocks_evsel = perf_evlist__last(evlist);
 
 	/* Second event */
-	err = parse_events(evlist, "cycles:u");
+	err = parse_events(evlist, "cycles:u", NULL);
 	if (err) {
 		pr_debug("Failed to parse event cycles:u\n");
 		goto out_err;
@@ -371,7 +371,7 @@ int test__switch_tracking(void)
 		goto out;
 	}
 
-	err = parse_events(evlist, sched_switch);
+	err = parse_events(evlist, sched_switch, NULL);
 	if (err) {
 		pr_debug("Failed to parse event %s\n", sched_switch);
 		goto out_err;
@@ -401,7 +401,7 @@ int test__switch_tracking(void)
 	perf_evsel__set_sample_bit(cycles_evsel, TIME);
 
 	/* Fourth event */
-	err = parse_events(evlist, "dummy:u");
+	err = parse_events(evlist, "dummy:u", NULL);
 	if (err) {
 		pr_debug("Failed to parse event dummy:u\n");
 		goto out_err;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index be06553..6978cc3 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -17,6 +17,7 @@
 #include "parse-events-flex.h"
 #include "pmu.h"
 #include "thread_map.h"
+#include "asm/bug.h"
 
 #define MAX_NAME_LEN 100
 
@@ -1019,11 +1020,13 @@ int parse_events_terms(struct list_head *terms, const char *str)
 	return ret;
 }
 
-int parse_events(struct perf_evlist *evlist, const char *str)
+int parse_events(struct perf_evlist *evlist, const char *str,
+		 struct parse_events_error *err)
 {
 	struct parse_events_evlist data = {
-		.list = LIST_HEAD_INIT(data.list),
-		.idx  = evlist->nr_entries,
+		.list  = LIST_HEAD_INIT(data.list),
+		.idx   = evlist->nr_entries,
+		.error = err,
 	};
 	int ret;
 
@@ -1044,16 +1047,87 @@ int parse_events(struct perf_evlist *evlist, const char *str)
 	return ret;
 }
 
+#define MAX_WIDTH 1000
+static int get_term_width(void)
+{
+	struct winsize ws;
+
+	get_term_dimensions(&ws);
+	return ws.ws_col > MAX_WIDTH ? MAX_WIDTH : ws.ws_col;
+}
+
+static void parse_events_print_error(struct parse_events_error *err,
+				     const char *event)
+{
+	const char *str = "invalid or unsupported event: ";
+	char _buf[MAX_WIDTH];
+	char *buf = (char *) event;
+	int idx = 0;
+
+	if (err->str) {
+		/* -2 for extra '' in the final fprintf */
+		int width       = get_term_width() - 2;
+		int len_event   = strlen(event);
+		int len_str, max_len, cut = 0;
+
+		/*
+		 * Maximum error index indent, we will cut
+		 * the event string if it's bigger.
+		 */
+		int max_err_idx = 10;
+
+		/*
+		 * Let's be specific with the message when
+		 * we have the precise error.
+		 */
+		str     = "event syntax error: ";
+		len_str = strlen(str);
+		max_len = width - len_str;
+
+		buf = _buf;
+
+		/* We're cutting from the beggining. */
+		if (err->idx > max_err_idx)
+			cut = err->idx - max_err_idx;
+
+		strncpy(buf, event + cut, max_len);
+
+		/* Mark cut parts with '..' on both sides. */
+		if (cut)
+			buf[0] = buf[1] = '.';
+
+		if ((len_event - cut) > max_len) {
+			buf[max_len - 1] = buf[max_len - 2] = '.';
+			buf[max_len] = 0;
+		}
+
+		idx = len_str + err->idx - cut;
+	}
+
+	fprintf(stderr, "%s'%s'\n", str, buf);
+	if (idx) {
+		fprintf(stderr, "%*s\\___ %s\n", idx + 1, "", err->str);
+		if (err->help)
+			fprintf(stderr, "\n%s\n", err->help);
+		free(err->str);
+		free(err->help);
+	}
+
+	fprintf(stderr, "Run 'perf list' for a list of valid events\n");
+}
+
+#undef MAX_WIDTH
+
 int parse_events_option(const struct option *opt, const char *str,
 			int unset __maybe_unused)
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
-	int ret = parse_events(evlist, str);
+	struct parse_events_error err = { .idx = 0, };
+	int ret = parse_events(evlist, str, &err);
+
+	if (ret)
+		parse_events_print_error(&err, str);
 
-	if (ret) {
-		fprintf(stderr, "invalid or unsupported event: '%s'\n", str);
-		fprintf(stderr, "Run 'perf list' for a list of valid events\n");
-	}
 	return ret;
 }
 
@@ -1535,3 +1609,13 @@ void parse_events__free_terms(struct list_head *terms)
 	list_for_each_entry_safe(term, h, terms, list)
 		free(term);
 }
+
+void parse_events_evlist_error(struct parse_events_evlist *data,
+			       int idx, const char *str)
+{
+	struct parse_events_error *err = data->error;
+
+	err->idx = idx;
+	err->str = strdup(str);
+	WARN_ONCE(!err->str, "WARNING: failed to allocate error string");
+}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 52a2dda..5ac2ffa0 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -12,6 +12,7 @@
 struct list_head;
 struct perf_evsel;
 struct perf_evlist;
+struct parse_events_error;
 
 struct option;
 
@@ -29,7 +30,8 @@ const char *event_type(int type);
 
 extern int parse_events_option(const struct option *opt, const char *str,
 			       int unset);
-extern int parse_events(struct perf_evlist *evlist, const char *str);
+extern int parse_events(struct perf_evlist *evlist, const char *str,
+			struct parse_events_error *error);
 extern int parse_events_terms(struct list_head *terms, const char *str);
 extern int parse_filter(const struct option *opt, const char *str, int unset);
 
@@ -74,10 +76,17 @@ struct parse_events_term {
 	bool used;
 };
 
+struct parse_events_error {
+	int   idx;	/* index in the parsed string */
+	char *str;      /* string to display at the index */
+	char *help;	/* optional help string */
+};
+
 struct parse_events_evlist {
-	struct list_head list;
-	int idx;
-	int nr_groups;
+	struct list_head	   list;
+	int			   idx;
+	int			   nr_groups;
+	struct parse_events_error *error;
 };
 
 struct parse_events_terms {
@@ -114,6 +123,8 @@ void parse_events__set_leader(char *name, struct list_head *list);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
 void parse_events_error(void *data, void *scanner, char const *msg);
+void parse_events_evlist_error(struct parse_events_evlist *data,
+			       int idx, const char *str);
 
 void print_events(const char *event_glob, bool name_only);
 
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index 0ccfa49..d457c52 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -20,7 +20,7 @@ static int perf_do_probe_api(setup_probe_fn_t fn, int cpu, const char *str)
 	if (!evlist)
 		return -ENOMEM;
 
-	if (parse_events(evlist, str))
+	if (parse_events(evlist, str, NULL))
 		goto out_delete;
 
 	evsel = perf_evlist__first(evlist);
@@ -216,7 +216,7 @@ bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str)
 	if (!temp_evlist)
 		return false;
 
-	err = parse_events(temp_evlist, str);
+	err = parse_events(temp_evlist, str, NULL);
 	if (err)
 		goto out_delete;
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Add flex support for parse_events_error
  2015-04-22 19:10 ` [PATCH 2/9] perf tools: Add flex support for parse_events_error Jiri Olsa
@ 2015-05-06  3:04   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, linux-kernel, acme, tglx, dsahern, a.p.zijlstra, jolsa,
	hpa, paulus, namhyung

Commit-ID:  6297d42372b6ff02135ce170b0d90ccf0b1531e4
Gitweb:     http://git.kernel.org/tip/6297d42372b6ff02135ce170b0d90ccf0b1531e4
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:17 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:37:59 -0300

perf tools: Add flex support for parse_events_error

Allowing flex parser to report back event parsing error, like:

  $ perf record -e cycles,cache-mises ls
  event syntax error: '..es,cache-mises'
                                 \___ parser error
  ...

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-3-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.h |  1 -
 tools/perf/util/parse-events.l | 37 +++++++++++++++++++++++++++++++++----
 tools/perf/util/parse-events.y |  7 ++++---
 3 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 5ac2ffa0..eb12bcd 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -122,7 +122,6 @@ perf_pmu__parse_check(const char *name);
 void parse_events__set_leader(char *name, struct list_head *list);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
-void parse_events_error(void *data, void *scanner, char const *msg);
 void parse_events_evlist_error(struct parse_events_evlist *data,
 			       int idx, const char *str);
 
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 8895cf3..330dd2d 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -3,6 +3,8 @@
 %option bison-bridge
 %option prefix="parse_events_"
 %option stack
+%option bison-locations
+%option yylineno
 
 %{
 #include <errno.h>
@@ -51,6 +53,18 @@ static int str(yyscan_t scanner, int token)
 	return token;
 }
 
+#define REWIND(__alloc)				\
+do {								\
+	YYSTYPE *__yylval = parse_events_get_lval(yyscanner);	\
+	char *text = parse_events_get_text(yyscanner);		\
+								\
+	if (__alloc)						\
+		__yylval->str = strdup(text);			\
+								\
+	yycolumn -= strlen(text);				\
+	yyless(0);						\
+} while (0)
+
 static int pmu_str_check(yyscan_t scanner)
 {
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
@@ -85,6 +99,13 @@ static int term(yyscan_t scanner, int type)
 	return PE_TERM;
 }
 
+#define YY_USER_ACTION					\
+do {							\
+	yylloc->last_column  = yylloc->first_column;	\
+	yylloc->first_column = yycolumn;		\
+	yycolumn += yyleng;				\
+} while (0);
+
 %}
 
 %x mem
@@ -119,6 +140,12 @@ modifier_bp	[rwx]{1,3}
 
 		if (start_token) {
 			parse_events_set_extra(NULL, yyscanner);
+			/*
+			 * The flex parser does not init locations variable
+			 * via the scan_string interface, so we need do the
+			 * init in here.
+			 */
+			yycolumn = 0;
 			return start_token;
 		}
          }
@@ -127,19 +154,21 @@ modifier_bp	[rwx]{1,3}
 <event>{
 
 {group}		{
-			BEGIN(INITIAL); yyless(0);
+			BEGIN(INITIAL);
+			REWIND(0);
 		}
 
 {event_pmu}	|
 {event}		{
-			str(yyscanner, PE_EVENT_NAME);
-			BEGIN(INITIAL); yyless(0);
+			BEGIN(INITIAL);
+			REWIND(1);
 			return PE_EVENT_NAME;
 		}
 
 .		|
 <<EOF>>		{
-			BEGIN(INITIAL); yyless(0);
+			BEGIN(INITIAL);
+			REWIND(0);
 		}
 
 }
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 72def07..14521ce 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -2,6 +2,7 @@
 %parse-param {void *_data}
 %parse-param {void *scanner}
 %lex-param {void* scanner}
+%locations
 
 %{
 
@@ -14,8 +15,6 @@
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
-extern int parse_events_lex (YYSTYPE* lvalp, void* scanner);
-
 #define ABORT_ON(val) \
 do { \
 	if (val) \
@@ -520,7 +519,9 @@ sep_slash_dc: '/' | ':' |
 
 %%
 
-void parse_events_error(void *data __maybe_unused, void *scanner __maybe_unused,
+void parse_events_error(YYLTYPE *loc, void *data,
+			void *scanner __maybe_unused,
 			char const *msg __maybe_unused)
 {
+	parse_events_evlist_error(data, loc->last_column, "parser error");
 }

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Always bail out when config_attr function fails
  2015-04-22 19:10 ` [PATCH 3/9] perf tools: Always bail out when config_attr function fails Jiri Olsa
@ 2015-05-06  3:04   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, paulus, dsahern, a.p.zijlstra, jolsa, namhyung, mingo, hpa,
	acme, linux-kernel

Commit-ID:  c056ba6a174f4d5d79fe27f259fc133041a451da
Gitweb:     http://git.kernel.org/tip/c056ba6a174f4d5d79fe27f259fc133041a451da
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:18 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:37:59 -0300

perf tools: Always bail out when config_attr function fails

Not sure why we allowed the fail state, but it's wrong.  Wrong type for
'name' term can cause segfault, and there's probably more fun hidden.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-4-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6978cc3..1e42f2c 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -549,6 +549,12 @@ do {								\
 } while (0)
 
 	switch (term->type_term) {
+	case PARSE_EVENTS__TERM_TYPE_USER:
+		/*
+		 * Always succeed for sysfs terms, as we dont know
+		 * at this point what type they need to have.
+		 */
+		return 0;
 	case PARSE_EVENTS__TERM_TYPE_CONFIG:
 		CHECK_TYPE_VAL(NUM);
 		attr->config = term->val.num;
@@ -583,12 +589,12 @@ do {								\
 }
 
 static int config_attr(struct perf_event_attr *attr,
-		       struct list_head *head, int fail)
+		       struct list_head *head)
 {
 	struct parse_events_term *term;
 
 	list_for_each_entry(term, head, list)
-		if (config_term(attr, term) && fail)
+		if (config_term(attr, term))
 			return -EINVAL;
 
 	return 0;
@@ -605,7 +611,7 @@ int parse_events_add_numeric(struct list_head *list, int *idx,
 	attr.config = config;
 
 	if (head_config &&
-	    config_attr(&attr, head_config, 1))
+	    config_attr(&attr, head_config))
 		return -EINVAL;
 
 	return add_event(list, idx, &attr, NULL);
@@ -659,7 +665,8 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
 	 * Configure hardcoded terms first, no need to check
 	 * return value when called with fail == 0 ;)
 	 */
-	config_attr(&attr, head_config, 0);
+	if (config_attr(&attr, head_config))
+		return -EINVAL;
 
 	if (perf_pmu__config(pmu, &attr, head_config))
 		return -EINVAL;

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Change parse_events_add_pmu interface
  2015-04-22 19:10 ` [PATCH 4/9] perf tools: Change parse_events_add_pmu interface Jiri Olsa
@ 2015-05-06  3:05   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, dsahern, a.p.zijlstra, acme, linux-kernel, paulus, tglx,
	mingo, namhyung, hpa

Commit-ID:  36adec85a86f2daa521cda48ea7be8a95c20ed10
Gitweb:     http://git.kernel.org/tip/36adec85a86f2daa521cda48ea7be8a95c20ed10
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:19 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:38:00 -0300

perf tools: Change parse_events_add_pmu interface

Changing parse_events_add_pmu interface to allow propagating of the
parse_events_error info.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-5-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 11 ++++++-----
 tools/perf/util/parse-events.h |  5 +++--
 tools/perf/util/parse-events.y |  6 +++---
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e42f2c..749af0d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -633,8 +633,9 @@ static char *pmu_event_name(struct list_head *head_terms)
 	return NULL;
 }
 
-int parse_events_add_pmu(struct list_head *list, int *idx,
-			 char *name, struct list_head *head_config)
+int parse_events_add_pmu(struct parse_events_evlist *data,
+			 struct list_head *list, char *name,
+			 struct list_head *head_config)
 {
 	struct perf_event_attr attr;
 	struct perf_pmu_info info;
@@ -654,7 +655,7 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
 
 	if (!head_config) {
 		attr.type = pmu->type;
-		evsel = __add_event(list, idx, &attr, NULL, pmu->cpus);
+		evsel = __add_event(list, &data->idx, &attr, NULL, pmu->cpus);
 		return evsel ? 0 : -ENOMEM;
 	}
 
@@ -671,8 +672,8 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
 	if (perf_pmu__config(pmu, &attr, head_config))
 		return -EINVAL;
 
-	evsel = __add_event(list, idx, &attr, pmu_event_name(head_config),
-			    pmu->cpus);
+	evsel = __add_event(list, &data->idx, &attr,
+			    pmu_event_name(head_config), pmu->cpus);
 	if (evsel) {
 		evsel->unit = info.unit;
 		evsel->scale = info.scale;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index eb12bcd..76ea3de 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -115,8 +115,9 @@ int parse_events_add_cache(struct list_head *list, int *idx,
 			   char *type, char *op_result1, char *op_result2);
 int parse_events_add_breakpoint(struct list_head *list, int *idx,
 				void *ptr, char *type, u64 len);
-int parse_events_add_pmu(struct list_head *list, int *idx,
-			 char *pmu , struct list_head *head_config);
+int parse_events_add_pmu(struct parse_events_evlist *data,
+			 struct list_head *list, char *name,
+			 struct list_head *head_config);
 enum perf_pmu_event_symbol_type
 perf_pmu__parse_check(const char *name);
 void parse_events__set_leader(char *name, struct list_head *list);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 14521ce..8459661 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -207,7 +207,7 @@ PE_NAME '/' event_config '/'
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(list, &data->idx, $1, $3));
+	ABORT_ON(parse_events_add_pmu(data, list, $1, $3));
 	parse_events__free_terms($3);
 	$$ = list;
 }
@@ -218,7 +218,7 @@ PE_NAME '/' '/'
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(list, &data->idx, $1, NULL));
+	ABORT_ON(parse_events_add_pmu(data, list, $1, NULL));
 	$$ = list;
 }
 |
@@ -235,7 +235,7 @@ PE_KERNEL_PMU_EVENT sep_dc
 	list_add_tail(&term->list, head);
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(list, &data->idx, "cpu", head));
+	ABORT_ON(parse_events_add_pmu(data, list, "cpu", head));
 	parse_events__free_terms(head);
 	$$ = list;
 }

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Add location to pmu event terms
  2015-04-22 19:10 ` [PATCH 5/9] perf tools: Add location to pmu event terms Jiri Olsa
@ 2015-05-06  3:05   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, mingo, paulus, linux-kernel, namhyung, a.p.zijlstra, jolsa,
	acme, dsahern, hpa

Commit-ID:  cecf3a2e185c1d843428166d644ba3b564231293
Gitweb:     http://git.kernel.org/tip/cecf3a2e185c1d843428166d644ba3b564231293
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:20 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:38:00 -0300

perf tools: Add location to pmu event terms

Saving the terms location within term struct, so it could be used later
for report.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-6-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 32 ++++++++++++++++++++++++--------
 tools/perf/util/parse-events.h |  8 ++++----
 tools/perf/util/parse-events.y | 16 ++++++++--------
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 749af0d..2994cb4 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -25,6 +25,12 @@
 extern int parse_events_debug;
 #endif
 int parse_events_parse(void *data, void *scanner);
+int parse_events_term__num(struct parse_events_term **term,
+			   int type_term, char *config, u64 num,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val);
+int parse_events_term__str(struct parse_events_term **term,
+			   int type_term, char *config, char *str,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val);
 
 static struct perf_pmu_event_symbol *perf_pmu_events_list;
 /*
@@ -1542,7 +1548,7 @@ int parse_events__is_hardcoded_term(struct parse_events_term *term)
 
 static int new_term(struct parse_events_term **_term, int type_val,
 		    int type_term, char *config,
-		    char *str, u64 num)
+		    char *str, u64 num, int err_term, int err_val)
 {
 	struct parse_events_term *term;
 
@@ -1554,6 +1560,8 @@ static int new_term(struct parse_events_term **_term, int type_val,
 	term->type_val  = type_val;
 	term->type_term = type_term;
 	term->config = config;
+	term->err_term = err_term;
+	term->err_val  = err_val;
 
 	switch (type_val) {
 	case PARSE_EVENTS__TERM_TYPE_NUM:
@@ -1572,17 +1580,23 @@ static int new_term(struct parse_events_term **_term, int type_val,
 }
 
 int parse_events_term__num(struct parse_events_term **term,
-			   int type_term, char *config, u64 num)
+			   int type_term, char *config, u64 num,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val)
 {
 	return new_term(term, PARSE_EVENTS__TERM_TYPE_NUM, type_term,
-			config, NULL, num);
+			config, NULL, num,
+			loc_term ? loc_term->first_column : 0,
+			loc_val ? loc_val->first_column : 0);
 }
 
 int parse_events_term__str(struct parse_events_term **term,
-			   int type_term, char *config, char *str)
+			   int type_term, char *config, char *str,
+			   YYLTYPE *loc_term, YYLTYPE *loc_val)
 {
 	return new_term(term, PARSE_EVENTS__TERM_TYPE_STR, type_term,
-			config, str, 0);
+			config, str, 0,
+			loc_term ? loc_term->first_column : 0,
+			loc_val ? loc_val->first_column : 0);
 }
 
 int parse_events_term__sym_hw(struct parse_events_term **term,
@@ -1596,18 +1610,20 @@ int parse_events_term__sym_hw(struct parse_events_term **term,
 	if (config)
 		return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
 				PARSE_EVENTS__TERM_TYPE_USER, config,
-				(char *) sym->symbol, 0);
+				(char *) sym->symbol, 0, 0, 0);
 	else
 		return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
 				PARSE_EVENTS__TERM_TYPE_USER,
-				(char *) "event", (char *) sym->symbol, 0);
+				(char *) "event", (char *) sym->symbol,
+				0, 0, 0);
 }
 
 int parse_events_term__clone(struct parse_events_term **new,
 			     struct parse_events_term *term)
 {
 	return new_term(new, term->type_val, term->type_term, term->config,
-			term->val.str, term->val.num);
+			term->val.str, term->val.num,
+			term->err_term, term->err_val);
 }
 
 void parse_events__free_terms(struct list_head *terms)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 76ea3de..6286ffd 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -74,6 +74,10 @@ struct parse_events_term {
 	int type_term;
 	struct list_head list;
 	bool used;
+
+	/* error string indexes for within parsed string */
+	int err_term;
+	int err_val;
 };
 
 struct parse_events_error {
@@ -94,10 +98,6 @@ struct parse_events_terms {
 };
 
 int parse_events__is_hardcoded_term(struct parse_events_term *term);
-int parse_events_term__num(struct parse_events_term **_term,
-			   int type_term, char *config, u64 num);
-int parse_events_term__str(struct parse_events_term **_term,
-			   int type_term, char *config, char *str);
 int parse_events_term__sym_hw(struct parse_events_term **term,
 			      char *config, unsigned idx);
 int parse_events_term__clone(struct parse_events_term **new,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 8459661..4862477 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -231,7 +231,7 @@ PE_KERNEL_PMU_EVENT sep_dc
 
 	ALLOC_LIST(head);
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, 1));
+					$1, 1, &@1, NULL));
 	list_add_tail(&term->list, head);
 
 	ALLOC_LIST(list);
@@ -251,7 +251,7 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
 
 	ALLOC_LIST(head);
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					&pmu_name, 1));
+					&pmu_name, 1, &@1, NULL));
 	list_add_tail(&term->list, head);
 
 	ALLOC_LIST(list);
@@ -449,7 +449,7 @@ PE_NAME '=' PE_NAME
 	struct parse_events_term *term;
 
 	ABORT_ON(parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, $3));
+					$1, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -458,7 +458,7 @@ PE_NAME '=' PE_VALUE
 	struct parse_events_term *term;
 
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, $3));
+					$1, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -476,7 +476,7 @@ PE_NAME
 	struct parse_events_term *term;
 
 	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, 1));
+					$1, 1, &@1, NULL));
 	$$ = term;
 }
 |
@@ -493,7 +493,7 @@ PE_TERM '=' PE_NAME
 {
 	struct parse_events_term *term;
 
-	ABORT_ON(parse_events_term__str(&term, (int)$1, NULL, $3));
+	ABORT_ON(parse_events_term__str(&term, (int)$1, NULL, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -501,7 +501,7 @@ PE_TERM '=' PE_VALUE
 {
 	struct parse_events_term *term;
 
-	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3));
+	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, &@1, &@3));
 	$$ = term;
 }
 |
@@ -509,7 +509,7 @@ PE_TERM
 {
 	struct parse_events_term *term;
 
-	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1));
+	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, &@1, NULL));
 	$$ = term;
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Add term support for parse_events_error
  2015-04-22 19:10 ` [PATCH 6/9] perf tools: Add term support for parse_events_error Jiri Olsa
@ 2015-05-06  3:05   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, a.p.zijlstra, hpa, acme, tglx, dsahern, namhyung, jolsa,
	linux-kernel, paulus

Commit-ID:  e64b020ba1adfd081a26c5a35a2990f91da043a0
Gitweb:     http://git.kernel.org/tip/e64b020ba1adfd081a26c5a35a2990f91da043a0
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:21 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:38:01 -0300

perf tools: Add term support for parse_events_error

Allowing event's term processing to report back error, like:

  $ perf record -e 'cpu/even=0x1/' ls
  event syntax error: 'cpu/even=0x1/'
                           \___ unknown term

  valid terms: pc,any,inv,edge,cmask,event,in_tx,ldlat,umask,in_tx_cp,offcore_rsp,config,config1,config2,name,period,branch_type

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-7-git-send-email-jolsa@kernel.org
[ Renamed 'error' variables to 'err', not to clash with util.h error() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/pmu.c         |  3 ++-
 tools/perf/util/parse-events.c |  2 +-
 tools/perf/util/parse-events.l |  4 +++
 tools/perf/util/pmu.c          | 57 +++++++++++++++++++++++++++++++++++++-----
 tools/perf/util/pmu.h          |  6 +++--
 5 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index eeb68bb1..faa04e9 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -152,7 +152,8 @@ int test__pmu(void)
 		if (ret)
 			break;
 
-		ret = perf_pmu__config_terms(&formats, &attr, terms, false);
+		ret = perf_pmu__config_terms(&formats, &attr, terms,
+					     false, NULL);
 		if (ret)
 			break;
 
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 2994cb4..1d810d1 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -675,7 +675,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 	if (config_attr(&attr, head_config))
 		return -EINVAL;
 
-	if (perf_pmu__config(pmu, &attr, head_config))
+	if (perf_pmu__config(pmu, &attr, head_config, data->error))
 		return -EINVAL;
 
 	evsel = __add_event(list, &data->idx, &attr,
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 330dd2d..09e738f 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -174,6 +174,10 @@ modifier_bp	[rwx]{1,3}
 }
 
 <config>{
+	/*
+	 * Please update formats_error_string any time
+	 * new static term is added.
+	 */
 config			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG); }
 config1			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG1); }
 config2			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG2); }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 4841167..244c66f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -579,6 +579,38 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
 	return -1;
 }
 
+static char *formats_error_string(struct list_head *formats)
+{
+	struct perf_pmu_format *format;
+	char *err, *str;
+	static const char *static_terms = "config,config1,config2,name,period,branch_type\n";
+	unsigned i = 0;
+
+	if (!asprintf(&str, "valid terms:"))
+		return NULL;
+
+	/* sysfs exported terms */
+	list_for_each_entry(format, formats, list) {
+		char c = i++ ? ',' : ' ';
+
+		err = str;
+		if (!asprintf(&str, "%s%c%s", err, c, format->name))
+			goto fail;
+		free(err);
+	}
+
+	/* static terms */
+	err = str;
+	if (!asprintf(&str, "%s,%s", err, static_terms))
+		goto fail;
+
+	free(err);
+	return str;
+fail:
+	free(err);
+	return NULL;
+}
+
 /*
  * Setup one of config[12] attr members based on the
  * user input data - term parameter.
@@ -587,7 +619,7 @@ static int pmu_config_term(struct list_head *formats,
 			   struct perf_event_attr *attr,
 			   struct parse_events_term *term,
 			   struct list_head *head_terms,
-			   bool zero)
+			   bool zero, struct parse_events_error *err)
 {
 	struct perf_pmu_format *format;
 	__u64 *vp;
@@ -611,6 +643,11 @@ static int pmu_config_term(struct list_head *formats,
 	if (!format) {
 		if (verbose)
 			printf("Invalid event/parameter '%s'\n", term->config);
+		if (err) {
+			err->idx  = term->err_term;
+			err->str  = strdup("unknown term");
+			err->help = formats_error_string(formats);
+		}
 		return -EINVAL;
 	}
 
@@ -636,9 +673,14 @@ static int pmu_config_term(struct list_head *formats,
 		val = term->val.num;
 	else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
 		if (strcmp(term->val.str, "?")) {
-			if (verbose)
+			if (verbose) {
 				pr_info("Invalid sysfs entry %s=%s\n",
 						term->config, term->val.str);
+			}
+			if (err) {
+				err->idx = term->err_val;
+				err->str = strdup("expected numeric value");
+			}
 			return -EINVAL;
 		}
 
@@ -654,12 +696,13 @@ static int pmu_config_term(struct list_head *formats,
 int perf_pmu__config_terms(struct list_head *formats,
 			   struct perf_event_attr *attr,
 			   struct list_head *head_terms,
-			   bool zero)
+			   bool zero, struct parse_events_error *err)
 {
 	struct parse_events_term *term;
 
 	list_for_each_entry(term, head_terms, list) {
-		if (pmu_config_term(formats, attr, term, head_terms, zero))
+		if (pmu_config_term(formats, attr, term, head_terms,
+				    zero, err))
 			return -EINVAL;
 	}
 
@@ -672,12 +715,14 @@ int perf_pmu__config_terms(struct list_head *formats,
  * 2) pmu format definitions - specified by pmu parameter
  */
 int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
-		     struct list_head *head_terms)
+		     struct list_head *head_terms,
+		     struct parse_events_error *err)
 {
 	bool zero = !!pmu->default_config;
 
 	attr->type = pmu->type;
-	return perf_pmu__config_terms(&pmu->format, attr, head_terms, zero);
+	return perf_pmu__config_terms(&pmu->format, attr, head_terms,
+				      zero, err);
 }
 
 static struct perf_pmu_alias *pmu_find_alias(struct perf_pmu *pmu,
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 6b1249f..7b9c8cf 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -4,6 +4,7 @@
 #include <linux/bitmap.h>
 #include <linux/perf_event.h>
 #include <stdbool.h>
+#include "parse-events.h"
 
 enum {
 	PERF_PMU_FORMAT_VALUE_CONFIG,
@@ -47,11 +48,12 @@ struct perf_pmu_alias {
 
 struct perf_pmu *perf_pmu__find(const char *name);
 int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
-		     struct list_head *head_terms);
+		     struct list_head *head_terms,
+		     struct parse_events_error *error);
 int perf_pmu__config_terms(struct list_head *formats,
 			   struct perf_event_attr *attr,
 			   struct list_head *head_terms,
-			   bool zero);
+			   bool zero, struct parse_events_error *error);
 int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
 			  struct perf_pmu_info *info);
 struct list_head *perf_pmu__alias(struct perf_pmu *pmu,

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Add static terms support for parse_events_error
  2015-04-22 19:10 ` [PATCH 7/9] perf tools: Add static terms " Jiri Olsa
@ 2015-05-06  3:05   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, linux-kernel, mingo, acme, dsahern, tglx, a.p.zijlstra,
	namhyung, jolsa, paulus

Commit-ID:  3b0e371cc05dfb624f990ea5e1da2bff615adaef
Gitweb:     http://git.kernel.org/tip/3b0e371cc05dfb624f990ea5e1da2bff615adaef
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:22 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:38:01 -0300

perf tools: Add static terms support for parse_events_error

Allowing static terms like 'name,period,config,config1..' processing to
report back error.

  $ perf record -e 'cpu/event=1,name=1/' ls
  event syntax error: '..=1,name=1/'
                                 \___ expected string value

  $ perf record -e 'cpu/event=1,period=krava/' ls
  event syntax error: '..,period=krava/'
                                 \___ expected numeric value

  $ perf record -e 'cpu/config=krava1/' ls
  event syntax error: '../config=krava1/'
                                 \___ expected numeric value

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-8-git-send-email-jolsa@kernel.org
[ Renamed 'error' variables to 'err', not to clash with util.h error() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1d810d1..278aebe 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -545,13 +545,31 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 	return add_event(list, idx, &attr, NULL);
 }
 
+static int check_type_val(struct parse_events_term *term,
+			  struct parse_events_error *err,
+			  int type)
+{
+	if (type == term->type_val)
+		return 0;
+
+	if (err) {
+		err->idx = term->err_val;
+		if (type == PARSE_EVENTS__TERM_TYPE_NUM)
+			err->str = strdup("expected numeric value");
+		else
+			err->str = strdup("expected string value");
+	}
+	return -EINVAL;
+}
+
 static int config_term(struct perf_event_attr *attr,
-		       struct parse_events_term *term)
+		       struct parse_events_term *term,
+		       struct parse_events_error *err)
 {
-#define CHECK_TYPE_VAL(type)					\
-do {								\
-	if (PARSE_EVENTS__TERM_TYPE_ ## type != term->type_val)	\
-		return -EINVAL;					\
+#define CHECK_TYPE_VAL(type)						   \
+do {									   \
+	if (check_type_val(term, err, PARSE_EVENTS__TERM_TYPE_ ## type)) \
+		return -EINVAL;						   \
 } while (0)
 
 	switch (term->type_term) {
@@ -595,12 +613,13 @@ do {								\
 }
 
 static int config_attr(struct perf_event_attr *attr,
-		       struct list_head *head)
+		       struct list_head *head,
+		       struct parse_events_error *err)
 {
 	struct parse_events_term *term;
 
 	list_for_each_entry(term, head, list)
-		if (config_term(attr, term))
+		if (config_term(attr, term, err))
 			return -EINVAL;
 
 	return 0;
@@ -617,7 +636,7 @@ int parse_events_add_numeric(struct list_head *list, int *idx,
 	attr.config = config;
 
 	if (head_config &&
-	    config_attr(&attr, head_config))
+	    config_attr(&attr, head_config, NULL))
 		return -EINVAL;
 
 	return add_event(list, idx, &attr, NULL);
@@ -672,7 +691,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 	 * Configure hardcoded terms first, no need to check
 	 * return value when called with fail == 0 ;)
 	 */
-	if (config_attr(&attr, head_config))
+	if (config_attr(&attr, head_config, data->error))
 		return -EINVAL;
 
 	if (perf_pmu__config(pmu, &attr, head_config, data->error))

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Add tracepoint support for parse_events_error
  2015-04-22 19:10 ` [PATCH 8/9] perf tools: Add tracepoint " Jiri Olsa
@ 2015-05-06  3:06   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:06 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, dsahern, jolsa, hpa, a.p.zijlstra, namhyung, linux-kernel,
	mingo, tglx, paulus

Commit-ID:  492d977444734e03c0633a238f1431b3c66b3e97
Gitweb:     http://git.kernel.org/tip/492d977444734e03c0633a238f1431b3c66b3e97
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:23 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:38:01 -0300

perf tools: Add tracepoint support for parse_events_error

Allowing tracepoint events processing to report back error.

  $ perf record -e 'sched:krava' ls
  event syntax error: 'sched:krava'
                       \___ unknown tracepoint
  ...

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-9-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.y | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 4862477..38a0f21 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -388,7 +388,13 @@ PE_NAME ':' PE_NAME
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_tracepoint(list, &data->idx, $1, $3));
+	if (parse_events_add_tracepoint(list, &data->idx, $1, $3)) {
+		struct parse_events_error *error = data->error;
+
+		error->idx = @1.first_column;
+		error->str = strdup("unknown tracepoint");
+		return -1;
+	}
 	$$ = list;
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [tip:perf/core] perf tools: Add symbolic events support for parse_events_error
  2015-04-22 19:10 ` [PATCH 9/9] perf tools: Add symbolic events " Jiri Olsa
@ 2015-05-06  3:06   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-05-06  3:06 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, hpa, paulus, tglx, linux-kernel, mingo, jolsa, dsahern,
	a.p.zijlstra, namhyung

Commit-ID:  87d650be1dcc9bd9bb200e73b985ddb740d067bc
Gitweb:     http://git.kernel.org/tip/87d650be1dcc9bd9bb200e73b985ddb740d067bc
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 22 Apr 2015 21:10:24 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 29 Apr 2015 10:38:02 -0300

perf tools: Add symbolic events support for parse_events_error

Allowing symbolic events processing to report back error.

  $ perf record -e 'cycles/period=krava/' ls
  event syntax error: '../period=krava/'
                                 \___ expected numeric value

  $ perf record -e 'cycles/name=1/' ls
  event syntax error: '..es/name=1/'
                                 \___ expected string value

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1429729824-13932-10-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c |  7 ++++---
 tools/perf/util/parse-events.h |  3 ++-
 tools/perf/util/parse-events.y | 11 ++++-------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 278aebe..80a50fd 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -625,7 +625,8 @@ static int config_attr(struct perf_event_attr *attr,
 	return 0;
 }
 
-int parse_events_add_numeric(struct list_head *list, int *idx,
+int parse_events_add_numeric(struct parse_events_evlist *data,
+			     struct list_head *list,
 			     u32 type, u64 config,
 			     struct list_head *head_config)
 {
@@ -636,10 +637,10 @@ int parse_events_add_numeric(struct list_head *list, int *idx,
 	attr.config = config;
 
 	if (head_config &&
-	    config_attr(&attr, head_config, NULL))
+	    config_attr(&attr, head_config, data->error))
 		return -EINVAL;
 
-	return add_event(list, idx, &attr, NULL);
+	return add_event(list, &data->idx, &attr, NULL);
 }
 
 static int parse_events__is_name_term(struct parse_events_term *term)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 6286ffd..e236f1b 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -108,7 +108,8 @@ int parse_events__modifier_group(struct list_head *list, char *event_mod);
 int parse_events_name(struct list_head *list, char *name);
 int parse_events_add_tracepoint(struct list_head *list, int *idx,
 				char *sys, char *event);
-int parse_events_add_numeric(struct list_head *list, int *idx,
+int parse_events_add_numeric(struct parse_events_evlist *data,
+			     struct list_head *list,
 			     u32 type, u64 config,
 			     struct list_head *head_config);
 int parse_events_add_cache(struct list_head *list, int *idx,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 38a0f21..3d11e00 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -274,8 +274,7 @@ value_sym '/' event_config '/'
 	int config = $1 & 255;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx,
-					  type, config, $3));
+	ABORT_ON(parse_events_add_numeric(data, list, type, config, $3));
 	parse_events__free_terms($3);
 	$$ = list;
 }
@@ -288,8 +287,7 @@ value_sym sep_slash_dc
 	int config = $1 & 255;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx,
-					  type, config, NULL));
+	ABORT_ON(parse_events_add_numeric(data, list, type, config, NULL));
 	$$ = list;
 }
 
@@ -405,7 +403,7 @@ PE_VALUE ':' PE_VALUE
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx, (u32)$1, $3, NULL));
+	ABORT_ON(parse_events_add_numeric(data, list, (u32)$1, $3, NULL));
 	$$ = list;
 }
 
@@ -416,8 +414,7 @@ PE_RAW
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_numeric(list, &data->idx,
-					  PERF_TYPE_RAW, $1, NULL));
+	ABORT_ON(parse_events_add_numeric(data, list, PERF_TYPE_RAW, $1, NULL));
 	$$ = list;
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-05-06  3:06 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-22 19:10 [PATCH 0/9] perf tools: Report event parsing errors Jiri Olsa
2015-04-22 19:10 ` [PATCH 1/9] perf tools: Add parse_events_error interface Jiri Olsa
2015-05-06  3:04   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 2/9] perf tools: Add flex support for parse_events_error Jiri Olsa
2015-05-06  3:04   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 3/9] perf tools: Always bail out when config_attr function fails Jiri Olsa
2015-05-06  3:04   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 4/9] perf tools: Change parse_events_add_pmu interface Jiri Olsa
2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 5/9] perf tools: Add location to pmu event terms Jiri Olsa
2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 6/9] perf tools: Add term support for parse_events_error Jiri Olsa
2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 7/9] perf tools: Add static terms " Jiri Olsa
2015-05-06  3:05   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 8/9] perf tools: Add tracepoint " Jiri Olsa
2015-05-06  3:06   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-22 19:10 ` [PATCH 9/9] perf tools: Add symbolic events " Jiri Olsa
2015-05-06  3:06   ` [tip:perf/core] " tip-bot for Jiri Olsa
2015-04-23 14:44 ` [PATCH 0/9] perf tools: Report event parsing errors Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).