linux-kernel - [PATCH -next 2/2] perf stat: Fix incorrect display of bperf when event count is 0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240925135523.367957-3-wutengda@huaweicloud.com>
Date: Wed, 25 Sep 2024 13:55:23 +0000
From: Tengda Wu <wutengda@...weicloud.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: song@...nel.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>,
	Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	kan.liang@...ux.intel.com,
	linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	bpf@...r.kernel.org
Subject: [PATCH -next 2/2] perf stat: Fix incorrect display of bperf when event count is 0

There are 2 possible reasons for the event count being 0: not
supported and not counted. Perf distinguishes between these two
possibilities through `evsel->supported`, but in bperf, this value
is always false. This is because bperf is prematurely break or
continue in the evlist__for_each_cpu loop under __run_perf_stat(),
skipping the `counter->supported` assignment, resulting in bperf
incorrectly displaying <not supported> when the count is 0.

The most direct way to fix it is to do `evsel->supported` assignment
when opening an event in bperf. However, since bperf only opens
events when loading the leader, the followers are not aware of
whether the event is supported or not. Therefore, we store the
`evsel->supported` value in a common location, which is the
`perf_event_attr_map`, to achieve synchronization of event support
across perf sessions.

Fixes: 7fac83aaf2ee ("perf stat: Introduce 'bperf' to share hardware PMCs with BPF")
Signed-off-by: Tengda Wu <wutengda@...weicloud.com>
---
 tools/lib/perf/include/perf/bpf_perf.h |  1 +
 tools/perf/util/bpf_counter.c          | 18 ++++++++++--------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/lib/perf/include/perf/bpf_perf.h b/tools/lib/perf/include/perf/bpf_perf.h
index e7cf6ba7b674..64c8d211726d 100644
--- a/tools/lib/perf/include/perf/bpf_perf.h
+++ b/tools/lib/perf/include/perf/bpf_perf.h
@@ -23,6 +23,7 @@
 struct perf_event_attr_map_entry {
 	__u32 link_id;
 	__u32 diff_map_id;
+	__u8 supported;
 };
 
 /* default attr_map name */
diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
index 3346129c20cf..c04b274c3c45 100644
--- a/tools/perf/util/bpf_counter.c
+++ b/tools/perf/util/bpf_counter.c
@@ -425,18 +425,19 @@ static int bperf_reload_leader_program(struct evsel *evsel, int attr_map_fd,
 	diff_map_fd = bpf_map__fd(skel->maps.diff_readings);
 	entry->link_id = bpf_link_get_id(link_fd);
 	entry->diff_map_id = bpf_map_get_id(diff_map_fd);
-	err = bpf_map_update_elem(attr_map_fd, &evsel->core.attr, entry, BPF_ANY);
-	assert(err == 0);
-
-	evsel->bperf_leader_link_fd = bpf_link_get_fd_by_id(entry->link_id);
-	assert(evsel->bperf_leader_link_fd >= 0);
-
 	/*
 	 * save leader_skel for install_pe, which is called within
 	 * following evsel__open_per_cpu call
 	 */
 	evsel->leader_skel = skel;
-	evsel__open_per_cpu(evsel, all_cpu_map, -1);
+	if (!evsel__open_per_cpu(evsel, all_cpu_map, -1))
+		entry->supported = true;
+
+	err = bpf_map_update_elem(attr_map_fd, &evsel->core.attr, entry, BPF_ANY);
+	assert(err == 0);
+
+	evsel->bperf_leader_link_fd = bpf_link_get_fd_by_id(entry->link_id);
+	assert(evsel->bperf_leader_link_fd >= 0);
 
 out:
 	bperf_leader_bpf__destroy(skel);
@@ -446,7 +447,7 @@ static int bperf_reload_leader_program(struct evsel *evsel, int attr_map_fd,
 
 static int bperf__load(struct evsel *evsel, struct target *target)
 {
-	struct perf_event_attr_map_entry entry = {0xffffffff, 0xffffffff};
+	struct perf_event_attr_map_entry entry = {0xffffffff, 0xffffffff, false};
 	int attr_map_fd, diff_map_fd = -1, err;
 	enum bperf_filter_type filter_type;
 	__u32 filter_entry_cnt, i;
@@ -494,6 +495,7 @@ static int bperf__load(struct evsel *evsel, struct target *target)
 		err = -1;
 		goto out;
 	}
+	evsel->supported = entry.supported;
 	/*
 	 * The bpf_link holds reference to the leader program, and the
 	 * leader program holds reference to the maps. Therefore, if
-- 
2.34.1