From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
Ian Rogers <irogers@google.com>,
Kan Liang <kan.liang@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users@vger.kernel.org,
Ravi Bangoria <ravi.bangoria@amd.com>,
Mark Rutland <mark.rutland@arm.com>,
James Clark <james.clark@arm.com>,
Kajol Jain <kjain@linux.ibm.com>,
Thomas Richter <tmricht@linux.ibm.com>,
Atish Patra <atishp@atishpatra.org>,
Palmer Dabbelt <palmer@rivosinc.com>,
Mingwei Zhang <mizhang@google.com>,
James Clark <james.clark@linaro.org>
Subject: [PATCH v5 7/9] perf tools: Check fallback error and order
Date: Tue, 15 Oct 2024 23:23:57 -0700 [thread overview]
Message-ID: <20241016062359.264929-8-namhyung@kernel.org> (raw)
In-Reply-To: <20241016062359.264929-1-namhyung@kernel.org>
The perf_event_open might fail due to various reasons, so blindly
reducing precise_ip level might not be the best way to deal with it.
It seems the kernel return -EOPNOTSUPP when PMU doesn't support the
given precise level. Let's try again with the correct error code.
This caused a problem on AMD, as it stops on precise_ip of 2 for IBS but
user events with exclude_kernel=1 cannot make progress. Let's add the
evsel__handle_error_quirks() to this case specially. I plan to work on
the kernel side to improve this situation but it'd still need some
special handling for IBS.
Acked-by: Kan Liang <kan.liang@linux.intel.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Reviewed-by: Ravi Bangoria <ravi.bangoria@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evsel.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a7213cfa75f31150..68380d08a21c5a36 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2060,9 +2060,9 @@ static void evsel__detect_missing_brstack_features(struct evsel *evsel)
/*
* Must probe features in the order they were added to the
- * perf_event_attr interface. These are PMU specific limitation
- * so we can detect with the given hardware event and stop on the
- * first one succeeded.
+ * perf_event_attr interface. These are kernel core limitation but
+ * specific to PMUs with branch stack. So we can detect with the given
+ * hardware event and stop on the first one succeeded.
*/
/* Please add new feature detection here. */
@@ -2288,6 +2288,25 @@ static bool evsel__detect_missing_features(struct evsel *evsel)
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)
@@ -2408,9 +2427,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
return 0;
try_fallback:
- if (evsel__precise_ip_fallback(evsel))
- goto retry_open;
-
if (evsel__ignore_missing_thread(evsel, perf_cpu_map__nr(cpus),
idx, threads, thread, err)) {
/* We just removed 1 thread, so lower the upper nthreads limit. */
@@ -2427,11 +2443,15 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
if (err == -EMFILE && rlimit__increase_nofile(&set_rlimit))
goto retry_open;
- if (err != -EINVAL || idx > 0 || thread > 0)
- goto out_close;
+ if (err == -EOPNOTSUPP && evsel__precise_ip_fallback(evsel))
+ goto retry_open;
- if (evsel__detect_missing_features(evsel))
+ if (err == -EINVAL && evsel__detect_missing_features(evsel))
goto fallback_missing_features;
+
+ if (evsel__handle_error_quirks(evsel, err))
+ goto retry_open;
+
out_close:
if (err)
threads->err_thread = thread;
--
2.47.0.rc1.288.g06298d1525-goog
next prev parent reply other threads:[~2024-10-16 6:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-16 6:23 [PATCHSET 0/9] perf tools: Do not set attr.exclude_guest by default (v5) Namhyung Kim
2024-10-16 6:23 ` [PATCH v5 1/9] perf tools: Add fallback for exclude_guest Namhyung Kim
2024-10-16 6:23 ` [PATCH v5 2/9] perf tools: Don't set attr.exclude_guest by default Namhyung Kim
2024-10-16 6:23 ` [PATCH v5 3/9] perf tools: Simplify evsel__add_modifier() Namhyung Kim
2024-10-16 6:23 ` [PATCH v5 4/9] perf tools: Do not set exclude_guest for precise_ip Namhyung Kim
2024-10-16 6:23 ` [PATCH v5 5/9] perf tools: Detect missing kernel features properly Namhyung Kim
2024-10-16 6:23 ` [PATCH v5 6/9] perf tools: Move x86__is_amd_cpu() to util/env.c Namhyung Kim
2024-10-16 6:23 ` Namhyung Kim [this message]
2024-10-16 6:23 ` [PATCH v5 8/9] perf record: Just use "cycles:P" as the default event Namhyung Kim
2024-10-16 6:23 ` [PATCH v5 9/9] perf test: Add precise_max subtest to the perf record shell test Namhyung Kim
2024-10-23 18:01 ` [PATCHSET 0/9] perf tools: Do not set attr.exclude_guest by default (v5) Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241016062359.264929-8-namhyung@kernel.org \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=atishp@atishpatra.org \
--cc=irogers@google.com \
--cc=james.clark@arm.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=kjain@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=mizhang@google.com \
--cc=palmer@rivosinc.com \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
--cc=tmricht@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.