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 4/9] perf tools: Do not set exclude_guest for precise_ip
Date: Tue, 15 Oct 2024 23:23:54 -0700 [thread overview]
Message-ID: <20241016062359.264929-5-namhyung@kernel.org> (raw)
In-Reply-To: <20241016062359.264929-1-namhyung@kernel.org>
It seems perf sets the exclude_guest bit because of Intel PEBS
implementation which uses a virtual address. IIUC now kernel disables
PEBS when it goes to the guest mode regardless of this bit so we don't
need to set it explicitly. At least for the other archs/vendors.
I found the commit 1342798cc13e set the exclude_guest for precise_ip
in the tool and the commit 20b279ddb38c added kernel side enforcement
which was reverted by commit a706d965dcfd later.
Actually it doesn't set the exclude_guest for the default event
(cycles:P) already.
$ grep -m1 vendor /proc/cpuinfo
vendor_id : GenuineIntel
$ perf record -e cycles:P true
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.002 MB perf.data (9 samples) ]
$ perf evlist -v | tr ',' '\n' | grep -e exclude -e precise
precise_ip: 3
But having lower 'p' modifier set the bit for some reason.
$ perf record -e cycles:pp true
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.002 MB perf.data (9 samples) ]
$ perf evlist -v | tr ',' '\n' | grep -e exclude -e precise
precise_ip: 2
exclude_guest: 1
Actually AMD IBS suffers from this because it doesn't support excludes
and having this bit effectively disables new features in the current
implementation (due to the missing feature check).
$ grep -m1 vendor /proc/cpuinfo
vendor_id : AuthenticAMD
$ perf record -W -e cycles:p -vv true 2>&1 | grep switching
switching off PERF_FORMAT_LOST support
switching off weight struct support
switching off bpf_event
switching off ksymbol
switching off cloexec flag
switching off mmap2
switching off exclude_guest, exclude_host
By not setting exclude_guest, we can fix this inconsistency and the
troubles.
Reviewed-by: Ian Rogers <irogers@google.com>
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/tests/parse-events.c | 12 ++++--------
tools/perf/util/parse-events.c | 4 ----
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 727683f249f66f5a..82a19674a38f774e 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -898,8 +898,7 @@ static int test__group1(struct evlist *evlist)
TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
- /* use of precise requires exclude_guest */
- TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
+ TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip == 2);
TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
@@ -1016,9 +1015,8 @@ static int test__group3(struct evlist *evlist __maybe_unused)
TEST_ASSERT_VAL("wrong exclude_kernel",
!evsel->core.attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
- /* use of precise requires exclude_guest */
TEST_ASSERT_VAL("wrong exclude guest",
- evsel->core.attr.exclude_guest);
+ !evsel->core.attr.exclude_guest);
TEST_ASSERT_VAL("wrong exclude host",
!evsel->core.attr.exclude_host);
TEST_ASSERT_VAL("wrong precise_ip",
@@ -1103,8 +1101,7 @@ static int test__group4(struct evlist *evlist __maybe_unused)
TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
- /* use of precise requires exclude_guest */
- TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
+ TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip == 1);
TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
@@ -1122,8 +1119,7 @@ static int test__group4(struct evlist *evlist __maybe_unused)
TEST_ASSERT_VAL("wrong exclude_user", evsel->core.attr.exclude_user);
TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
- /* use of precise requires exclude_guest */
- TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
+ TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip == 2);
TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 687125570f563382..afeb8d815bbff26d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1733,10 +1733,6 @@ static int parse_events__modifier_list(struct parse_events_state *parse_state,
int exclude = eu | ek | eh;
int exclude_GH = group ? evsel->exclude_GH : 0;
- if (mod.precise) {
- /* use of precise requires exclude_guest */
- eG = 1;
- }
if (mod.user) {
if (!exclude)
exclude = eu = ek = eh = 1;
--
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 ` Namhyung Kim [this message]
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 ` [PATCH v5 7/9] perf tools: Check fallback error and order Namhyung Kim
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-5-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).