Received-SPF: pass (google.com: domain of linux-kernel+bounces-189423-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sat, 25 May 2024 08:29:27 -0700
Message-Id: <20240525152927.665498-1-irogers@google.com>
Precedence: bulk
Mime-Version: 1.0
Subject: [PATCH v1] perf evlist: Force adding default events only to core PMUs
From: Ian Rogers <irogers@google.com>
To: Linus Torvalds <torvalds@linux-foundation.org>, Peter Zijlstra <peterz@infradead.org>, 
	Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Namhyung Kim <namhyung@kernel.org>, 
	Mark Rutland <mark.rutland@arm.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, 
	Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, 
	Kan Liang <kan.liang@linux.intel.com>, James Clark <james.clark@arm.com>, 
	Dominique Martinet <asmadeus@codewreck.org>, linux-perf-users@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

PMUs other than core PMUs may have a 'cycles' event. Default opening a
non-core cycles event with perf record can lead to perf_event_open
failure. Avoid this by only opening the default 'cycles' event on core
PMUs.

Closes: https://lore.kernel.org/lkml/CAHk-=wiWvtFyedDNpoV7a8Fq_FpbB+F5KmWK2xPY3QoYseOf_A@mail.gmail.com/
Fixes: 617824a7f0f7 ("perf parse-events: Prefer sysfs/JSON hardware events over legacy")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-record.c |  6 ++----
 tools/perf/builtin-top.c    |  3 +--
 tools/perf/util/evlist.c    | 43 ++++++++++++++++++++++++++++++++++---
 tools/perf/util/evlist.h    |  1 +
 4 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 66a3de8ac661..b968c3c2def6 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -3198,7 +3198,7 @@ static int switch_output_setup(struct record *rec)
 	unsigned long val;
 
 	/*
-	 * If we're using --switch-output-events, then we imply its 
+	 * If we're using --switch-output-events, then we imply its
 	 * --switch-output=signal, as we'll send a SIGUSR2 from the side band
 	 *  thread to its parent.
 	 */
@@ -4154,9 +4154,7 @@ int cmd_record(int argc, const char **argv)
 		record.opts.tail_synthesize = true;
 
 	if (rec->evlist->core.nr_entries == 0) {
-		bool can_profile_kernel = perf_event_paranoid_check(1);
-
-		err = parse_event(rec->evlist, can_profile_kernel ? "cycles:P" : "cycles:Pu");
+		err = evlist__add_default_events(rec->evlist);
 		if (err)
 			goto out;
 	}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 1d6aef51c122..90b97fc24edb 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1665,8 +1665,7 @@ int cmd_top(int argc, const char **argv)
 		goto out_delete_evlist;
 
 	if (!top.evlist->core.nr_entries) {
-		bool can_profile_kernel = perf_event_paranoid_check(1);
-		int err = parse_event(top.evlist, can_profile_kernel ? "cycles:P" : "cycles:Pu");
+		int err = evlist__add_default_events(top.evlist);
 
 		if (err)
 			goto out_delete_evlist;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 3a719edafc7a..ddca50cb049f 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -32,6 +32,7 @@
 #include "util/sample.h"
 #include "util/bpf-filter.h"
 #include "util/stat.h"
+#include "util/strbuf.h"
 #include "util/util.h"
 #include <signal.h>
 #include <unistd.h>
@@ -93,14 +94,12 @@ struct evlist *evlist__new(void)
 struct evlist *evlist__new_default(void)
 {
 	struct evlist *evlist = evlist__new();
-	bool can_profile_kernel;
 	int err;
 
 	if (!evlist)
 		return NULL;
 
-	can_profile_kernel = perf_event_paranoid_check(1);
-	err = parse_event(evlist, can_profile_kernel ? "cycles:P" : "cycles:Pu");
+	err = evlist__add_default_events(evlist);
 	if (err) {
 		evlist__delete(evlist);
 		return NULL;
@@ -187,6 +186,44 @@ void evlist__delete(struct evlist *evlist)
 	free(evlist);
 }
 
+int evlist__add_default_events(struct evlist *evlist)
+{
+	struct perf_pmu *pmu = NULL;
+	bool can_profile_kernel = perf_event_paranoid_check(1);
+	struct strbuf events;
+	bool first = true;
+	int err;
+
+	err = strbuf_init(&events, /*hint=*/32);
+	if (err)
+		return err;
+
+	while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
+		if (!first) {
+			err = strbuf_addch(&events, ',');
+			if (err)
+				goto err_out;
+		}
+		err = strbuf_addstr(&events, pmu->name);
+		if (err < 0)
+			goto err_out;
+
+		if (can_profile_kernel)
+			err = strbuf_addstr(&events, "/cycles/P");
+		else
+			err = strbuf_addstr(&events, "/cycles/Pu");
+
+		if (err < 0)
+			goto err_out;
+
+		first = false;
+	}
+	err = parse_event(evlist, events.buf);
+err_out:
+	strbuf_release(&events);
+	return err;
+}
+
 void evlist__add(struct evlist *evlist, struct evsel *entry)
 {
 	perf_evlist__add(&evlist->core, &entry->core);
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index cb91dc9117a2..269db02f7b45 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -97,6 +97,7 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
 void evlist__exit(struct evlist *evlist);
 void evlist__delete(struct evlist *evlist);
 
+int evlist__add_default_events(struct evlist *evlist);
 void evlist__add(struct evlist *evlist, struct evsel *entry);
 void evlist__remove(struct evlist *evlist, struct evsel *evsel);
 
-- 
2.45.1.288.g0e0cd299f1-goog