Prompt proper error message when wrong config terms is specificed for
hw/sw type perf events. Currently, unknown terms is not reported as an
error because pmu events have valid terms in sysfs. But for hw/sw
events, valid config terms are fixed and it should be reported if
wrong terms are given.
Before this patch:
$ perf record -e 'cpu-clock/freq=200/' -a sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.028 MB perf.data (202 samples) ]
$ perf record -e 'cpu-clock/freqx=200/' -a sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.042 MB perf.data (506 samples) ]
After this patch:
$ perf record -e 'cpu-clock/freqx=200/' -a sleep 1
event syntax error: 'cpu-clock/freqx=200/'
\___ unknown term
valid terms:
config,config1,config2,name,period,freq,branch_type,time,call-graph,stack-size
Run 'perf list' for a list of valid events
usage: perf record [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>]
-e, --event <event> event selector. use 'perf list' to list available
events
Signed-off-by: He Kuang <heku...@huawei.com>
---
tools/perf/util/parse-events.c | 60 +++++++++++++++++++++++++++++++++++-------
tools/perf/util/parse-events.h | 1 +
tools/perf/util/pmu.c | 35 +++++++++---------------
3 files changed, 63 insertions(+), 33 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 61c2bc2..f624b99 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -599,7 +599,11 @@ static int check_type_val(struct parse_events_term *term,
return -EINVAL;
}
-static int config_term(struct perf_event_attr *attr,
+typedef int config_term_func_t(struct perf_event_attr *attr,
+ struct parse_events_term *term,
+ struct parse_events_error *err);
+
+static int config_term_common(struct perf_event_attr *attr,
struct parse_events_term *term,
struct parse_events_error *err)
{
@@ -610,12 +614,6 @@ 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;
@@ -658,6 +656,9 @@ do {
\
CHECK_TYPE_VAL(STR);
break;
default:
+ err->str = strdup("unknown term");
+ err->idx = term->err_term;
+ err->help = parse_events_formats_error_string(NULL);
return -EINVAL;
}
@@ -665,9 +666,24 @@ do {
\
#undef CHECK_TYPE_VAL
}
+static int config_term_pmu(struct perf_event_attr *attr,
+ struct parse_events_term *term,
+ struct parse_events_error *err)
+{
+ if (term->type_term == 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;
+ else
+ return config_term_common(attr, term, err);
+}
+
static int config_attr(struct perf_event_attr *attr,
struct list_head *head,
- struct parse_events_error *err)
+ struct parse_events_error *err,
+ config_term_func_t config_term)
{
struct parse_events_term *term;
@@ -735,7 +751,8 @@ int parse_events_add_numeric(struct parse_events_evlist
*data,
attr.config = config;
if (head_config) {
- if (config_attr(&attr, head_config, data->error))
+ if (config_attr(&attr, head_config, data->error,
+ config_term_common))
return -EINVAL;
if (get_config_terms(head_config, &config_terms))
@@ -795,7 +812,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, data->error))
+ if (config_attr(&attr, head_config, data->error, config_term_pmu))
return -EINVAL;
if (get_config_terms(head_config, &config_terms))
@@ -1861,3 +1878,26 @@ void parse_events_evlist_error(struct
parse_events_evlist *data,
err->str = strdup(str);
WARN_ONCE(!err->str, "WARNING: failed to allocate error string");
}
+
+char *parse_events_formats_error_string(char *additional_terms)
+{
+ char *str;
+ static const char *static_terms = "config,config1,config2,name,"
+ "period,freq,branch_type,time,"
+ "call-graph,stack-size\n";
+
+ /* valid terms */
+ if (additional_terms) {
+ if (!asprintf(&str, "valid terms: %s,%s",
+ additional_terms, static_terms))
+ goto fail;
+ free(additional_terms);
+ } else {
+ if (!asprintf(&str, "valid terms: %s", static_terms))
+ goto fail;
+ }
+ return str;
+
+fail:
+ return NULL;
+}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ffee7ec..c7b904a 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -156,5 +156,6 @@ int print_hwcache_events(const char *event_glob, bool
name_only);
extern int is_valid_tracepoint(const char *event_string);
int valid_event_mount(const char *eventfs);
+char *parse_events_formats_error_string(char *additional_terms);
#endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 89c91a1..ea3dd04 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -626,38 +626,26 @@ static int pmu_resolve_param_term(struct
parse_events_term *term,
return -1;
}
-static char *formats_error_string(struct list_head *formats)
+static char *pmu_formats_string(struct list_head *formats)
{
struct perf_pmu_format *format;
- char *err, *str;
- static const char *static_terms = "config,config1,config2,name,"
- "period,freq,branch_type,time,"
- "call-graph,stack-size\n";
+ char *str;
+ struct strbuf buf;
unsigned i = 0;
- if (!asprintf(&str, "valid terms:"))
+ if (!formats)
return NULL;
+ strbuf_init(&buf, 0);
/* 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);
- }
+ list_for_each_entry(format, formats, list)
+ strbuf_addf(&buf, i++ ? ",%s" : "%s",
+ format->name);
- /* static terms */
- err = str;
- if (!asprintf(&str, "%s,%s", err, static_terms))
- goto fail;
+ str = strbuf_detach(&buf, NULL);
+ strbuf_release(&buf);
- free(err);
return str;
-fail:
- free(err);
- return NULL;
}
/*
@@ -695,7 +683,8 @@ static int pmu_config_term(struct list_head *formats,
if (err) {
err->idx = term->err_term;
err->str = strdup("unknown term");
- err->help = formats_error_string(formats);
+ err->help = parse_events_formats_error_string(
+ pmu_formats_string(formats));
}
return -EINVAL;
}
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
