perf parse-events: Improve error message for double setting

Double setting information for an event would produce an error message
associated with the PMU rather than the term that was double setting.
Improve the error message to be on the term.

Before:

  $ perf stat -e 'cpu/inst_retired.any,inst_retired.any/' true
  event syntax error: 'cpu/inst_retired.any,inst_retired.any/'
                       \___ Bad event or PMU

  Unabled to find PMU or event on a PMU of 'cpu'
  Run 'perf list' for a list of valid events
  $

After:

  $ perf stat -e 'cpu/inst_retired.any,inst_retired.any/' true
  event syntax error: '..etired.any,inst_retired.any/'
                                    \___ Bad event or PMU

  Unabled to find PMU or event on a PMU of 'cpu'

  Initial error:

  event syntax error: '..etired.any,inst_retired.any/'
                                    \___ Attempt to set event's scale twice
  Run 'perf list' for a list of valid events

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Gaosheng Cui <cuigaosheng1@huawei.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jing Zhang <renyu.zj@linux.alibaba.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/20230824041330.266337-6-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7cad82a9f5786dde2277dfdeea377c11a675388d..781747bedc3ea848ab2ff38a9886549834b47569 100644 (file)
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1348,7 +1348,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
                return evsel ? 0 : -ENOMEM;
        }
 
-       if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, head_config, &info))
+       if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, head_config, &info, err))
                return -EINVAL;
 
        if (verbose > 1) {

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index d91367155e29522aee73b6b5d337ce885a160e35..0c82bbeea7d6fe3f112fc0a9ef886feae172506d 100644 (file)
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1443,17 +1443,33 @@ static struct perf_pmu_alias *pmu_find_alias(struct perf_pmu *pmu,
 
 
 static int check_info_data(struct perf_pmu_alias *alias,
-                          struct perf_pmu_info *info)
+                          struct perf_pmu_info *info,
+                          struct parse_events_error *err,
+                          int column)
 {
        /*
         * Only one term in event definition can
         * define unit, scale and snapshot, fail
         * if there's more than one.
         */
-       if ((info->unit && alias->unit[0]) ||
-           (info->scale && alias->scale) ||
-           (info->snapshot && alias->snapshot))
+       if (info->unit && alias->unit[0]) {
+               parse_events_error__handle(err, column,
+                                       strdup("Attempt to set event's unit twice"),
+                                       NULL);
+               return -EINVAL;
+       }
+       if (info->scale && alias->scale) {
+               parse_events_error__handle(err, column,
+                                       strdup("Attempt to set event's scale twice"),
+                                       NULL);
+               return -EINVAL;
+       }
+       if (info->snapshot && alias->snapshot) {
+               parse_events_error__handle(err, column,
+                                       strdup("Attempt to set event snapshot twice"),
+                                       NULL);
                return -EINVAL;
+       }
 
        if (alias->unit[0])
                info->unit = alias->unit;
@@ -1472,7 +1488,7 @@ static int check_info_data(struct perf_pmu_alias *alias,
  * defined for the alias
  */
 int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
-                         struct perf_pmu_info *info)
+                         struct perf_pmu_info *info, struct parse_events_error *err)
 {
        struct parse_events_term *term, *h;
        struct perf_pmu_alias *alias;
@@ -1493,10 +1509,14 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
                if (!alias)
                        continue;
                ret = pmu_alias_terms(alias, &term->list);
-               if (ret)
+               if (ret) {
+                       parse_events_error__handle(err, term->err_term,
+                                               strdup("Failure to duplicate terms"),
+                                               NULL);
                        return ret;
+               }
 
-               ret = check_info_data(alias, info);
+               ret = check_info_data(alias, info, err, term->err_term);
                if (ret)
                        return ret;
 

diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index f37e3d75094f52c167c320d0a69ade308182c091..03211de345c18b961db537ef3090b5c0b8ed728b 100644 (file)
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -185,7 +185,7 @@ int perf_pmu__config_terms(struct perf_pmu *pmu,
 __u64 perf_pmu__format_bits(struct perf_pmu *pmu, const char *name);
 int perf_pmu__format_type(struct perf_pmu *pmu, const char *name);
 int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
-                         struct perf_pmu_info *info);
+                         struct perf_pmu_info *info, struct parse_events_error *err);
 int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, pmu_event_callback cb);
 
 int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd, bool eager_load);