[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250425214008.176100-9-irogers@google.com>
Date: Fri, 25 Apr 2025 14:40:06 -0700
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.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 <kan.liang@...ux.intel.com>, James Clark <james.clark@...aro.org>,
Dapeng Mi <dapeng1.mi@...ux.intel.com>, Thomas Richter <tmricht@...ux.ibm.com>,
Veronika Molnarova <vmolnaro@...hat.com>, Hao Ge <gehao@...inos.cn>,
Howard Chu <howardchu95@...il.com>, Weilin Wang <weilin.wang@...el.com>,
Levi Yun <yeoreum.yun@....com>, "Dr. David Alan Gilbert" <linux@...blig.org>,
Dominique Martinet <asmadeus@...ewreck.org>, Xu Yang <xu.yang_2@....com>,
Tengda Wu <wutengda@...weicloud.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: [PATCH v3 08/10] perf bench evlist-open-close: Switch user option to
use BPF filter
The existing --uid option finds user processes by scanning
/proc. Scanning /proc is inherently racy due to processes being in
/proc but then exiting before perf does the perf_event_open. When the
perf_event_open fails perf will terminate reporting errors which is a
disappointing user experience. Scanning /proc when perf starts also
cannot inform perf of new user processes starting.
The ability to filter perf events with BPF isn't new, and has been in
the perf tool for 10 years:
https://lore.kernel.org/all/1444826502-49291-8-git-send-email-wangnan0@huawei.com/
An ability to do filtering on the command line with a BPF program
that's part of perf was added 2 years ago:
https://lore.kernel.org/all/20230314234237.3008956-1-namhyung@kernel.org/
This was then extended to support uids as a way of filtering:
https://lore.kernel.org/all/20240524205227.244375-1-irogers@google.com/
This change switches the benchmark's --uid option to use the BPF
filter code to avoid the inherent race and existing failures.
Using BPF has permission issues in loading the BPF program not present
in scanning /proc. As the scanning approach would miss new programs
and fail due to the race, this is considered preferable. The change
also avoids opening a perf event per PID, which is less overhead in
the kernel.
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/bench/evlist-open-close.c | 36 ++++++++++++++++------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/tools/perf/bench/evlist-open-close.c b/tools/perf/bench/evlist-open-close.c
index 79cedcf94a39..bfaf50e4e519 100644
--- a/tools/perf/bench/evlist-open-close.c
+++ b/tools/perf/bench/evlist-open-close.c
@@ -57,7 +57,7 @@ static int evlist__count_evsel_fds(struct evlist *evlist)
return cnt;
}
-static struct evlist *bench__create_evlist(char *evstr)
+static struct evlist *bench__create_evlist(char *evstr, const char *uid_str)
{
struct parse_events_error err;
struct evlist *evlist = evlist__new();
@@ -78,6 +78,18 @@ static struct evlist *bench__create_evlist(char *evstr)
goto out_delete_evlist;
}
parse_events_error__exit(&err);
+ if (uid_str) {
+ uid_t uid = parse_uid(uid_str);
+
+ if (uid == UINT_MAX) {
+ pr_err("Invalid User: %s", uid_str);
+ ret = -EINVAL;
+ goto out_delete_evlist;
+ }
+ ret = parse_uid_filter(evlist, uid);
+ if (ret)
+ goto out_delete_evlist;
+ }
ret = evlist__create_maps(evlist, &opts.target);
if (ret < 0) {
pr_err("Not enough memory to create thread/cpu maps\n");
@@ -117,10 +129,10 @@ static int bench__do_evlist_open_close(struct evlist *evlist)
return 0;
}
-static int bench_evlist_open_close__run(char *evstr)
+static int bench_evlist_open_close__run(char *evstr, const char *uid_str)
{
// used to print statistics only
- struct evlist *evlist = bench__create_evlist(evstr);
+ struct evlist *evlist = bench__create_evlist(evstr, uid_str);
double time_average, time_stddev;
struct timeval start, end, diff;
struct stats time_stats;
@@ -142,7 +154,7 @@ static int bench_evlist_open_close__run(char *evstr)
for (i = 0; i < iterations; i++) {
pr_debug("Started iteration %d\n", i);
- evlist = bench__create_evlist(evstr);
+ evlist = bench__create_evlist(evstr, uid_str);
if (!evlist)
return -ENOMEM;
@@ -206,6 +218,7 @@ static char *bench__repeat_event_string(const char *evstr, int n)
int bench_evlist_open_close(int argc, const char **argv)
{
+ const char *uid_str = NULL;
const struct option options[] = {
OPT_STRING('e', "event", &event_string, "event",
"event selector. use 'perf list' to list available events"),
@@ -221,7 +234,7 @@ int bench_evlist_open_close(int argc, const char **argv)
"record events on existing process id"),
OPT_STRING('t', "tid", &opts.target.tid, "tid",
"record events on existing thread id"),
- OPT_STRING('u', "uid", &opts.target.uid_str, "user", "user to profile"),
+ OPT_STRING('u', "uid", &uid_str, "user", "user to profile"),
OPT_BOOLEAN(0, "per-thread", &opts.target.per_thread, "use per-thread mmaps"),
OPT_END()
};
@@ -245,15 +258,8 @@ int bench_evlist_open_close(int argc, const char **argv)
goto out;
}
- err = target__parse_uid(&opts.target);
- if (err) {
- target__strerror(&opts.target, err, errbuf, sizeof(errbuf));
- pr_err("%s", errbuf);
- goto out;
- }
-
- /* Enable ignoring missing threads when -u/-p option is defined. */
- opts.ignore_missing_thread = opts.target.uid != UINT_MAX || opts.target.pid;
+ /* Enable ignoring missing threads when -p option is defined. */
+ opts.ignore_missing_thread = opts.target.pid;
evstr = bench__repeat_event_string(event_string, nr_events);
if (!evstr) {
@@ -261,7 +267,7 @@ int bench_evlist_open_close(int argc, const char **argv)
goto out;
}
- err = bench_evlist_open_close__run(evstr);
+ err = bench_evlist_open_close__run(evstr, uid_str);
free(evstr);
out:
--
2.49.0.850.g28803427d3-goog
Powered by blists - more mailing lists