Re: [PATCH 07/10] perf stat: Add event parsing error handling to add_default_attributes
From: Arnaldo Carvalho de Melo
Date: Thu Jun 07 2018 - 15:05:04 EST
Em Thu, Jun 07, 2018 at 12:15:10AM +0200, Jiri Olsa escreveu:
> Adding missing error handling for parse_events calls
> in add_default_attributes functions. The error handler
> displays error details, like for transactions (-T):
Applied up to here, that are trivial, waiting a bit more discussion
about the really new stuff,
Thanks,
- Arnaldo
> Before:
> $ perf stat -T
> Cannot set up transaction events
>
> After:
> $ perf stat -T
> Cannot set up transaction events
> event syntax error: '..cycles,cpu/cycles-t/,cpu/tx-start/,cpu/el-start/,cpu/cycles-ct/}'
> \___ unknown term
>
> Link: http://lkml.kernel.org/n/tip-hmctifkfiaij47xb9en1hlcf@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/perf/builtin-stat.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index a8f93885763a..cc3dd85d5a60 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2451,14 +2451,13 @@ static int add_default_attributes(void)
> (PERF_COUNT_HW_CACHE_OP_PREFETCH << 8) |
> (PERF_COUNT_HW_CACHE_RESULT_MISS << 16) },
> };
> + struct parse_events_error errinfo;
>
> /* Set attrs if no event is selected and !null_run: */
> if (null_run)
> return 0;
>
> if (transaction_run) {
> - struct parse_events_error errinfo;
> -
> if (pmu_have_event("cpu", "cycles-ct") &&
> pmu_have_event("cpu", "el-start"))
> err = parse_events(evsel_list, transaction_attrs,
> @@ -2469,6 +2468,7 @@ static int add_default_attributes(void)
> &errinfo);
> if (err) {
> fprintf(stderr, "Cannot set up transaction events\n");
> + parse_events_print_error(&errinfo, transaction_attrs);
> return -1;
> }
> return 0;
> @@ -2494,10 +2494,11 @@ static int add_default_attributes(void)
> pmu_have_event("msr", "smi")) {
> if (!force_metric_only)
> metric_only = true;
> - err = parse_events(evsel_list, smi_cost_attrs, NULL);
> + err = parse_events(evsel_list, smi_cost_attrs, &errinfo);
> } else {
> fprintf(stderr, "To measure SMI cost, it needs "
> "msr/aperf/, msr/smi/ and cpu/cycles/ support\n");
> + parse_events_print_error(&errinfo, smi_cost_attrs);
> return -1;
> }
> if (err) {
> @@ -2532,12 +2533,13 @@ static int add_default_attributes(void)
> if (topdown_attrs[0] && str) {
> if (warn)
> arch_topdown_group_warn();
> - err = parse_events(evsel_list, str, NULL);
> + err = parse_events(evsel_list, str, &errinfo);
> if (err) {
> fprintf(stderr,
> "Cannot set up top down events %s: %d\n",
> str, err);
> free(str);
> + parse_events_print_error(&errinfo, str);
> return -1;
> }
> } else {
> --
> 2.13.6