[PATCH 1/6] perf tools: Add parse_events_error interface

Jiri Olsa
2015-04-18T17:27:05Z

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;
};

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

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:

$ perf record -e 'sched:krava' ls
invalid or unsupported event: 'sched:krava'
\___ unknown tracepoint

$ ./perf record -e 'cpu/even=0x1/' ls
invalid or unsupported event: 'cpu/even=0x1/'
\___ unknown term

$ perf record -e cycles,cache-mises ls
invalid or unsupported event: '..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 | 90 ++++++++++++++++++++++++++++++---
tools/perf/util/parse-events.h | 18 +++++--
tools/perf/util/record.c | 4 +-
14 files changed, 116 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..87c72dbc2cd5 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,77 @@ 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 = strlen(str);
+ int max_len = width - len_str;
+ int cut = 0;
+ /*
+ * Maximum error index indent, we will cut
+ * the event string if it's bigger.
+ */
+ int max_err_idx = 10;
+
+ 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);
+ free(error->str);
+ }
+
+ 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 +1599,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..90bdd877c62e 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,16 @@ struct parse_events_term {
bool used;
};

+struct parse_events_error {
+ int idx;
+ char *str;
+};
+
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 +122,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 8acd0df88b5c..c7259a5ad185 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);
@@ -207,7 +207,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