From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754982AbbIYDVk (ORCPT ); Thu, 24 Sep 2015 23:21:40 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:40498 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754695AbbIYDVi (ORCPT ); Thu, 24 Sep 2015 23:21:38 -0400 From: He Kuang To: , , , , , CC: , , , Subject: [PATCH 1/2] perf tools: Prompt error message for wrong terms of hw/sw events Date: Fri, 25 Sep 2015 03:21:18 +0000 Message-ID: <1443151279-141093-1-git-send-email-hekuang@huawei.com> X-Mailer: git-send-email 1.8.3.4 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.107.193.250] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020203.5604BDBD.0008,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 238b695d69ffa2c20c72e253f8983d52 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 [] [] or: perf record [] -- [] -e, --event event selector. use 'perf list' to list available events Signed-off-by: He Kuang --- 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