* [PATCH] perf tools: Remove evsel__handle_error_quirks()
@ 2025-04-10 1:02 Namhyung Kim
2025-04-10 3:26 ` Chun-Tse Shao
0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2025-04-10 1:02 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Chun-Tse Shao, Ravi Bangoria
The evsel__handle_error_quirks() is to fixup invalid event attributes on
some architecture based on the error code. Currently it's only used for
AMD to disable precise_ip not to use IBS which has more restrictions.
But the commit c33aea446bf555ab changed call evsel__precise_ip_fallback
for any errors so there's no difference with the above function. To
make matter worse, it caused a problem with branch stack on Zen3.
The IBS doesn't support branch stack so it should use a regular core
PMU event. The default event is set precise_max and it starts with 3.
And evsel__precise_ip_fallback() tries with it and reduces the level one
by one. At last it tries with 0 but it also failed on Zen3 since the
branch stack is not supported for the cycles event.
At this point, evsel__precise_ip_fallback() restores the original
precise_ip value (3) in the hope that it can succeed with other modifier
(like exclude_kernel). Then evsel__handle_error_quirks() see it has
precise_ip != 0 and make it retry with 0. This created an infinite
loop.
Before:
$ perf record -b -vv |& grep removing
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
removing precise_ip on AMD
...
After:
$ perf record -b true
Error:
Failure to open event 'cycles:P' on PMU 'cpu' which will be removed.
Invalid event (cycles:P) in per-thread mode, enable system wide with '-a'.
Error:
Failure to open any events for recording.
Fixes: c33aea446bf555ab ("perf tools: Fix precise_ip fallback logic")
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evsel.c | 22 ----------------------
1 file changed, 22 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 1974395492d7da5e..3c030da2e477c707 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2566,25 +2566,6 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
return false;
}
-static bool evsel__handle_error_quirks(struct evsel *evsel, int error)
-{
- /*
- * AMD core PMU tries to forward events with precise_ip to IBS PMU
- * implicitly. But IBS PMU has more restrictions so it can fail with
- * supported event attributes. Let's forward it back to the core PMU
- * by clearing precise_ip only if it's from precise_max (:P).
- */
- if ((error == -EINVAL || error == -ENOENT) && x86__is_amd_cpu() &&
- evsel->core.attr.precise_ip && evsel->precise_max) {
- evsel->core.attr.precise_ip = 0;
- pr_debug2_peo("removing precise_ip on AMD\n");
- display_attr(&evsel->core.attr);
- return true;
- }
-
- return false;
-}
-
static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
struct perf_thread_map *threads,
int start_cpu_map_idx, int end_cpu_map_idx)
@@ -2730,9 +2711,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
if (evsel__precise_ip_fallback(evsel))
goto retry_open;
- if (evsel__handle_error_quirks(evsel, err))
- goto retry_open;
-
out_close:
if (err)
threads->err_thread = thread;
--
2.49.0.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] perf tools: Remove evsel__handle_error_quirks()
2025-04-10 1:02 [PATCH] perf tools: Remove evsel__handle_error_quirks() Namhyung Kim
@ 2025-04-10 3:26 ` Chun-Tse Shao
2025-04-10 16:31 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Chun-Tse Shao @ 2025-04-10 3:26 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang, Jiri Olsa,
Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Ravi Bangoria
On Wed, Apr 9, 2025 at 6:02 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> The evsel__handle_error_quirks() is to fixup invalid event attributes on
> some architecture based on the error code. Currently it's only used for
> AMD to disable precise_ip not to use IBS which has more restrictions.
>
> But the commit c33aea446bf555ab changed call evsel__precise_ip_fallback
> for any errors so there's no difference with the above function. To
> make matter worse, it caused a problem with branch stack on Zen3.
>
> The IBS doesn't support branch stack so it should use a regular core
> PMU event. The default event is set precise_max and it starts with 3.
> And evsel__precise_ip_fallback() tries with it and reduces the level one
> by one. At last it tries with 0 but it also failed on Zen3 since the
> branch stack is not supported for the cycles event.
>
> At this point, evsel__precise_ip_fallback() restores the original
> precise_ip value (3) in the hope that it can succeed with other modifier
> (like exclude_kernel). Then evsel__handle_error_quirks() see it has
> precise_ip != 0 and make it retry with 0. This created an infinite
> loop.
>
> Before:
>
> $ perf record -b -vv |& grep removing
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> removing precise_ip on AMD
> ...
>
> After:
>
> $ perf record -b true
> Error:
> Failure to open event 'cycles:P' on PMU 'cpu' which will be removed.
> Invalid event (cycles:P) in per-thread mode, enable system wide with '-a'.
> Error:
> Failure to open any events for recording.
>
> Fixes: c33aea446bf555ab ("perf tools: Fix precise_ip fallback logic")
> Cc: Ravi Bangoria <ravi.bangoria@amd.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Chun-Tse Shao <ctshao@google.com>
> ---
> tools/perf/util/evsel.c | 22 ----------------------
> 1 file changed, 22 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 1974395492d7da5e..3c030da2e477c707 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -2566,25 +2566,6 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
> return false;
> }
>
> -static bool evsel__handle_error_quirks(struct evsel *evsel, int error)
> -{
> - /*
> - * AMD core PMU tries to forward events with precise_ip to IBS PMU
> - * implicitly. But IBS PMU has more restrictions so it can fail with
> - * supported event attributes. Let's forward it back to the core PMU
> - * by clearing precise_ip only if it's from precise_max (:P).
> - */
> - if ((error == -EINVAL || error == -ENOENT) && x86__is_amd_cpu() &&
> - evsel->core.attr.precise_ip && evsel->precise_max) {
> - evsel->core.attr.precise_ip = 0;
> - pr_debug2_peo("removing precise_ip on AMD\n");
> - display_attr(&evsel->core.attr);
> - return true;
> - }
> -
> - return false;
> -}
> -
> static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> struct perf_thread_map *threads,
> int start_cpu_map_idx, int end_cpu_map_idx)
> @@ -2730,9 +2711,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> if (evsel__precise_ip_fallback(evsel))
> goto retry_open;
>
> - if (evsel__handle_error_quirks(evsel, err))
> - goto retry_open;
> -
> out_close:
> if (err)
> threads->err_thread = thread;
> --
> 2.49.0.504.g3bcea36a83-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] perf tools: Remove evsel__handle_error_quirks()
2025-04-10 3:26 ` Chun-Tse Shao
@ 2025-04-10 16:31 ` Namhyung Kim
0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2025-04-10 16:31 UTC (permalink / raw)
To: Chun-Tse Shao
Cc: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang, Jiri Olsa,
Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Ravi Bangoria
Hello,
On Wed, Apr 09, 2025 at 08:26:07PM -0700, Chun-Tse Shao wrote:
> On Wed, Apr 9, 2025 at 6:02 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > The evsel__handle_error_quirks() is to fixup invalid event attributes on
> > some architecture based on the error code. Currently it's only used for
> > AMD to disable precise_ip not to use IBS which has more restrictions.
> >
> > But the commit c33aea446bf555ab changed call evsel__precise_ip_fallback
> > for any errors so there's no difference with the above function. To
> > make matter worse, it caused a problem with branch stack on Zen3.
> >
> > The IBS doesn't support branch stack so it should use a regular core
> > PMU event. The default event is set precise_max and it starts with 3.
> > And evsel__precise_ip_fallback() tries with it and reduces the level one
> > by one. At last it tries with 0 but it also failed on Zen3 since the
> > branch stack is not supported for the cycles event.
> >
> > At this point, evsel__precise_ip_fallback() restores the original
> > precise_ip value (3) in the hope that it can succeed with other modifier
> > (like exclude_kernel). Then evsel__handle_error_quirks() see it has
> > precise_ip != 0 and make it retry with 0. This created an infinite
> > loop.
> >
> > Before:
> >
> > $ perf record -b -vv |& grep removing
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > removing precise_ip on AMD
> > ...
> >
> > After:
> >
> > $ perf record -b true
> > Error:
> > Failure to open event 'cycles:P' on PMU 'cpu' which will be removed.
> > Invalid event (cycles:P) in per-thread mode, enable system wide with '-a'.
> > Error:
> > Failure to open any events for recording.
> >
> > Fixes: c33aea446bf555ab ("perf tools: Fix precise_ip fallback logic")
> > Cc: Ravi Bangoria <ravi.bangoria@amd.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> Tested-by: Chun-Tse Shao <ctshao@google.com>
Thanks for the test.
Arnaldo, I think we can have this in the perf-tools tree.
Thanks,
Namhyung
> > ---
> > tools/perf/util/evsel.c | 22 ----------------------
> > 1 file changed, 22 deletions(-)
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 1974395492d7da5e..3c030da2e477c707 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -2566,25 +2566,6 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
> > return false;
> > }
> >
> > -static bool evsel__handle_error_quirks(struct evsel *evsel, int error)
> > -{
> > - /*
> > - * AMD core PMU tries to forward events with precise_ip to IBS PMU
> > - * implicitly. But IBS PMU has more restrictions so it can fail with
> > - * supported event attributes. Let's forward it back to the core PMU
> > - * by clearing precise_ip only if it's from precise_max (:P).
> > - */
> > - if ((error == -EINVAL || error == -ENOENT) && x86__is_amd_cpu() &&
> > - evsel->core.attr.precise_ip && evsel->precise_max) {
> > - evsel->core.attr.precise_ip = 0;
> > - pr_debug2_peo("removing precise_ip on AMD\n");
> > - display_attr(&evsel->core.attr);
> > - return true;
> > - }
> > -
> > - return false;
> > -}
> > -
> > static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> > struct perf_thread_map *threads,
> > int start_cpu_map_idx, int end_cpu_map_idx)
> > @@ -2730,9 +2711,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> > if (evsel__precise_ip_fallback(evsel))
> > goto retry_open;
> >
> > - if (evsel__handle_error_quirks(evsel, err))
> > - goto retry_open;
> > -
> > out_close:
> > if (err)
> > threads->err_thread = thread;
> > --
> > 2.49.0.504.g3bcea36a83-goog
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-10 16:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-10 1:02 [PATCH] perf tools: Remove evsel__handle_error_quirks() Namhyung Kim
2025-04-10 3:26 ` Chun-Tse Shao
2025-04-10 16:31 ` Namhyung Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.