linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/10] Move uid filtering to BPF filters
@ 2025-06-04 17:45 Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 01/10] perf parse-events filter: Use evsel__find_pmu Ian Rogers
                   ` (11 more replies)
  0 siblings, 12 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Rather than scanning /proc and skipping PIDs based on their UIDs, use
BPF filters for uid filtering. The /proc scanning in thread_map is
racy as the PID may exit before the perf_event_open causing perf to
abort. BPF UID filters are more robust as they avoid the race. The
/proc scanning also misses processes starting after the perf
command. Add a helper for commands that support UID filtering and wire
up. Remove the non-BPF UID filtering support given it doesn't work.

v4: Add a warning message on top of Namhyung's BPF filter error message:
https://lore.kernel.org/lkml/20250604054234.23608-1-namhyung@kernel.org/
    in the parse_uid_filter helper. In TUI the warning is shown then
    the BPF error shown, with stdio the warning appears below the BPF
    errors.

v3: Add lengthier commit messages as requested by Arnaldo. Rebase on
    tmp.perf-tools-next.

v2: Add a perf record uid test (Namhyung) and force setting
    system-wide for perf trace and perf record (Namhyung). Ensure the
    uid filter isn't set on tracepoint evsels.

v1: https://lore.kernel.org/lkml/20250111190143.1029906-1-irogers@google.com/

Ian Rogers (10):
  perf parse-events filter: Use evsel__find_pmu
  perf target: Separate parse_uid into its own function
  perf parse-events: Add parse_uid_filter helper
  perf record: Switch user option to use BPF filter
  perf tests record: Add basic uid filtering test
  perf top: Switch user option to use BPF filter
  perf trace: Switch user option to use BPF filter
  perf bench evlist-open-close: Switch user option to use BPF filter
  perf target: Remove uid from target
  perf thread_map: Remove uid options

 tools/perf/bench/evlist-open-close.c        | 36 ++++++++------
 tools/perf/builtin-ftrace.c                 |  1 -
 tools/perf/builtin-kvm.c                    |  2 -
 tools/perf/builtin-record.c                 | 27 ++++++-----
 tools/perf/builtin-stat.c                   |  4 +-
 tools/perf/builtin-top.c                    | 22 +++++----
 tools/perf/builtin-trace.c                  | 27 +++++++----
 tools/perf/tests/backward-ring-buffer.c     |  1 -
 tools/perf/tests/event-times.c              |  8 ++-
 tools/perf/tests/keep-tracking.c            |  2 +-
 tools/perf/tests/mmap-basic.c               |  2 +-
 tools/perf/tests/openat-syscall-all-cpus.c  |  2 +-
 tools/perf/tests/openat-syscall-tp-fields.c |  1 -
 tools/perf/tests/openat-syscall.c           |  2 +-
 tools/perf/tests/perf-record.c              |  1 -
 tools/perf/tests/perf-time-to-tsc.c         |  2 +-
 tools/perf/tests/shell/record.sh            | 26 ++++++++++
 tools/perf/tests/switch-tracking.c          |  2 +-
 tools/perf/tests/task-exit.c                |  1 -
 tools/perf/tests/thread-map.c               |  2 +-
 tools/perf/util/bpf-filter.c                |  2 +-
 tools/perf/util/evlist.c                    |  3 +-
 tools/perf/util/parse-events.c              | 47 +++++++++++++-----
 tools/perf/util/parse-events.h              |  1 +
 tools/perf/util/python.c                    | 10 ++--
 tools/perf/util/target.c                    | 54 +++------------------
 tools/perf/util/target.h                    | 15 ++----
 tools/perf/util/thread_map.c                | 32 ++----------
 tools/perf/util/thread_map.h                |  6 +--
 tools/perf/util/top.c                       |  4 +-
 tools/perf/util/top.h                       |  1 +
 31 files changed, 164 insertions(+), 182 deletions(-)

-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 01/10] perf parse-events filter: Use evsel__find_pmu
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 02/10] perf target: Separate parse_uid into its own function Ian Rogers
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Rather than manually scanning PMUs, use evsel__find_pmu that can use
the PMU set during event parsing.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/parse-events.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 2380de56a207..d96adf23dc94 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2564,9 +2564,8 @@ foreach_evsel_in_last_glob(struct evlist *evlist,
 static int set_filter(struct evsel *evsel, const void *arg)
 {
 	const char *str = arg;
-	bool found = false;
 	int nr_addr_filters = 0;
-	struct perf_pmu *pmu = NULL;
+	struct perf_pmu *pmu;
 
 	if (evsel == NULL) {
 		fprintf(stderr,
@@ -2584,16 +2583,11 @@ static int set_filter(struct evsel *evsel, const void *arg)
 		return 0;
 	}
 
-	while ((pmu = perf_pmus__scan(pmu)) != NULL)
-		if (pmu->type == evsel->core.attr.type) {
-			found = true;
-			break;
-		}
-
-	if (found)
+	pmu = evsel__find_pmu(evsel);
+	if (pmu) {
 		perf_pmu__scan_file(pmu, "nr_addr_filters",
 				    "%d", &nr_addr_filters);
-
+	}
 	if (!nr_addr_filters)
 		return perf_bpf_filter__parse(&evsel->bpf_filters, str);
 
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 02/10] perf target: Separate parse_uid into its own function
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 01/10] perf parse-events filter: Use evsel__find_pmu Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper Ian Rogers
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Allow parse_uid to be called without a struct target. Rather than have
two errors, remove TARGET_ERRNO__USER_NOT_FOUND and use
TARGET_ERRNO__INVALID_UID as the handling is identical.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/target.c | 22 ++++++++++++----------
 tools/perf/util/target.h |  3 ++-
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 0f383418e3df..f3ad59ccfa99 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -94,15 +94,13 @@ enum target_errno target__validate(struct target *target)
 	return ret;
 }
 
-enum target_errno target__parse_uid(struct target *target)
+uid_t parse_uid(const char *str)
 {
 	struct passwd pwd, *result;
 	char buf[1024];
-	const char *str = target->uid_str;
 
-	target->uid = UINT_MAX;
 	if (str == NULL)
-		return TARGET_ERRNO__SUCCESS;
+		return UINT_MAX;
 
 	/* Try user name first */
 	getpwnam_r(str, &pwd, buf, sizeof(buf), &result);
@@ -115,16 +113,22 @@ enum target_errno target__parse_uid(struct target *target)
 		int uid = strtol(str, &endptr, 10);
 
 		if (*endptr != '\0')
-			return TARGET_ERRNO__INVALID_UID;
+			return UINT_MAX;
 
 		getpwuid_r(uid, &pwd, buf, sizeof(buf), &result);
 
 		if (result == NULL)
-			return TARGET_ERRNO__USER_NOT_FOUND;
+			return UINT_MAX;
 	}
 
-	target->uid = result->pw_uid;
-	return TARGET_ERRNO__SUCCESS;
+	return result->pw_uid;
+}
+
+enum target_errno target__parse_uid(struct target *target)
+{
+	target->uid = parse_uid(target->uid_str);
+
+	return target->uid != UINT_MAX ? TARGET_ERRNO__SUCCESS : TARGET_ERRNO__INVALID_UID;
 }
 
 /*
@@ -142,7 +146,6 @@ static const char *target__error_str[] = {
 	"BPF switch overriding UID",
 	"BPF switch overriding THREAD",
 	"Invalid User: %s",
-	"Problems obtaining information for user %s",
 };
 
 int target__strerror(struct target *target, int errnum,
@@ -171,7 +174,6 @@ int target__strerror(struct target *target, int errnum,
 		break;
 
 	case TARGET_ERRNO__INVALID_UID:
-	case TARGET_ERRNO__USER_NOT_FOUND:
 		snprintf(buf, buflen, msg, target->uid_str);
 		break;
 
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 2ee2cc30340f..e082bda990fb 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -48,12 +48,13 @@ enum target_errno {
 
 	/* for target__parse_uid() */
 	TARGET_ERRNO__INVALID_UID,
-	TARGET_ERRNO__USER_NOT_FOUND,
 
 	__TARGET_ERRNO__END,
 };
 
 enum target_errno target__validate(struct target *target);
+
+uid_t parse_uid(const char *str);
 enum target_errno target__parse_uid(struct target *target);
 
 int target__strerror(struct target *target, int errnum, char *buf, size_t buflen);
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 01/10] perf parse-events filter: Use evsel__find_pmu Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 02/10] perf target: Separate parse_uid into its own function Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-06 17:41   ` Namhyung Kim
  2025-06-04 17:45 ` [PATCH v4 04/10] perf record: Switch user option to use BPF filter Ian Rogers
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Add parse_uid_filter filter as a helper to parse_filter, that
constructs a uid filter string. As uid filters don't work with
tracepoint filters, add a is_possible_tp_filter function so the
tracepoint filter isn't attempted for tracepoint evsels.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/parse-events.c | 33 ++++++++++++++++++++++++++++++++-
 tools/perf/util/parse-events.h |  1 +
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d96adf23dc94..7f34e602fc08 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -25,6 +25,7 @@
 #include "pmu.h"
 #include "pmus.h"
 #include "asm/bug.h"
+#include "ui/ui.h"
 #include "util/parse-branch-options.h"
 #include "util/evsel_config.h"
 #include "util/event.h"
@@ -2561,6 +2562,12 @@ foreach_evsel_in_last_glob(struct evlist *evlist,
 	return 0;
 }
 
+/* Will a tracepoint filter work for str or should a BPF filter be used? */
+static bool is_possible_tp_filter(const char *str)
+{
+	return strstr(str, "uid") == NULL;
+}
+
 static int set_filter(struct evsel *evsel, const void *arg)
 {
 	const char *str = arg;
@@ -2573,7 +2580,7 @@ static int set_filter(struct evsel *evsel, const void *arg)
 		return -1;
 	}
 
-	if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT) {
+	if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT && is_possible_tp_filter(str)) {
 		if (evsel__append_tp_filter(evsel, str) < 0) {
 			fprintf(stderr,
 				"not enough memory to hold filter string\n");
@@ -2609,6 +2616,30 @@ int parse_filter(const struct option *opt, const char *str,
 					  (const void *)str);
 }
 
+int parse_uid_filter(struct evlist *evlist, uid_t uid)
+{
+	struct option opt = {
+		.value = &evlist,
+	};
+	char buf[128];
+	int ret;
+
+	snprintf(buf, sizeof(buf), "uid == %d", uid);
+	ret = parse_filter(&opt, buf, /*unset=*/0);
+	if (ret) {
+		if (use_browser >= 1) {
+			/*
+			 * Use ui__warning so a pop up appears above the
+			 * underlying BPF error message.
+			 */
+			ui__warning("Failed to add UID filtering that uses BPF filtering.\n");
+		} else {
+			fprintf(stderr, "Failed to add UID filtering that uses BPF filtering.\n");
+		}
+	}
+	return ret;
+}
+
 static int add_exclude_perf_filter(struct evsel *evsel,
 				   const void *arg __maybe_unused)
 {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ab242f671031..46e5a01be61c 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -45,6 +45,7 @@ static inline int parse_events(struct evlist *evlist, const char *str,
 int parse_event(struct evlist *evlist, const char *str);
 
 int parse_filter(const struct option *opt, const char *str, int unset);
+int parse_uid_filter(struct evlist *evlist, uid_t uid);
 int exclude_perf(const struct option *opt, const char *arg, int unset);
 
 enum parse_events__term_val_type {
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 04/10] perf record: Switch user option to use BPF filter
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (2 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 05/10] perf tests record: Add basic uid filtering test Ian Rogers
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Finding user processes by scanning /proc is inherently racy and
results in perf_event_open failures. Use a BPF filter to drop samples
where the uid doesn't match. Ensure adding the BPF filter forces
system-wide.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-record.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 8059bce85a51..0b566f300569 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -175,6 +175,7 @@ struct record {
 	bool			timestamp_boundary;
 	bool			off_cpu;
 	const char		*filter_action;
+	const char		*uid_str;
 	struct switch_output	switch_output;
 	unsigned long long	samples;
 	unsigned long		output_max_size;	/* = 0: unlimited */
@@ -3513,8 +3514,7 @@ static struct option __record_options[] = {
 		     "or ranges of time to enable events e.g. '-D 10-20,30-40'",
 		     record__parse_event_enable_time),
 	OPT_BOOLEAN(0, "kcore", &record.opts.kcore, "copy /proc/kcore"),
-	OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
-		   "user to profile"),
+	OPT_STRING('u', "uid", &record.uid_str, "user", "user to profile"),
 
 	OPT_CALLBACK_NOOPT('b', "branch-any", &record.opts.branch_stack,
 		     "branch any", "sample any taken branches",
@@ -4256,19 +4256,24 @@ int cmd_record(int argc, const char **argv)
 		ui__warning("%s\n", errbuf);
 	}
 
-	err = target__parse_uid(&rec->opts.target);
-	if (err) {
-		int saved_errno = errno;
+	if (rec->uid_str) {
+		uid_t uid = parse_uid(rec->uid_str);
 
-		target__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
-		ui__error("%s", errbuf);
+		if (uid == UINT_MAX) {
+			ui__error("Invalid User: %s", rec->uid_str);
+			err = -EINVAL;
+			goto out;
+		}
+		err = parse_uid_filter(rec->evlist, uid);
+		if (err)
+			goto out;
 
-		err = -saved_errno;
-		goto out;
+		/* User ID filtering implies system wide. */
+		rec->opts.target.system_wide = true;
 	}
 
-	/* Enable ignoring missing threads when -u/-p option is defined. */
-	rec->opts.ignore_missing_thread = rec->opts.target.uid != UINT_MAX || rec->opts.target.pid;
+	/* Enable ignoring missing threads when -p option is defined. */
+	rec->opts.ignore_missing_thread = rec->opts.target.pid;
 
 	evlist__warn_user_requested_cpus(rec->evlist, rec->opts.target.cpu_list);
 
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 05/10] perf tests record: Add basic uid filtering test
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (3 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 04/10] perf record: Switch user option to use BPF filter Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 06/10] perf top: Switch user option to use BPF filter Ian Rogers
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Based on the system-wide test with changes around how failure is
handled as BPF permissions are a bigger issue than perf event
paranoia.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/record.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 587f62e34414..2022a4f739be 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -231,6 +231,31 @@ test_cgroup() {
   echo "Cgroup sampling test [Success]"
 }
 
+test_uid() {
+  echo "Uid sampling test"
+  if ! perf record -aB --synth=no --uid "$(id -u)" -o "${perfdata}" ${testprog} \
+    > "${script_output}" 2>&1
+  then
+    if grep -q "libbpf.*EPERM" "${script_output}"
+    then
+      echo "Uid sampling [Skipped permissions]"
+      return
+    else
+      echo "Uid sampling [Failed to record]"
+      err=1
+      # cat "${script_output}"
+      return
+    fi
+  fi
+  if ! perf report -i "${perfdata}" -q | grep -q "${testsym}"
+  then
+    echo "Uid sampling [Failed missing output]"
+    err=1
+    return
+  fi
+  echo "Uid sampling test [Success]"
+}
+
 test_leader_sampling() {
   echo "Basic leader sampling test"
   if ! perf record -o "${perfdata}" -e "{cycles,cycles}:Su" -- \
@@ -345,6 +370,7 @@ test_system_wide
 test_workload
 test_branch_counter
 test_cgroup
+test_uid
 test_leader_sampling
 test_topdown_leader_sampling
 test_precise_max
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 06/10] perf top: Switch user option to use BPF filter
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (4 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 05/10] perf tests record: Add basic uid filtering test Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 07/10] perf trace: " Ian Rogers
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Finding user processes by scanning /proc is inherently racy and
results in perf_event_open failures. Use a BPF filter to drop samples
where the uid doesn't match.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-top.c | 22 ++++++++++++----------
 tools/perf/util/top.c    |  4 ++--
 tools/perf/util/top.h    |  1 +
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7b6cde87d2af..051ded5ba9ba 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -643,7 +643,7 @@ static void *display_thread_tui(void *arg)
 	 */
 	evlist__for_each_entry(top->evlist, pos) {
 		struct hists *hists = evsel__hists(pos);
-		hists->uid_filter_str = top->record_opts.target.uid_str;
+		hists->uid_filter_str = top->uid_str;
 	}
 
 	ret = evlist__tui_browse_hists(top->evlist, help, &hbt, top->min_percent,
@@ -1571,7 +1571,7 @@ int cmd_top(int argc, const char **argv)
 		    "Add prefix to source file path names in programs (with --prefix-strip)"),
 	OPT_STRING(0, "prefix-strip", &annotate_opts.prefix_strip, "N",
 		    "Strip first N entries of source file path name in programs (with --prefix)"),
-	OPT_STRING('u', "uid", &target->uid_str, "user", "user to profile"),
+	OPT_STRING('u', "uid", &top.uid_str, "user", "user to profile"),
 	OPT_CALLBACK(0, "percent-limit", &top, "percent",
 		     "Don't show entries under that percent", parse_percent_limit),
 	OPT_CALLBACK(0, "percentage", NULL, "relative|absolute",
@@ -1762,15 +1762,17 @@ int cmd_top(int argc, const char **argv)
 		ui__warning("%s\n", errbuf);
 	}
 
-	status = target__parse_uid(target);
-	if (status) {
-		int saved_errno = errno;
-
-		target__strerror(target, status, errbuf, BUFSIZ);
-		ui__error("%s\n", errbuf);
+	if (top.uid_str) {
+		uid_t uid = parse_uid(top.uid_str);
 
-		status = -saved_errno;
-		goto out_delete_evlist;
+		if (uid == UINT_MAX) {
+			ui__error("Invalid User: %s", top.uid_str);
+			status = -EINVAL;
+			goto out_delete_evlist;
+		}
+		status = parse_uid_filter(top.evlist, uid);
+		if (status)
+			goto out_delete_evlist;
 	}
 
 	if (target__none(target))
diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index 4db3d1bd686c..b06e10a116bb 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -88,9 +88,9 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
 	else if (target->tid)
 		ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %s",
 				target->tid);
-	else if (target->uid_str != NULL)
+	else if (top->uid_str != NULL)
 		ret += SNPRINTF(bf + ret, size - ret, " (uid: %s",
-				target->uid_str);
+				top->uid_str);
 	else
 		ret += SNPRINTF(bf + ret, size - ret, " (all");
 
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index 4c5588dbb131..04ff926846be 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -48,6 +48,7 @@ struct perf_top {
 	const char	   *sym_filter;
 	float		   min_percent;
 	unsigned int	   nr_threads_synthesize;
+	const char	   *uid_str;
 
 	struct {
 		struct ordered_events	*in;
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 07/10] perf trace: Switch user option to use BPF filter
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (5 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 06/10] perf top: Switch user option to use BPF filter Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 08/10] perf bench evlist-open-close: " Ian Rogers
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Finding user processes by scanning /proc is inherently racy and
results in perf_event_open failures. Use a BPF filter to drop samples
where the uid doesn't match. Ensure adding the BPF filter forces
system-wide.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-trace.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 2ab1b8e05ad3..4bb062b96f51 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -236,6 +236,7 @@ struct trace {
 		struct ordered_events	data;
 		u64			last;
 	} oe;
+	const char		*uid_str;
 };
 
 static void trace__load_vmlinux_btf(struct trace *trace __maybe_unused)
@@ -4412,8 +4413,8 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 		evlist__add(evlist, pgfault_min);
 	}
 
-	/* Enable ignoring missing threads when -u/-p option is defined. */
-	trace->opts.ignore_missing_thread = trace->opts.target.uid != UINT_MAX || trace->opts.target.pid;
+	/* Enable ignoring missing threads when -p option is defined. */
+	trace->opts.ignore_missing_thread = trace->opts.target.pid;
 
 	if (trace->sched &&
 	    evlist__add_newtp(evlist, "sched", "sched_stat_runtime", trace__sched_stat_runtime))
@@ -5445,8 +5446,7 @@ int cmd_trace(int argc, const char **argv)
 		    "child tasks do not inherit counters"),
 	OPT_CALLBACK('m', "mmap-pages", &trace.opts.mmap_pages, "pages",
 		     "number of mmap data pages", evlist__parse_mmap_pages),
-	OPT_STRING('u', "uid", &trace.opts.target.uid_str, "user",
-		   "user to profile"),
+	OPT_STRING('u', "uid", &trace.uid_str, "user", "user to profile"),
 	OPT_CALLBACK(0, "duration", &trace, "float",
 		     "show only events with duration > N.M ms",
 		     trace__set_duration),
@@ -5804,11 +5804,19 @@ int cmd_trace(int argc, const char **argv)
 		goto out_close;
 	}
 
-	err = target__parse_uid(&trace.opts.target);
-	if (err) {
-		target__strerror(&trace.opts.target, err, bf, sizeof(bf));
-		fprintf(trace.output, "%s", bf);
-		goto out_close;
+	if (trace.uid_str) {
+		uid_t uid = parse_uid(trace.uid_str);
+
+		if (uid == UINT_MAX) {
+			ui__error("Invalid User: %s", trace.uid_str);
+			err = -EINVAL;
+			goto out_close;
+		}
+		err = parse_uid_filter(trace.evlist, uid);
+		if (err)
+			goto out_close;
+
+		trace.opts.target.system_wide = true;
 	}
 
 	if (!argc && target__none(&trace.opts.target))
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 08/10] perf bench evlist-open-close: Switch user option to use BPF filter
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (6 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 07/10] perf trace: " Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 09/10] perf target: Remove uid from target Ian Rogers
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Finding user processes by scanning /proc is inherently racy and
results in perf_event_open failures. Use a BPF filter to drop samples
where the uid doesn't match.

Signed-off-by: Ian Rogers <irogers@google.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.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 09/10] perf target: Remove uid from target
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (7 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 08/10] perf bench evlist-open-close: " Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-04 17:45 ` [PATCH v4 10/10] perf thread_map: Remove uid options Ian Rogers
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Gathering threads with a uid by scanning /proc is inherently racy
leading to perf_event_open failures that quit perf. All users of the
functionality now use BPF filters, so remove uid and uid_str from
target.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-ftrace.c                 |  1 -
 tools/perf/builtin-kvm.c                    |  2 -
 tools/perf/builtin-stat.c                   |  4 +-
 tools/perf/builtin-trace.c                  |  1 -
 tools/perf/tests/backward-ring-buffer.c     |  1 -
 tools/perf/tests/event-times.c              |  4 +-
 tools/perf/tests/openat-syscall-tp-fields.c |  1 -
 tools/perf/tests/perf-record.c              |  1 -
 tools/perf/tests/task-exit.c                |  1 -
 tools/perf/util/bpf-filter.c                |  2 +-
 tools/perf/util/evlist.c                    |  3 +-
 tools/perf/util/target.c                    | 46 +--------------------
 tools/perf/util/target.h                    | 12 +-----
 13 files changed, 6 insertions(+), 73 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index bba36ebc2aa7..3a253a1b9f45 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -1663,7 +1663,6 @@ int cmd_ftrace(int argc, const char **argv)
 	int (*cmd_func)(struct perf_ftrace *) = NULL;
 	struct perf_ftrace ftrace = {
 		.tracer = DEFAULT_TRACER,
-		.target = { .uid = UINT_MAX, },
 	};
 	const struct option common_options[] = {
 	OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 67fd2b006b0b..d75bd3684980 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1871,8 +1871,6 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
 	kvm->opts.user_interval = 1;
 	kvm->opts.mmap_pages = 512;
 	kvm->opts.target.uses_mmap = false;
-	kvm->opts.target.uid_str = NULL;
-	kvm->opts.target.uid = UINT_MAX;
 
 	symbol__init(NULL);
 	disable_buildid_cache();
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index bf0e5e12d992..50fc53adb7e4 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -108,9 +108,7 @@ static struct parse_events_option_args parse_events_option_args = {
 
 static bool all_counters_use_bpf = true;
 
-static struct target target = {
-	.uid	= UINT_MAX,
-};
+static struct target target;
 
 static volatile sig_atomic_t	child_pid			= -1;
 static int			detailed_run			=  0;
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 4bb062b96f51..bf9b5d0630d3 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -5399,7 +5399,6 @@ int cmd_trace(int argc, const char **argv)
 	struct trace trace = {
 		.opts = {
 			.target = {
-				.uid	   = UINT_MAX,
 				.uses_mmap = true,
 			},
 			.user_freq     = UINT_MAX,
diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index 79a980b1e786..c5e7999f2817 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -91,7 +91,6 @@ static int test__backward_ring_buffer(struct test_suite *test __maybe_unused, in
 	struct parse_events_error parse_error;
 	struct record_opts opts = {
 		.target = {
-			.uid = UINT_MAX,
 			.uses_mmap = true,
 		},
 		.freq	      = 0,
diff --git a/tools/perf/tests/event-times.c b/tools/perf/tests/event-times.c
index deefe5003bfc..2148024b4f4a 100644
--- a/tools/perf/tests/event-times.c
+++ b/tools/perf/tests/event-times.c
@@ -17,9 +17,7 @@
 static int attach__enable_on_exec(struct evlist *evlist)
 {
 	struct evsel *evsel = evlist__last(evlist);
-	struct target target = {
-		.uid = UINT_MAX,
-	};
+	struct target target = {};
 	const char *argv[] = { "true", NULL, };
 	char sbuf[STRERR_BUFSIZE];
 	int err;
diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c
index 0ef4ba7c1571..2a139d2781a8 100644
--- a/tools/perf/tests/openat-syscall-tp-fields.c
+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -28,7 +28,6 @@ static int test__syscall_openat_tp_fields(struct test_suite *test __maybe_unused
 {
 	struct record_opts opts = {
 		.target = {
-			.uid = UINT_MAX,
 			.uses_mmap = true,
 		},
 		.no_buffering = true,
diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
index 0958c7c8995f..0b3c37e66871 100644
--- a/tools/perf/tests/perf-record.c
+++ b/tools/perf/tests/perf-record.c
@@ -45,7 +45,6 @@ static int test__PERF_RECORD(struct test_suite *test __maybe_unused, int subtest
 {
 	struct record_opts opts = {
 		.target = {
-			.uid = UINT_MAX,
 			.uses_mmap = true,
 		},
 		.no_buffering = true,
diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index 8e328bbd509d..4053ff2813bb 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -46,7 +46,6 @@ static int test__task_exit(struct test_suite *test __maybe_unused, int subtest _
 	struct evsel *evsel;
 	struct evlist *evlist;
 	struct target target = {
-		.uid		= UINT_MAX,
 		.uses_mmap	= true,
 	};
 	const char *argv[] = { "true", NULL };
diff --git a/tools/perf/util/bpf-filter.c b/tools/perf/util/bpf-filter.c
index 92e2f054b45e..d0e013eeb0f7 100644
--- a/tools/perf/util/bpf-filter.c
+++ b/tools/perf/util/bpf-filter.c
@@ -450,7 +450,7 @@ int perf_bpf_filter__prepare(struct evsel *evsel, struct target *target)
 	struct bpf_program *prog;
 	struct bpf_link *link;
 	struct perf_bpf_filter_entry *entry;
-	bool needs_idx_hash = !target__has_cpu(target) && !target->uid_str;
+	bool needs_idx_hash = !target__has_cpu(target);
 
 	entry = calloc(MAX_FILTERS, sizeof(*entry));
 	if (entry == NULL)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index dcd1130502df..bed91bc88510 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1006,8 +1006,7 @@ int evlist__create_maps(struct evlist *evlist, struct target *target)
 	 * per-thread data. thread_map__new_str will call
 	 * thread_map__new_all_cpus to enumerate all threads.
 	 */
-	threads = thread_map__new_str(target->pid, target->tid, target->uid,
-				      all_threads);
+	threads = thread_map__new_str(target->pid, target->tid, UINT_MAX, all_threads);
 
 	if (!threads)
 		return -1;
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index f3ad59ccfa99..8cf71bea295a 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -28,20 +28,6 @@ enum target_errno target__validate(struct target *target)
 			ret = TARGET_ERRNO__PID_OVERRIDE_CPU;
 	}
 
-	/* UID and PID are mutually exclusive */
-	if (target->tid && target->uid_str) {
-		target->uid_str = NULL;
-		if (ret == TARGET_ERRNO__SUCCESS)
-			ret = TARGET_ERRNO__PID_OVERRIDE_UID;
-	}
-
-	/* UID and CPU are mutually exclusive */
-	if (target->uid_str && target->cpu_list) {
-		target->cpu_list = NULL;
-		if (ret == TARGET_ERRNO__SUCCESS)
-			ret = TARGET_ERRNO__UID_OVERRIDE_CPU;
-	}
-
 	/* PID and SYSTEM are mutually exclusive */
 	if (target->tid && target->system_wide) {
 		target->system_wide = false;
@@ -49,13 +35,6 @@ enum target_errno target__validate(struct target *target)
 			ret = TARGET_ERRNO__PID_OVERRIDE_SYSTEM;
 	}
 
-	/* UID and SYSTEM are mutually exclusive */
-	if (target->uid_str && target->system_wide) {
-		target->system_wide = false;
-		if (ret == TARGET_ERRNO__SUCCESS)
-			ret = TARGET_ERRNO__UID_OVERRIDE_SYSTEM;
-	}
-
 	/* BPF and CPU are mutually exclusive */
 	if (target->bpf_str && target->cpu_list) {
 		target->cpu_list = NULL;
@@ -70,13 +49,6 @@ enum target_errno target__validate(struct target *target)
 			ret = TARGET_ERRNO__BPF_OVERRIDE_PID;
 	}
 
-	/* BPF and UID are mutually exclusive */
-	if (target->bpf_str && target->uid_str) {
-		target->uid_str = NULL;
-		if (ret == TARGET_ERRNO__SUCCESS)
-			ret = TARGET_ERRNO__BPF_OVERRIDE_UID;
-	}
-
 	/* BPF and THREADS are mutually exclusive */
 	if (target->bpf_str && target->per_thread) {
 		target->per_thread = false;
@@ -124,31 +96,19 @@ uid_t parse_uid(const char *str)
 	return result->pw_uid;
 }
 
-enum target_errno target__parse_uid(struct target *target)
-{
-	target->uid = parse_uid(target->uid_str);
-
-	return target->uid != UINT_MAX ? TARGET_ERRNO__SUCCESS : TARGET_ERRNO__INVALID_UID;
-}
-
 /*
  * This must have a same ordering as the enum target_errno.
  */
 static const char *target__error_str[] = {
 	"PID/TID switch overriding CPU",
-	"PID/TID switch overriding UID",
-	"UID switch overriding CPU",
 	"PID/TID switch overriding SYSTEM",
-	"UID switch overriding SYSTEM",
 	"SYSTEM/CPU switch overriding PER-THREAD",
 	"BPF switch overriding CPU",
 	"BPF switch overriding PID/TID",
-	"BPF switch overriding UID",
 	"BPF switch overriding THREAD",
-	"Invalid User: %s",
 };
 
-int target__strerror(struct target *target, int errnum,
+int target__strerror(struct target *target __maybe_unused, int errnum,
 			  char *buf, size_t buflen)
 {
 	int idx;
@@ -173,10 +133,6 @@ int target__strerror(struct target *target, int errnum,
 		snprintf(buf, buflen, "%s", msg);
 		break;
 
-	case TARGET_ERRNO__INVALID_UID:
-		snprintf(buf, buflen, msg, target->uid_str);
-		break;
-
 	default:
 		/* cannot reach here */
 		break;
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index e082bda990fb..84ebb9c940c6 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -9,9 +9,7 @@ struct target {
 	const char   *pid;
 	const char   *tid;
 	const char   *cpu_list;
-	const char   *uid_str;
 	const char   *bpf_str;
-	uid_t	     uid;
 	bool	     system_wide;
 	bool	     uses_mmap;
 	bool	     default_per_cpu;
@@ -36,32 +34,24 @@ enum target_errno {
 
 	/* for target__validate() */
 	TARGET_ERRNO__PID_OVERRIDE_CPU	= __TARGET_ERRNO__START,
-	TARGET_ERRNO__PID_OVERRIDE_UID,
-	TARGET_ERRNO__UID_OVERRIDE_CPU,
 	TARGET_ERRNO__PID_OVERRIDE_SYSTEM,
-	TARGET_ERRNO__UID_OVERRIDE_SYSTEM,
 	TARGET_ERRNO__SYSTEM_OVERRIDE_THREAD,
 	TARGET_ERRNO__BPF_OVERRIDE_CPU,
 	TARGET_ERRNO__BPF_OVERRIDE_PID,
-	TARGET_ERRNO__BPF_OVERRIDE_UID,
 	TARGET_ERRNO__BPF_OVERRIDE_THREAD,
 
-	/* for target__parse_uid() */
-	TARGET_ERRNO__INVALID_UID,
-
 	__TARGET_ERRNO__END,
 };
 
 enum target_errno target__validate(struct target *target);
 
 uid_t parse_uid(const char *str);
-enum target_errno target__parse_uid(struct target *target);
 
 int target__strerror(struct target *target, int errnum, char *buf, size_t buflen);
 
 static inline bool target__has_task(struct target *target)
 {
-	return target->tid || target->pid || target->uid_str;
+	return target->tid || target->pid;
 }
 
 static inline bool target__has_cpu(struct target *target)
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 10/10] perf thread_map: Remove uid options
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (8 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 09/10] perf target: Remove uid from target Ian Rogers
@ 2025-06-04 17:45 ` Ian Rogers
  2025-06-06  2:44 ` [PATCH v4 00/10] Move uid filtering to BPF filters Namhyung Kim
  2025-06-10 18:38 ` Namhyung Kim
  11 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2025-06-04 17:45 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Dapeng Mi,
	Thomas Richter, Veronika Molnarova, Chun-Tse Shao, Leo Yan,
	Hao Ge, Howard Chu, Weilin Wang, Levi Yun, Dr. David Alan Gilbert,
	Gautam Menghani, Tengda Wu, linux-perf-users, linux-kernel, bpf

Now the target doesn't have a uid, it is handled through BPF filters,
remove the uid options to thread_map creation. Tidy up the functions
used in tests to avoid passing unused arguments.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/event-times.c             |  4 +--
 tools/perf/tests/keep-tracking.c           |  2 +-
 tools/perf/tests/mmap-basic.c              |  2 +-
 tools/perf/tests/openat-syscall-all-cpus.c |  2 +-
 tools/perf/tests/openat-syscall.c          |  2 +-
 tools/perf/tests/perf-time-to-tsc.c        |  2 +-
 tools/perf/tests/switch-tracking.c         |  2 +-
 tools/perf/tests/thread-map.c              |  2 +-
 tools/perf/util/evlist.c                   |  2 +-
 tools/perf/util/python.c                   | 10 +++----
 tools/perf/util/thread_map.c               | 32 ++--------------------
 tools/perf/util/thread_map.h               |  6 ++--
 12 files changed, 20 insertions(+), 48 deletions(-)

diff --git a/tools/perf/tests/event-times.c b/tools/perf/tests/event-times.c
index 2148024b4f4a..ae3b98bb42cf 100644
--- a/tools/perf/tests/event-times.c
+++ b/tools/perf/tests/event-times.c
@@ -62,7 +62,7 @@ static int attach__current_disabled(struct evlist *evlist)
 
 	pr_debug("attaching to current thread as disabled\n");
 
-	threads = thread_map__new(-1, getpid(), UINT_MAX);
+	threads = thread_map__new_by_tid(getpid());
 	if (threads == NULL) {
 		pr_debug("thread_map__new\n");
 		return -1;
@@ -88,7 +88,7 @@ static int attach__current_enabled(struct evlist *evlist)
 
 	pr_debug("attaching to current thread as enabled\n");
 
-	threads = thread_map__new(-1, getpid(), UINT_MAX);
+	threads = thread_map__new_by_tid(getpid());
 	if (threads == NULL) {
 		pr_debug("failed to call thread_map__new\n");
 		return -1;
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 5a3b2bed07f3..eafb49eb0b56 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -78,7 +78,7 @@ static int test__keep_tracking(struct test_suite *test __maybe_unused, int subte
 	int found, err = -1;
 	const char *comm;
 
-	threads = thread_map__new(-1, getpid(), UINT_MAX);
+	threads = thread_map__new_by_tid(getpid());
 	CHECK_NOT_NULL__(threads);
 
 	cpus = perf_cpu_map__new_online_cpus();
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index bd2106628b34..04b547c6bdbe 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -46,7 +46,7 @@ static int test__basic_mmap(struct test_suite *test __maybe_unused, int subtest
 	char sbuf[STRERR_BUFSIZE];
 	struct mmap *md;
 
-	threads = thread_map__new(-1, getpid(), UINT_MAX);
+	threads = thread_map__new_by_tid(getpid());
 	if (threads == NULL) {
 		pr_debug("thread_map__new\n");
 		return -1;
diff --git a/tools/perf/tests/openat-syscall-all-cpus.c b/tools/perf/tests/openat-syscall-all-cpus.c
index fb114118c876..3644d6f52c07 100644
--- a/tools/perf/tests/openat-syscall-all-cpus.c
+++ b/tools/perf/tests/openat-syscall-all-cpus.c
@@ -28,7 +28,7 @@ static int test__openat_syscall_event_on_all_cpus(struct test_suite *test __mayb
 	struct evsel *evsel;
 	unsigned int nr_openat_calls = 111, i;
 	cpu_set_t cpu_set;
-	struct perf_thread_map *threads = thread_map__new(-1, getpid(), UINT_MAX);
+	struct perf_thread_map *threads = thread_map__new_by_tid(getpid());
 	char sbuf[STRERR_BUFSIZE];
 	char errbuf[BUFSIZ];
 
diff --git a/tools/perf/tests/openat-syscall.c b/tools/perf/tests/openat-syscall.c
index 131b62271bfa..b54cbe5f1808 100644
--- a/tools/perf/tests/openat-syscall.c
+++ b/tools/perf/tests/openat-syscall.c
@@ -20,7 +20,7 @@ static int test__openat_syscall_event(struct test_suite *test __maybe_unused,
 	int err = TEST_FAIL, fd;
 	struct evsel *evsel;
 	unsigned int nr_openat_calls = 111, i;
-	struct perf_thread_map *threads = thread_map__new(-1, getpid(), UINT_MAX);
+	struct perf_thread_map *threads = thread_map__new_by_tid(getpid());
 	char sbuf[STRERR_BUFSIZE];
 	char errbuf[BUFSIZ];
 
diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
index d3e40fa5482c..d4437410c99f 100644
--- a/tools/perf/tests/perf-time-to-tsc.c
+++ b/tools/perf/tests/perf-time-to-tsc.c
@@ -90,7 +90,7 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
 	struct mmap *md;
 
 
-	threads = thread_map__new(-1, getpid(), UINT_MAX);
+	threads = thread_map__new_by_tid(getpid());
 	CHECK_NOT_NULL__(threads);
 
 	cpus = perf_cpu_map__new_online_cpus();
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index 6b3aac283c37..5be294014d3b 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -351,7 +351,7 @@ static int test__switch_tracking(struct test_suite *test __maybe_unused, int sub
 	const char *comm;
 	int err = -1;
 
-	threads = thread_map__new(-1, getpid(), UINT_MAX);
+	threads = thread_map__new_by_tid(getpid());
 	if (!threads) {
 		pr_debug("thread_map__new failed!\n");
 		goto out_err;
diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
index 1fe521466bf4..54209592168d 100644
--- a/tools/perf/tests/thread-map.c
+++ b/tools/perf/tests/thread-map.c
@@ -115,7 +115,7 @@ static int test__thread_map_remove(struct test_suite *test __maybe_unused, int s
 	TEST_ASSERT_VAL("failed to allocate map string",
 			asprintf(&str, "%d,%d", getpid(), getppid()) >= 0);
 
-	threads = thread_map__new_str(str, NULL, 0, false);
+	threads = thread_map__new_str(str, /*tid=*/NULL, /*all_threads=*/false);
 	free(str);
 
 	TEST_ASSERT_VAL("failed to allocate thread_map",
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index bed91bc88510..5664ebf6bbc6 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1006,7 +1006,7 @@ int evlist__create_maps(struct evlist *evlist, struct target *target)
 	 * per-thread data. thread_map__new_str will call
 	 * thread_map__new_all_cpus to enumerate all threads.
 	 */
-	threads = thread_map__new_str(target->pid, target->tid, UINT_MAX, all_threads);
+	threads = thread_map__new_str(target->pid, target->tid, all_threads);
 
 	if (!threads)
 		return -1;
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 321c333877fa..82666bcd2eda 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -566,14 +566,14 @@ struct pyrf_thread_map {
 static int pyrf_thread_map__init(struct pyrf_thread_map *pthreads,
 				 PyObject *args, PyObject *kwargs)
 {
-	static char *kwlist[] = { "pid", "tid", "uid", NULL };
-	int pid = -1, tid = -1, uid = UINT_MAX;
+	static char *kwlist[] = { "pid", "tid", NULL };
+	int pid = -1, tid = -1;
 
-	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|iii",
-					 kwlist, &pid, &tid, &uid))
+	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii",
+					 kwlist, &pid, &tid))
 		return -1;
 
-	pthreads->threads = thread_map__new(pid, tid, uid);
+	pthreads->threads = thread_map__new(pid, tid);
 	if (pthreads->threads == NULL)
 		return -1;
 	return 0;
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index b5f12390c355..ca193c1374ed 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -72,7 +72,7 @@ struct perf_thread_map *thread_map__new_by_tid(pid_t tid)
 	return threads;
 }
 
-static struct perf_thread_map *__thread_map__new_all_cpus(uid_t uid)
+static struct perf_thread_map *thread_map__new_all_cpus(void)
 {
 	DIR *proc;
 	int max_threads = 32, items, i;
@@ -98,15 +98,6 @@ static struct perf_thread_map *__thread_map__new_all_cpus(uid_t uid)
 		if (*end) /* only interested in proper numerical dirents */
 			continue;
 
-		snprintf(path, sizeof(path), "/proc/%s", dirent->d_name);
-
-		if (uid != UINT_MAX) {
-			struct stat st;
-
-			if (stat(path, &st) != 0 || st.st_uid != uid)
-				continue;
-		}
-
 		snprintf(path, sizeof(path), "/proc/%d/task", pid);
 		items = scandir(path, &namelist, filter, NULL);
 		if (items <= 0) {
@@ -157,24 +148,11 @@ static struct perf_thread_map *__thread_map__new_all_cpus(uid_t uid)
 	goto out_closedir;
 }
 
-struct perf_thread_map *thread_map__new_all_cpus(void)
-{
-	return __thread_map__new_all_cpus(UINT_MAX);
-}
-
-struct perf_thread_map *thread_map__new_by_uid(uid_t uid)
-{
-	return __thread_map__new_all_cpus(uid);
-}
-
-struct perf_thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
+struct perf_thread_map *thread_map__new(pid_t pid, pid_t tid)
 {
 	if (pid != -1)
 		return thread_map__new_by_pid(pid);
 
-	if (tid == -1 && uid != UINT_MAX)
-		return thread_map__new_by_uid(uid);
-
 	return thread_map__new_by_tid(tid);
 }
 
@@ -289,15 +267,11 @@ struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str)
 	goto out;
 }
 
-struct perf_thread_map *thread_map__new_str(const char *pid, const char *tid,
-				       uid_t uid, bool all_threads)
+struct perf_thread_map *thread_map__new_str(const char *pid, const char *tid, bool all_threads)
 {
 	if (pid)
 		return thread_map__new_by_pid_str(pid);
 
-	if (!tid && uid != UINT_MAX)
-		return thread_map__new_by_uid(uid);
-
 	if (all_threads)
 		return thread_map__new_all_cpus();
 
diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
index 00ec05fc1656..fc16d87f32fb 100644
--- a/tools/perf/util/thread_map.h
+++ b/tools/perf/util/thread_map.h
@@ -11,13 +11,11 @@ struct perf_record_thread_map;
 struct perf_thread_map *thread_map__new_dummy(void);
 struct perf_thread_map *thread_map__new_by_pid(pid_t pid);
 struct perf_thread_map *thread_map__new_by_tid(pid_t tid);
-struct perf_thread_map *thread_map__new_by_uid(uid_t uid);
-struct perf_thread_map *thread_map__new_all_cpus(void);
-struct perf_thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid);
+struct perf_thread_map *thread_map__new(pid_t pid, pid_t tid);
 struct perf_thread_map *thread_map__new_event(struct perf_record_thread_map *event);
 
 struct perf_thread_map *thread_map__new_str(const char *pid,
-		const char *tid, uid_t uid, bool all_threads);
+		const char *tid, bool all_threads);
 
 struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str);
 
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 00/10] Move uid filtering to BPF filters
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (9 preceding siblings ...)
  2025-06-04 17:45 ` [PATCH v4 10/10] perf thread_map: Remove uid options Ian Rogers
@ 2025-06-06  2:44 ` Namhyung Kim
  2025-06-10 18:38 ` Namhyung Kim
  11 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2025-06-06  2:44 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, James Clark, Dapeng Mi, Thomas Richter,
	Veronika Molnarova, Chun-Tse Shao, Leo Yan, Hao Ge, Howard Chu,
	Weilin Wang, Levi Yun, Dr. David Alan Gilbert, Gautam Menghani,
	Tengda Wu, linux-perf-users, linux-kernel, bpf

Hi Ian,

On Wed, Jun 04, 2025 at 10:45:34AM -0700, Ian Rogers wrote:
> Rather than scanning /proc and skipping PIDs based on their UIDs, use
> BPF filters for uid filtering. The /proc scanning in thread_map is
> racy as the PID may exit before the perf_event_open causing perf to
> abort. BPF UID filters are more robust as they avoid the race. The
> /proc scanning also misses processes starting after the perf
> command. Add a helper for commands that support UID filtering and wire
> up. Remove the non-BPF UID filtering support given it doesn't work.
> 
> v4: Add a warning message on top of Namhyung's BPF filter error message:
> https://lore.kernel.org/lkml/20250604054234.23608-1-namhyung@kernel.org/
>     in the parse_uid_filter helper. In TUI the warning is shown then
>     the BPF error shown, with stdio the warning appears below the BPF
>     errors.
> 
> v3: Add lengthier commit messages as requested by Arnaldo. Rebase on
>     tmp.perf-tools-next.
> 
> v2: Add a perf record uid test (Namhyung) and force setting
>     system-wide for perf trace and perf record (Namhyung). Ensure the
>     uid filter isn't set on tracepoint evsels.
> 
> v1: https://lore.kernel.org/lkml/20250111190143.1029906-1-irogers@google.com/
> 
> Ian Rogers (10):
>   perf parse-events filter: Use evsel__find_pmu
>   perf target: Separate parse_uid into its own function
>   perf parse-events: Add parse_uid_filter helper
>   perf record: Switch user option to use BPF filter
>   perf tests record: Add basic uid filtering test
>   perf top: Switch user option to use BPF filter
>   perf trace: Switch user option to use BPF filter
>   perf bench evlist-open-close: Switch user option to use BPF filter
>   perf target: Remove uid from target
>   perf thread_map: Remove uid options

I've noticed two things.

* it takes a quite long time to load the BPF filter.  Not sure what's
  the issue, but maybe annoying for users.

* normal users cannot use BPF filter even if the BPF is loaded and
  pinned already.  It works fine for perf record.

I don't think the issues are from this change though.

Thanks,
Namhyung

> 
>  tools/perf/bench/evlist-open-close.c        | 36 ++++++++------
>  tools/perf/builtin-ftrace.c                 |  1 -
>  tools/perf/builtin-kvm.c                    |  2 -
>  tools/perf/builtin-record.c                 | 27 ++++++-----
>  tools/perf/builtin-stat.c                   |  4 +-
>  tools/perf/builtin-top.c                    | 22 +++++----
>  tools/perf/builtin-trace.c                  | 27 +++++++----
>  tools/perf/tests/backward-ring-buffer.c     |  1 -
>  tools/perf/tests/event-times.c              |  8 ++-
>  tools/perf/tests/keep-tracking.c            |  2 +-
>  tools/perf/tests/mmap-basic.c               |  2 +-
>  tools/perf/tests/openat-syscall-all-cpus.c  |  2 +-
>  tools/perf/tests/openat-syscall-tp-fields.c |  1 -
>  tools/perf/tests/openat-syscall.c           |  2 +-
>  tools/perf/tests/perf-record.c              |  1 -
>  tools/perf/tests/perf-time-to-tsc.c         |  2 +-
>  tools/perf/tests/shell/record.sh            | 26 ++++++++++
>  tools/perf/tests/switch-tracking.c          |  2 +-
>  tools/perf/tests/task-exit.c                |  1 -
>  tools/perf/tests/thread-map.c               |  2 +-
>  tools/perf/util/bpf-filter.c                |  2 +-
>  tools/perf/util/evlist.c                    |  3 +-
>  tools/perf/util/parse-events.c              | 47 +++++++++++++-----
>  tools/perf/util/parse-events.h              |  1 +
>  tools/perf/util/python.c                    | 10 ++--
>  tools/perf/util/target.c                    | 54 +++------------------
>  tools/perf/util/target.h                    | 15 ++----
>  tools/perf/util/thread_map.c                | 32 ++----------
>  tools/perf/util/thread_map.h                |  6 +--
>  tools/perf/util/top.c                       |  4 +-
>  tools/perf/util/top.h                       |  1 +
>  31 files changed, 164 insertions(+), 182 deletions(-)
> 
> -- 
> 2.50.0.rc0.604.gd4ff7b7c86-goog
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper
  2025-06-04 17:45 ` [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper Ian Rogers
@ 2025-06-06 17:41   ` Namhyung Kim
  2025-06-06 18:13     ` Ian Rogers
  0 siblings, 1 reply; 16+ messages in thread
From: Namhyung Kim @ 2025-06-06 17:41 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, James Clark, Dapeng Mi, Thomas Richter,
	Veronika Molnarova, Chun-Tse Shao, Leo Yan, Hao Ge, Howard Chu,
	Weilin Wang, Levi Yun, Dr. David Alan Gilbert, Gautam Menghani,
	Tengda Wu, linux-perf-users, linux-kernel, bpf

Hi Ian,

On Wed, Jun 04, 2025 at 10:45:37AM -0700, Ian Rogers wrote:
> Add parse_uid_filter filter as a helper to parse_filter, that
> constructs a uid filter string. As uid filters don't work with
> tracepoint filters, add a is_possible_tp_filter function so the
> tracepoint filter isn't attempted for tracepoint evsels.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/parse-events.c | 33 ++++++++++++++++++++++++++++++++-
>  tools/perf/util/parse-events.h |  1 +
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index d96adf23dc94..7f34e602fc08 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -25,6 +25,7 @@
>  #include "pmu.h"
>  #include "pmus.h"
>  #include "asm/bug.h"
> +#include "ui/ui.h"
>  #include "util/parse-branch-options.h"
>  #include "util/evsel_config.h"
>  #include "util/event.h"
> @@ -2561,6 +2562,12 @@ foreach_evsel_in_last_glob(struct evlist *evlist,
>  	return 0;
>  }
>  
> +/* Will a tracepoint filter work for str or should a BPF filter be used? */
> +static bool is_possible_tp_filter(const char *str)
> +{
> +	return strstr(str, "uid") == NULL;
> +}
> +
>  static int set_filter(struct evsel *evsel, const void *arg)
>  {
>  	const char *str = arg;
> @@ -2573,7 +2580,7 @@ static int set_filter(struct evsel *evsel, const void *arg)
>  		return -1;
>  	}
>  
> -	if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT) {
> +	if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT && is_possible_tp_filter(str)) {
>  		if (evsel__append_tp_filter(evsel, str) < 0) {
>  			fprintf(stderr,
>  				"not enough memory to hold filter string\n");
> @@ -2609,6 +2616,30 @@ int parse_filter(const struct option *opt, const char *str,
>  					  (const void *)str);
>  }
>  
> +int parse_uid_filter(struct evlist *evlist, uid_t uid)

It failed to build on alpine 3.18.

util/parse-events.h:48:45: error: unknown type name 'uid_t'                     
   48 | int parse_uid_filter(struct evlist *evlist, uid_t uid);                 
      |                                             ^~~~~

I'll add this.

Thanks,
Namhyung


---8<---
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ab242f6710313993..931780911e071ec6 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -11,6 +11,7 @@
 #include <linux/perf_event.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/types.h>
 
 struct evsel;
 struct evlist;



> +{
> +	struct option opt = {
> +		.value = &evlist,
> +	};
> +	char buf[128];
> +	int ret;
> +
> +	snprintf(buf, sizeof(buf), "uid == %d", uid);
> +	ret = parse_filter(&opt, buf, /*unset=*/0);
> +	if (ret) {
> +		if (use_browser >= 1) {
> +			/*
> +			 * Use ui__warning so a pop up appears above the
> +			 * underlying BPF error message.
> +			 */
> +			ui__warning("Failed to add UID filtering that uses BPF filtering.\n");
> +		} else {
> +			fprintf(stderr, "Failed to add UID filtering that uses BPF filtering.\n");
> +		}
> +	}
> +	return ret;
> +}
> +
>  static int add_exclude_perf_filter(struct evsel *evsel,
>  				   const void *arg __maybe_unused)
>  {
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index ab242f671031..46e5a01be61c 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -45,6 +45,7 @@ static inline int parse_events(struct evlist *evlist, const char *str,
>  int parse_event(struct evlist *evlist, const char *str);
>  
>  int parse_filter(const struct option *opt, const char *str, int unset);
> +int parse_uid_filter(struct evlist *evlist, uid_t uid);
>  int exclude_perf(const struct option *opt, const char *arg, int unset);
>  
>  enum parse_events__term_val_type {
> -- 
> 2.50.0.rc0.604.gd4ff7b7c86-goog
> 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper
  2025-06-06 17:41   ` Namhyung Kim
@ 2025-06-06 18:13     ` Ian Rogers
  2025-06-06 20:01       ` Namhyung Kim
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Rogers @ 2025-06-06 18:13 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, James Clark, Dapeng Mi, Thomas Richter,
	Veronika Molnarova, Chun-Tse Shao, Leo Yan, Hao Ge, Howard Chu,
	Weilin Wang, Levi Yun, Dr. David Alan Gilbert, Gautam Menghani,
	Tengda Wu, linux-perf-users, linux-kernel, bpf

On Fri, Jun 6, 2025 at 10:41 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Wed, Jun 04, 2025 at 10:45:37AM -0700, Ian Rogers wrote:
> > Add parse_uid_filter filter as a helper to parse_filter, that
> > constructs a uid filter string. As uid filters don't work with
> > tracepoint filters, add a is_possible_tp_filter function so the
> > tracepoint filter isn't attempted for tracepoint evsels.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/parse-events.c | 33 ++++++++++++++++++++++++++++++++-
> >  tools/perf/util/parse-events.h |  1 +
> >  2 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index d96adf23dc94..7f34e602fc08 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -25,6 +25,7 @@
> >  #include "pmu.h"
> >  #include "pmus.h"
> >  #include "asm/bug.h"
> > +#include "ui/ui.h"
> >  #include "util/parse-branch-options.h"
> >  #include "util/evsel_config.h"
> >  #include "util/event.h"
> > @@ -2561,6 +2562,12 @@ foreach_evsel_in_last_glob(struct evlist *evlist,
> >       return 0;
> >  }
> >
> > +/* Will a tracepoint filter work for str or should a BPF filter be used? */
> > +static bool is_possible_tp_filter(const char *str)
> > +{
> > +     return strstr(str, "uid") == NULL;
> > +}
> > +
> >  static int set_filter(struct evsel *evsel, const void *arg)
> >  {
> >       const char *str = arg;
> > @@ -2573,7 +2580,7 @@ static int set_filter(struct evsel *evsel, const void *arg)
> >               return -1;
> >       }
> >
> > -     if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT) {
> > +     if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT && is_possible_tp_filter(str)) {
> >               if (evsel__append_tp_filter(evsel, str) < 0) {
> >                       fprintf(stderr,
> >                               "not enough memory to hold filter string\n");
> > @@ -2609,6 +2616,30 @@ int parse_filter(const struct option *opt, const char *str,
> >                                         (const void *)str);
> >  }
> >
> > +int parse_uid_filter(struct evlist *evlist, uid_t uid)
>
> It failed to build on alpine 3.18.
>
> util/parse-events.h:48:45: error: unknown type name 'uid_t'
>    48 | int parse_uid_filter(struct evlist *evlist, uid_t uid);
>       |                                             ^~~~~
>
> I'll add this.

Thanks Namhyung! I see this in tmp.perf-tools-next so I'll assume
there's no need for a v5.

Ian

> Thanks,
> Namhyung
>
>
> ---8<---
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index ab242f6710313993..931780911e071ec6 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -11,6 +11,7 @@
>  #include <linux/perf_event.h>
>  #include <stdio.h>
>  #include <string.h>
> +#include <sys/types.h>
>
>  struct evsel;
>  struct evlist;
>
>
>
> > +{
> > +     struct option opt = {
> > +             .value = &evlist,
> > +     };
> > +     char buf[128];
> > +     int ret;
> > +
> > +     snprintf(buf, sizeof(buf), "uid == %d", uid);
> > +     ret = parse_filter(&opt, buf, /*unset=*/0);
> > +     if (ret) {
> > +             if (use_browser >= 1) {
> > +                     /*
> > +                      * Use ui__warning so a pop up appears above the
> > +                      * underlying BPF error message.
> > +                      */
> > +                     ui__warning("Failed to add UID filtering that uses BPF filtering.\n");
> > +             } else {
> > +                     fprintf(stderr, "Failed to add UID filtering that uses BPF filtering.\n");
> > +             }
> > +     }
> > +     return ret;
> > +}
> > +
> >  static int add_exclude_perf_filter(struct evsel *evsel,
> >                                  const void *arg __maybe_unused)
> >  {
> > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> > index ab242f671031..46e5a01be61c 100644
> > --- a/tools/perf/util/parse-events.h
> > +++ b/tools/perf/util/parse-events.h
> > @@ -45,6 +45,7 @@ static inline int parse_events(struct evlist *evlist, const char *str,
> >  int parse_event(struct evlist *evlist, const char *str);
> >
> >  int parse_filter(const struct option *opt, const char *str, int unset);
> > +int parse_uid_filter(struct evlist *evlist, uid_t uid);
> >  int exclude_perf(const struct option *opt, const char *arg, int unset);
> >
> >  enum parse_events__term_val_type {
> > --
> > 2.50.0.rc0.604.gd4ff7b7c86-goog
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper
  2025-06-06 18:13     ` Ian Rogers
@ 2025-06-06 20:01       ` Namhyung Kim
  0 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2025-06-06 20:01 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, James Clark, Dapeng Mi, Thomas Richter,
	Veronika Molnarova, Chun-Tse Shao, Leo Yan, Hao Ge, Howard Chu,
	Weilin Wang, Levi Yun, Dr. David Alan Gilbert, Gautam Menghani,
	Tengda Wu, linux-perf-users, linux-kernel, bpf

On Fri, Jun 06, 2025 at 11:13:10AM -0700, Ian Rogers wrote:
> On Fri, Jun 6, 2025 at 10:41 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hi Ian,
> >
> > On Wed, Jun 04, 2025 at 10:45:37AM -0700, Ian Rogers wrote:
> > > Add parse_uid_filter filter as a helper to parse_filter, that
> > > constructs a uid filter string. As uid filters don't work with
> > > tracepoint filters, add a is_possible_tp_filter function so the
> > > tracepoint filter isn't attempted for tracepoint evsels.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/perf/util/parse-events.c | 33 ++++++++++++++++++++++++++++++++-
> > >  tools/perf/util/parse-events.h |  1 +
> > >  2 files changed, 33 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > > index d96adf23dc94..7f34e602fc08 100644
> > > --- a/tools/perf/util/parse-events.c
> > > +++ b/tools/perf/util/parse-events.c
> > > @@ -25,6 +25,7 @@
> > >  #include "pmu.h"
> > >  #include "pmus.h"
> > >  #include "asm/bug.h"
> > > +#include "ui/ui.h"
> > >  #include "util/parse-branch-options.h"
> > >  #include "util/evsel_config.h"
> > >  #include "util/event.h"
> > > @@ -2561,6 +2562,12 @@ foreach_evsel_in_last_glob(struct evlist *evlist,
> > >       return 0;
> > >  }
> > >
> > > +/* Will a tracepoint filter work for str or should a BPF filter be used? */
> > > +static bool is_possible_tp_filter(const char *str)
> > > +{
> > > +     return strstr(str, "uid") == NULL;
> > > +}
> > > +
> > >  static int set_filter(struct evsel *evsel, const void *arg)
> > >  {
> > >       const char *str = arg;
> > > @@ -2573,7 +2580,7 @@ static int set_filter(struct evsel *evsel, const void *arg)
> > >               return -1;
> > >       }
> > >
> > > -     if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT) {
> > > +     if (evsel->core.attr.type == PERF_TYPE_TRACEPOINT && is_possible_tp_filter(str)) {
> > >               if (evsel__append_tp_filter(evsel, str) < 0) {
> > >                       fprintf(stderr,
> > >                               "not enough memory to hold filter string\n");
> > > @@ -2609,6 +2616,30 @@ int parse_filter(const struct option *opt, const char *str,
> > >                                         (const void *)str);
> > >  }
> > >
> > > +int parse_uid_filter(struct evlist *evlist, uid_t uid)
> >
> > It failed to build on alpine 3.18.
> >
> > util/parse-events.h:48:45: error: unknown type name 'uid_t'
> >    48 | int parse_uid_filter(struct evlist *evlist, uid_t uid);
> >       |                                             ^~~~~
> >
> > I'll add this.
> 
> Thanks Namhyung! I see this in tmp.perf-tools-next so I'll assume
> there's no need for a v5.

Right, I've updated the branch with the fix.

Thanks,
Namhyung


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 00/10] Move uid filtering to BPF filters
  2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
                   ` (10 preceding siblings ...)
  2025-06-06  2:44 ` [PATCH v4 00/10] Move uid filtering to BPF filters Namhyung Kim
@ 2025-06-10 18:38 ` Namhyung Kim
  11 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2025-06-10 18:38 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, James Clark, Dapeng Mi, Thomas Richter,
	Veronika Molnarova, Chun-Tse Shao, Leo Yan, Hao Ge, Howard Chu,
	Weilin Wang, Levi Yun, Dr. David Alan Gilbert, Gautam Menghani,
	Tengda Wu, linux-perf-users, linux-kernel, bpf, Ian Rogers

On Wed, 04 Jun 2025 10:45:34 -0700, Ian Rogers wrote:
> Rather than scanning /proc and skipping PIDs based on their UIDs, use
> BPF filters for uid filtering. The /proc scanning in thread_map is
> racy as the PID may exit before the perf_event_open causing perf to
> abort. BPF UID filters are more robust as they avoid the race. The
> /proc scanning also misses processes starting after the perf
> command. Add a helper for commands that support UID filtering and wire
> up. Remove the non-BPF UID filtering support given it doesn't work.
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-06-10 18:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-04 17:45 [PATCH v4 00/10] Move uid filtering to BPF filters Ian Rogers
2025-06-04 17:45 ` [PATCH v4 01/10] perf parse-events filter: Use evsel__find_pmu Ian Rogers
2025-06-04 17:45 ` [PATCH v4 02/10] perf target: Separate parse_uid into its own function Ian Rogers
2025-06-04 17:45 ` [PATCH v4 03/10] perf parse-events: Add parse_uid_filter helper Ian Rogers
2025-06-06 17:41   ` Namhyung Kim
2025-06-06 18:13     ` Ian Rogers
2025-06-06 20:01       ` Namhyung Kim
2025-06-04 17:45 ` [PATCH v4 04/10] perf record: Switch user option to use BPF filter Ian Rogers
2025-06-04 17:45 ` [PATCH v4 05/10] perf tests record: Add basic uid filtering test Ian Rogers
2025-06-04 17:45 ` [PATCH v4 06/10] perf top: Switch user option to use BPF filter Ian Rogers
2025-06-04 17:45 ` [PATCH v4 07/10] perf trace: " Ian Rogers
2025-06-04 17:45 ` [PATCH v4 08/10] perf bench evlist-open-close: " Ian Rogers
2025-06-04 17:45 ` [PATCH v4 09/10] perf target: Remove uid from target Ian Rogers
2025-06-04 17:45 ` [PATCH v4 10/10] perf thread_map: Remove uid options Ian Rogers
2025-06-06  2:44 ` [PATCH v4 00/10] Move uid filtering to BPF filters Namhyung Kim
2025-06-10 18:38 ` Namhyung Kim

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).