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
Subject: [PATCH v1] perf evlist: Force adding default events only to core PMUs
Date: Sat, 25 May 2024 08:29:27 -0700 [thread overview]
Message-ID: <20240525152927.665498-1-irogers@google.com> (raw)
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
next reply other threads:[~2024-05-25 15:30 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-25 15:29 Ian Rogers [this message]
2024-05-25 16:43 ` [PATCH v1] perf evlist: Force adding default events only to core PMUs Linus Torvalds
2024-05-25 21:14 ` Linus Torvalds
2024-05-27 10:58 ` Leo Yan
2024-05-28 5:36 ` Ian Rogers
2024-05-28 17:00 ` Linus Torvalds
2024-05-28 17:39 ` Ian Rogers
2024-05-28 18:12 ` Linus Torvalds
2024-05-28 18:58 ` Ian Rogers
2024-05-28 19:42 ` Linus Torvalds
2024-05-28 20:03 ` Ian Rogers
2024-05-28 20:33 ` Linus Torvalds
2024-05-28 21:37 ` Ian Rogers
2024-05-28 21:42 ` Linus Torvalds
2024-05-28 19:44 ` Arnaldo Carvalho de Melo
2024-05-28 19:51 ` Ian Rogers
2024-05-29 14:50 ` James Clark
2024-05-29 17:33 ` Ian Rogers
2024-05-30 15:37 ` James Clark
2024-05-30 16:14 ` Ian Rogers
2024-05-29 18:44 ` Arnaldo Carvalho de Melo
2024-05-29 19:25 ` Ian Rogers
2024-05-30 5:35 ` Namhyung Kim
2024-05-30 12:48 ` James Clark
2024-05-30 13:46 ` Ian Rogers
2024-05-30 22:51 ` Namhyung Kim
2024-06-05 20:29 ` Namhyung Kim
2024-06-05 23:02 ` Ian Rogers
2024-06-06 7:09 ` Namhyung Kim
2024-06-06 9:42 ` James Clark
2024-06-06 13:51 ` Arnaldo Carvalho de Melo
2024-06-07 6:10 ` Namhyung Kim
2024-06-06 13:47 ` Arnaldo Carvalho de Melo
2024-05-28 20:00 ` Linus Torvalds
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=20240525152927.665498-1-irogers@google.com \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=asmadeus@codewreck.org \
--cc=james.clark@arm.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.org \
/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).