[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9dd303cb-0455-d8ac-ce0c-f4a8320b787b@arm.com>
Date: Wed, 31 Jan 2024 11:49:28 +0000
From: James Clark <james.clark@....com>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, tchen168@....edu,
Michael Petlan <mpetlan@...hat.com>
Subject: Re: [PATCH v1 2/2] perf parse-events: Improve error location of terms
cloned from an event
On 31/01/2024 06:30, Ian Rogers wrote:
> A PMU event/alias will have a set of format terms that replace it when
> an event is parsed. The location of the terms is their position when
> parsed for the event/alias either from sysfs or json. This location is
> of little use when an event fails to parse as the error will be given
> in terms of the location in the string of events parsed not the json
> or sysfs string. Fix this by making the cloned terms location that of
> the event/alias.
>
> If a cloned term from an event/alias is invalid the bad format is hard
> to determine from the error string. Add the name of the bad format
> into the error string.
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> These fixes were inspired by the poor error output in:
> https://lore.kernel.org/linux-perf-users/alpine.LRH.2.20.2401300733310.11354@Diego/
> ---
> tools/perf/util/pmu.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 355f813f960d..437386dedd5c 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -657,7 +657,7 @@ static int pmu_aliases_parse(struct perf_pmu *pmu)
> return 0;
> }
>
> -static int pmu_alias_terms(struct perf_pmu_alias *alias, struct list_head *terms)
> +static int pmu_alias_terms(struct perf_pmu_alias *alias, int err_loc, struct list_head *terms)
> {
> struct parse_events_term *term, *cloned;
> struct parse_events_terms clone_terms;
> @@ -675,6 +675,7 @@ static int pmu_alias_terms(struct perf_pmu_alias *alias, struct list_head *terms
> * which we don't want for implicit terms in aliases.
> */
> cloned->weak = true;
> + cloned->err_term = cloned->err_val = err_loc;
> list_add_tail(&cloned->list, &clone_terms.terms);
> }
> list_splice_init(&clone_terms.terms, terms);
> @@ -1363,8 +1364,8 @@ static int pmu_config_term(const struct perf_pmu *pmu,
>
> parse_events_error__handle(err, term->err_val,
> asprintf(&err_str,
> - "value too big for format, maximum is %llu",
> - (unsigned long long)max_val) < 0
> + "value too big for format (%s), maximum is %llu",
> + format->name, (unsigned long long)max_val) < 0
> ? strdup("value too big for format")
> : err_str,
> NULL);
Hi Ian,
I went to test this, but since b30d4f0b6954 ("perf parse-events:
Additional error reporting") I don't get this size error message
anymore, just a "bad event/PMU not found" type error. I'm not sure if
this is something Arm specific, or you're seeing the same thing?
Before b30d4f0b6954:
$ perf record -e bus_access_rd/long=2
event syntax error: '..ss_rd/long=2/'
\___ value too big for format, maximum
is 1
Initial error:
event syntax error: 'bus_access_rd/long=2/'
\___ Cannot find PMU `bus_access_rd'. Missing
kernel support?
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
After b30d4f0b6954:
$ perf record -e bus_access_rd/long=2
event syntax error: '..ss_rd/long=2/'
\___ Bad event or PMU
Unabled to find PMU or event on a PMU of 'bus_access_rd'
Initial error:
event syntax error: 'bus_access_rd/long=2/'
\___ Cannot find PMU `bus_access_rd'. Missing
kernel support?
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
Powered by blists - more mailing lists