[PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions

[PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions

[Ian Rogers]

Under system-wide (-a) parallel streaming mode (--threads=cpu),
background recording threads can be inundated by a continuous
firehose of hardware samples generated by the OS. In this state,
a background thread's local hit count remains unequal to its
sample count, causing it to bypass the blocking fdarray__poll()
call entirely on each iteration of its recording loop.

Because the termination check relies on the POLLHUP event status
populated specifically by fdarray__poll(), bypassing it prevents
the background thread from ever recognizing that its control pipe
was closed by the main thread. This traps the background thread
in an infinite recording loop, hanging the main thread indefinitely
as it awaits a termination acknowledgment that never arrives.

Ensure teardown completion by adding explicit evlist__disable()
calls in the main thread's cleanup paths at out_child: and
out_child_no_flush:. Additionally, patch fdarray__filter() to
respect the fdarray_flag__nonfilterable flag, preventing it
from incorrectly setting the background thread's control pipe
file descriptor to -1 and clearing its revents mask upon
processing termination POLLHUP signals.

Fixes: f94563fac269 ("perf record: fix poll storm when monitored threads exit")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/api/fd/array.c    | 6 ++++--
 tools/perf/builtin-record.c | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
index ffe8272af59b..16a047f1906e 100644
--- a/tools/lib/api/fd/array.c
+++ b/tools/lib/api/fd/array.c
@@ -115,6 +115,9 @@ int fdarray__filter(struct fdarray *fda, short revents,
 		return 0;
 
 	for (fd = 0; fd < fda->nr; ++fd) {
+		if (fda->priv[fd].flags & fdarray_flag__nonfilterable)
+			continue;
+
 		if (!fda->entries[fd].events)
 			continue;
 
@@ -132,8 +135,7 @@ int fdarray__filter(struct fdarray *fda, short revents,
 			continue;
 		}
 
-		if (!(fda->priv[fd].flags & fdarray_flag__nonfilterable))
-			++nr;
+		++nr;
 	}
 
 	return nr;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ebd3ed0c9b3e..d1276382b77a 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2890,11 +2890,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		record__synthesize_workload(rec, true);
 
 out_child:
+	evlist__disable(rec->evlist);
 	record__stop_threads(rec);
 	record__mmap_read_all(rec, true);
 	goto out_free_threads;
 out_child_no_flush:
 	/* mmap read already failed — retrying would just fail again */
+	evlist__disable(rec->evlist);
 	record__stop_threads(rec);
 out_free_threads:
 	record__free_thread_data(rec);
-- 
2.55.0.795.g602f6c329a-goog

[PATCH v1 2/2] perf cap: If capability is missing still perform root test

[Ian Rogers]

Refine the capability checking logic in perf_cap__capable() to in the
event the capability is missing do the fallback used_root
test. Previously this was only performed when the capability syscall
failed.

Fixes: e25ebda78e23 ("perf cap: Tidy up and improve capability testing")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/cap.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cap.c b/tools/perf/util/cap.c
index ac6d1d9a523d..a2baecbc3145 100644
--- a/tools/perf/util/cap.c
+++ b/tools/perf/util/cap.c
@@ -20,6 +20,7 @@ bool perf_cap__capable(int cap, bool *used_root)
 	};
 	struct __user_cap_data_struct data[MAX_LINUX_CAPABILITY_U32S] = {};
 	__u32 cap_val;
+	bool has_cap = false;
 
 	*used_root = false;
 	while (syscall(SYS_capget, &header, &data[0]) == -1) {
@@ -37,12 +38,21 @@ bool perf_cap__capable(int cap, bool *used_root)
 	if (cap >= 32) {
 		if (header.version == _LINUX_CAPABILITY_VERSION_3) {
 			cap_val = data[1].effective;
+			has_cap = (cap_val & (1U << (cap & 0x1f))) != 0;
 		} else {
 			/* Capability beyond 32 is requested but only 32 are supported. */
-			return false;
+			has_cap = false;
 		}
 	} else {
 		cap_val = data[0].effective;
+		has_cap = (cap_val & (1U << (cap & 0x1f))) != 0;
 	}
-	return (cap_val & (1 << (cap & 0x1f))) != 0;
+
+	if (has_cap) {
+		*used_root = false;
+		return true;
+	}
+
+	*used_root = true;
+	return geteuid() == 0;
 }
-- 
2.55.0.795.g602f6c329a-goog

Re: [PATCH v1 2/2] perf cap: If capability is missing still perform root test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The capability fallback logic overrides valid capability checks for root users and incorrectly short-circuits sequential capability checks for non-root users by improperly setting the `used_root` flag.
--

--- Patch [2]: [PATCH v1 2/2] perf cap: If capability is missing still perform root test ---
commit 8515c6747c6ffb6d27af14829f03c002334e5c67
Author: Ian Rogers <irogers@google.com>

perf cap: If capability is missing still perform root test

This commit refines the capability checking logic in perf_cap__capable() to
fallback to the used_root test if a capability is missing, rather than only
doing so when the capget syscall fails.

> diff --git a/tools/perf/util/cap.c b/tools/perf/util/cap.c
> --- a/tools/perf/util/cap.c
> +++ b/tools/perf/util/cap.c

[ ... ]

> @@ -37,12 +38,21 @@ bool perf_cap__capable(int cap, bool *used_root)
>  	if (cap >= 32) {
>  		if (header.version == _LINUX_CAPABILITY_VERSION_3) {
>  			cap_val = data[1].effective;
> +			has_cap = (cap_val & (1U << (cap & 0x1f))) != 0;
>  		} else {
>  			/* Capability beyond 32 is requested but only 32 are supported. */
> -			return false;
> +			has_cap = false;
>  		}
>  	} else {
>  		cap_val = data[0].effective;
> +		has_cap = (cap_val & (1U << (cap & 0x1f))) != 0;
>  	}
> -	return (cap_val & (1 << (cap & 0x1f))) != 0;
> +
> +	if (has_cap) {
> +		*used_root = false;
> +		return true;
> +	}
> +
> +	*used_root = true;
> +	return geteuid() == 0;
>  }

[Severity: High]
Does setting *used_root = true unconditionally here when a capability is
missing break sequential capability checks for non-root users?

For example, if an unprivileged user lacks CAP_SYS_ADMIN, this will return
false but set used_root to true. In callers like perf_event_paranoid_check():

tools/perf/util/util.c:perf_event_paranoid_check() {
    ...
    if (!used_root && perf_cap__capable(CAP_PERFMON, &used_root))
    ...
}

If a previous check for CAP_SYS_ADMIN failed and poisoned the used_root
flag, won't it cause the subsequent check for CAP_PERFMON to be skipped
entirely?

This pattern appears in other locations as well, such as
check_bpf_filter_capable() where a failed CAP_BPF check might skip the
fallback for non-root users to access root-pinned BPF filters because
!used_root evaluates to false.

Additionally, could this override valid capability drops for root users
in containers? Because geteuid() == 0 evaluates to true, it seems this might
falsely claim root users still possess the capability even if it was
explicitly dropped, potentially leading to later failures instead of
graceful fallbacks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710053927.1864021-1-irogers@google.com?part=2

Re: [PATCH v1 2/2] perf cap: If capability is missing still perform root test

[Namhyung Kim]

On Fri, Jul 10, 2026 at 05:47:10AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The capability fallback logic overrides valid capability checks for root users and incorrectly short-circuits sequential capability checks for non-root users by improperly setting the `used_root` flag.
> --
> 
> --- Patch [2]: [PATCH v1 2/2] perf cap: If capability is missing still perform root test ---
> commit 8515c6747c6ffb6d27af14829f03c002334e5c67
> Author: Ian Rogers <irogers@google.com>
> 
> perf cap: If capability is missing still perform root test
> 
> This commit refines the capability checking logic in perf_cap__capable() to
> fallback to the used_root test if a capability is missing, rather than only
> doing so when the capget syscall fails.

I think we can process this independently.  It's not related to patch 1,
right?

> 
> > diff --git a/tools/perf/util/cap.c b/tools/perf/util/cap.c
> > --- a/tools/perf/util/cap.c
> > +++ b/tools/perf/util/cap.c
> 
> [ ... ]
> 
> > @@ -37,12 +38,21 @@ bool perf_cap__capable(int cap, bool *used_root)
> >  	if (cap >= 32) {
> >  		if (header.version == _LINUX_CAPABILITY_VERSION_3) {
> >  			cap_val = data[1].effective;
> > +			has_cap = (cap_val & (1U << (cap & 0x1f))) != 0;
> >  		} else {
> >  			/* Capability beyond 32 is requested but only 32 are supported. */
> > -			return false;
> > +			has_cap = false;
> >  		}
> >  	} else {
> >  		cap_val = data[0].effective;
> > +		has_cap = (cap_val & (1U << (cap & 0x1f))) != 0;
> >  	}
> > -	return (cap_val & (1 << (cap & 0x1f))) != 0;
> > +
> > +	if (has_cap) {
> > +		*used_root = false;
> > +		return true;
> > +	}
> > +
> > +	*used_root = true;
> > +	return geteuid() == 0;
> >  }
> 
> [Severity: High]
> Does setting *used_root = true unconditionally here when a capability is
> missing break sequential capability checks for non-root users?
> 
> For example, if an unprivileged user lacks CAP_SYS_ADMIN, this will return
> false but set used_root to true. In callers like perf_event_paranoid_check():
> 
> tools/perf/util/util.c:perf_event_paranoid_check() {
>     ...
>     if (!used_root && perf_cap__capable(CAP_PERFMON, &used_root))
>     ...
> }
> 
> If a previous check for CAP_SYS_ADMIN failed and poisoned the used_root
> flag, won't it cause the subsequent check for CAP_PERFMON to be skipped
> entirely?
> 
> This pattern appears in other locations as well, such as
> check_bpf_filter_capable() where a failed CAP_BPF check might skip the
> fallback for non-root users to access root-pinned BPF filters because
> !used_root evaluates to false.
> 
> Additionally, could this override valid capability drops for root users
> in containers? Because geteuid() == 0 evaluates to true, it seems this might
> falsely claim root users still possess the capability even if it was
> explicitly dropped, potentially leading to later failures instead of
> graceful fallbacks.

Can we simply remove the used_root parameter from perf_cap__capable()?
Normally I think root would have the capability.  If not, maybe we need
to reject the operation as it may happen in a container setup.

Thanks,
Namhyung

[PATCH v2] perf cap: Remove used_root parameter and simplify capability checks

[Ian Rogers]

Refactor perf_cap__capable() to completely remove the used_root out-parameter
as requested by the maintainer. Relying on an explicit used_root boolean
poisoned sequential capability checks (e.g. failing CAP_SYS_ADMIN checks
poisoning the flag for subsequent CAP_PERFMON evaluations for unprivileged
users) and created redundant complexity across check_ftrace_capable(),
symbol__read_kptr_restrict(), and perf_event_paranoid_check().

Streamline the capability API to perform a pure true/false boolean
evaluation. The function checks the Effective set using SYS_capget; if
the syscall is missing or fails on legacy kernels, it cleanly falls back
to checking EUID == 0. This perfectly preserves modern capability-aware host
sessions, guarantees transparent fallback for older kernels, and correctly
rejects privileged operations for containerized root processes that have
explicitly dropped their capability bounding and permitted sets.

Fixes: e25ebda78e23 ("perf cap: Tidy up and improve capability testing")
Suggested-by: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-ftrace.c  | 13 +++----------
 tools/perf/util/bpf-filter.c | 22 +++++++++-------------
 tools/perf/util/cap.c        |  4 +---
 tools/perf/util/cap.h        |  3 +--
 tools/perf/util/symbol.c     |  3 +--
 tools/perf/util/util.c       | 12 +++---------
 6 files changed, 18 insertions(+), 39 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 9e4c5220d43c..f7126196b092 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -72,18 +72,11 @@ static void ftrace__workload_exec_failed_signal(int signo __maybe_unused,
 
 static bool check_ftrace_capable(void)
 {
-	bool used_root;
-
-	if (perf_cap__capable(CAP_PERFMON, &used_root))
-		return true;
-
-	if (!used_root && perf_cap__capable(CAP_SYS_ADMIN, &used_root))
+	if (perf_cap__capable(CAP_PERFMON) ||
+	    perf_cap__capable(CAP_SYS_ADMIN))
 		return true;
 
-	pr_err("ftrace only works for %s!\n",
-		used_root ? "root"
-			  : "users with the CAP_PERFMON or CAP_SYS_ADMIN capability"
-		);
+	pr_err("ftrace only works for users with the CAP_PERFMON or CAP_SYS_ADMIN capability!\n");
 	return false;
 }
 
diff --git a/tools/perf/util/bpf-filter.c b/tools/perf/util/bpf-filter.c
index 1a2e7b388d57..bcd81084e342 100644
--- a/tools/perf/util/bpf-filter.c
+++ b/tools/perf/util/bpf-filter.c
@@ -629,24 +629,20 @@ struct perf_bpf_filter_expr *perf_bpf_filter_expr__new(enum perf_bpf_filter_term
 
 static bool check_bpf_filter_capable(void)
 {
-	bool used_root;
+	int fd;
 
-	if (perf_cap__capable(CAP_BPF, &used_root))
+	if (perf_cap__capable(CAP_BPF))
 		return true;
 
-	if (!used_root) {
-		/* Check if root already pinned the filter programs and maps */
-		int fd = get_pinned_fd("filters");
-
-		if (fd >= 0) {
-			close(fd);
-			return true;
-		}
+	/* Check if root already pinned the filter programs and maps */
+	fd = get_pinned_fd("filters");
+	if (fd >= 0) {
+		close(fd);
+		return true;
 	}
 
-	pr_err("Error: BPF filter only works for %s!\n"
-	       "\tPlease run 'perf record --setup-filter pin' as root first.\n",
-	       used_root ? "root" : "users with the CAP_BPF capability");
+	pr_err("Error: BPF filter only works for users with the CAP_BPF capability!\n"
+	       "\tPlease run 'perf record --setup-filter pin' as root first.\n");
 
 	return false;
 }
diff --git a/tools/perf/util/cap.c b/tools/perf/util/cap.c
index ac6d1d9a523d..272bd8255ff1 100644
--- a/tools/perf/util/cap.c
+++ b/tools/perf/util/cap.c
@@ -12,7 +12,7 @@
 
 #define MAX_LINUX_CAPABILITY_U32S _LINUX_CAPABILITY_U32S_3
 
-bool perf_cap__capable(int cap, bool *used_root)
+bool perf_cap__capable(int cap)
 {
 	struct __user_cap_header_struct header = {
 		.version = _LINUX_CAPABILITY_VERSION_3,
@@ -21,7 +21,6 @@ bool perf_cap__capable(int cap, bool *used_root)
 	struct __user_cap_data_struct data[MAX_LINUX_CAPABILITY_U32S] = {};
 	__u32 cap_val;
 
-	*used_root = false;
 	while (syscall(SYS_capget, &header, &data[0]) == -1) {
 		/* Retry, first attempt has set the header.version correctly. */
 		if (errno == EINVAL && header.version != _LINUX_CAPABILITY_VERSION_3 &&
@@ -29,7 +28,6 @@ bool perf_cap__capable(int cap, bool *used_root)
 			continue;
 
 		pr_debug2("capget syscall failed (%m) fall back on root check\n");
-		*used_root = true;
 		return geteuid() == 0;
 	}
 
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
index c1b8ac033ccc..bf09fb20c779 100644
--- a/tools/perf/util/cap.h
+++ b/tools/perf/util/cap.h
@@ -18,7 +18,6 @@
 #define CAP_BPF		39
 #endif
 
-/* Query if a capability is supported, used_root is set if the fallback root check was used. */
-bool perf_cap__capable(int cap, bool *used_root);
+bool perf_cap__capable(int cap);
 
 #endif /* __PERF_CAP_H */
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a562702b4841..94f9c8faedda 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2452,8 +2452,7 @@ static bool symbol__read_kptr_restrict(void)
 {
 	bool value = false;
 	FILE *fp = fopen("/proc/sys/kernel/kptr_restrict", "r");
-	bool used_root;
-	bool cap_syslog = perf_cap__capable(CAP_SYSLOG, &used_root);
+	bool cap_syslog = perf_cap__capable(CAP_SYSLOG);
 
 	if (fp != NULL) {
 		char line[8];
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 2c2a5c449ffd..8f7cd32f524d 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -378,15 +378,9 @@ int perf_event_paranoid(void)
 
 bool perf_event_paranoid_check(int max_level)
 {
-	bool used_root;
-
-	if (perf_cap__capable(CAP_SYS_ADMIN, &used_root))
-		return true;
-
-	if (!used_root && perf_cap__capable(CAP_PERFMON, &used_root))
-		return true;
-
-	return perf_event_paranoid() <= max_level;
+	return perf_cap__capable(CAP_SYS_ADMIN) ||
+	       perf_cap__capable(CAP_PERFMON) ||
+	       perf_event_paranoid() <= max_level;
 }
 
 int perf_tip(char **strp, const char *dirpath)
-- 
2.55.0.141.g00534a21ce-goog

Re: [PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch unconditionally skips `fdarray_flag__nonfilterable` file descriptors in `fdarray__filter()`, causing a memory leak of `perf_mmap` references and an infinite busy loop if a system-wide event or other nonfilterable file descriptor receives an error (e.g., POLLERR, POLLHUP).
--

--- Patch [1]: [PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions ---
commit 6940fed42e3ec7ba75ed80d3ab8c1cc4f0aad35d
Author: Ian Rogers <irogers@google.com>

perf record: Fix teardown hang on system-wide multi-threaded sessions

This patch attempts to resolve a hang during teardown in system-wide streaming
mode by adding explicit evlist disables. It also modifies the fdarray filter
logic to bypass nonfilterable file descriptors when processing POLLHUP signals.

> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index f0f195207fca9..3bfcd2aa7c07f 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -115,6 +115,9 @@ int fdarray__filter(struct fdarray *fda, short revents,
>  		return 0;
>  
>  	for (fd = 0; fd < fda->nr; ++fd) {
> +		if (fda->priv[fd].flags & fdarray_flag__nonfilterable)
> +			continue;
> +

[Severity: High]
Does skipping the filter logic for non-filterable file descriptors here cause
an infinite poll loop if a system-wide event receives an error?

In tools/lib/perf/evlist.c:mmap_per_evsel(), system-wide event file
descriptors are marked with fdarray_flag__nonfilterable:

    flgs = evsel->system_wide ? fdarray_flag__nonfilterable : fdarray_flag__default;

If one of these file descriptors encounters an error like POLLERR or POLLHUP,
it seems this new continue statement bypasses the block that clears the events
mask and invokes the entry_destructor.

Could this cause poll() to continuously return immediately with the same
error, leading to a CPU busy loop and leaking perf_mmap references since the
destructor is skipped?

>  		if (!fda->entries[fd].events)
>  			continue;
>  
> @@ -126,8 +129,7 @@ int fdarray__filter(struct fdarray *fda, short revents,
>  			continue;
>  		}
>  
> -		if (!(fda->priv[fd].flags & fdarray_flag__nonfilterable))
> -			++nr;
> +		++nr;
>  	}
>  
>  	return nr;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710053927.1864021-1-irogers@google.com?part=1

Re: [PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions

[Namhyung Kim]

Hi Ian,

On Thu, Jul 09, 2026 at 10:39:06PM -0700, Ian Rogers wrote:
> Under system-wide (-a) parallel streaming mode (--threads=cpu),
> background recording threads can be inundated by a continuous
> firehose of hardware samples generated by the OS. In this state,
> a background thread's local hit count remains unequal to its
> sample count, causing it to bypass the blocking fdarray__poll()
> call entirely on each iteration of its recording loop.
> 
> Because the termination check relies on the POLLHUP event status
> populated specifically by fdarray__poll(), bypassing it prevents
> the background thread from ever recognizing that its control pipe
> was closed by the main thread. This traps the background thread
> in an infinite recording loop, hanging the main thread indefinitely
> as it awaits a termination acknowledgment that never arrives.
> 
> Ensure teardown completion by adding explicit evlist__disable()
> calls in the main thread's cleanup paths at out_child: and
> out_child_no_flush:. Additionally, patch fdarray__filter() to
> respect the fdarray_flag__nonfilterable flag, preventing it
> from incorrectly setting the background thread's control pipe
> file descriptor to -1 and clearing its revents mask upon
> processing termination POLLHUP signals.
> 
> Fixes: f94563fac269 ("perf record: fix poll storm when monitored threads exit")
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Ian Rogers <irogers@google.com>

I didn't know you already sent the fix so I was doing the same.  It
seems your fix is better as it added evlist__disable().  I'll test this
and pick up if it's ok.

Thanks,
Namhyung

> ---
>  tools/lib/api/fd/array.c    | 6 ++++--
>  tools/perf/builtin-record.c | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index ffe8272af59b..16a047f1906e 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -115,6 +115,9 @@ int fdarray__filter(struct fdarray *fda, short revents,
>  		return 0;
>  
>  	for (fd = 0; fd < fda->nr; ++fd) {
> +		if (fda->priv[fd].flags & fdarray_flag__nonfilterable)
> +			continue;
> +
>  		if (!fda->entries[fd].events)
>  			continue;
>  
> @@ -132,8 +135,7 @@ int fdarray__filter(struct fdarray *fda, short revents,
>  			continue;
>  		}
>  
> -		if (!(fda->priv[fd].flags & fdarray_flag__nonfilterable))
> -			++nr;
> +		++nr;
>  	}
>  
>  	return nr;
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index ebd3ed0c9b3e..d1276382b77a 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -2890,11 +2890,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  		record__synthesize_workload(rec, true);
>  
>  out_child:
> +	evlist__disable(rec->evlist);
>  	record__stop_threads(rec);
>  	record__mmap_read_all(rec, true);
>  	goto out_free_threads;
>  out_child_no_flush:
>  	/* mmap read already failed — retrying would just fail again */
> +	evlist__disable(rec->evlist);
>  	record__stop_threads(rec);
>  out_free_threads:
>  	record__free_thread_data(rec);
> -- 
> 2.55.0.795.g602f6c329a-goog
> 

Re: (subset) [PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions

[Namhyung Kim]

On Thu, 09 Jul 2026 22:39:06 -0700, Ian Rogers wrote:
> Under system-wide (-a) parallel streaming mode (--threads=cpu),
> background recording threads can be inundated by a continuous
> firehose of hardware samples generated by the OS. In this state,
> a background thread's local hit count remains unequal to its
> sample count, causing it to bypass the blocking fdarray__poll()
> call entirely on each iteration of its recording loop.
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung

[PATCH v2] perf record: Fix destructor invocation and event counting in fdarray__filter

[Ian Rogers]

Fix the API-level fdarray filtering logic in fdarray__filter() following
the integration of the system-wide recording teardown fix. Rather than
bypassing the evaluation loop entirely for nonfilterable file
descriptors (which accidentally skips calling registered entry
destructors for control pipes and wakeup FDs, while creating poll storms
and reference leaks for system-wide events that also have nonfilterable
set), separate the lifecycle and masking logic.

Reorder the filtering sequence so that registered entry destructors are
safely executed first for all file descriptors. Then, utilize the
fdarray_flag__non_perf_event flag to perfectly distinguish and bypass
destructive file descriptor zeroing (.fd = -1) and revents mask
clearing for background control pipes and timers. This allows system-wide
event file descriptors to be correctly destroyed, zeroed, and filtered out upon
receiving POLLHUP/POLLERR signals without triggering busy loops, while safely
preserving control pipe teardown signaling and destructors.

Fixes: fb4751e79c45 ("perf record: Fix teardown hang on system-wide multi-threaded sessions")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/api/fd/array.c    | 6 +++---
 tools/perf/builtin-record.c | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
index 16a047f1906e..92d9a3fb652c 100644
--- a/tools/lib/api/fd/array.c
+++ b/tools/lib/api/fd/array.c
@@ -115,9 +115,6 @@ int fdarray__filter(struct fdarray *fda, short revents,
 		return 0;
 
 	for (fd = 0; fd < fda->nr; ++fd) {
-		if (fda->priv[fd].flags & fdarray_flag__nonfilterable)
-			continue;
-
 		if (!fda->entries[fd].events)
 			continue;
 
@@ -125,6 +122,9 @@ int fdarray__filter(struct fdarray *fda, short revents,
 			if (entry_destructor)
 				entry_destructor(fda, fd, arg);
 
+			if (fda->priv[fd].flags & fdarray_flag__non_perf_event)
+				continue;
+
 			/*
 			 * Set fd to -1 so poll() ignores this entry; otherwise
 			 * POLLHUP/POLLERR are still reported for events=0 fds
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f58d7e3c7879..c6644dab1cfb 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1242,7 +1242,9 @@ static int record__alloc_thread_data(struct record *rec, struct evlist *evlist)
 				goto out_free;
 			}
 			ret = fdarray__add(&thread_data[t].pollfd, thread_data[t].pipes.msg[0],
-					   POLLIN | POLLERR | POLLHUP, fdarray_flag__nonfilterable);
+					   POLLIN | POLLERR | POLLHUP,
+					   fdarray_flag__nonfilterable |
+					   fdarray_flag__non_perf_event);
 			if (ret < 0) {
 				pr_err("Failed to add descriptor to thread[%d] pollfd\n", t);
 				goto out_free;
-- 
2.55.0.141.g00534a21ce-goog

Re: [PATCH v2] perf record: Fix destructor invocation and event counting in fdarray__filter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf record and perf trace hang indefinitely when profiling a target process that exits.
--

--- Patch [1]: [PATCH v2] perf record: Fix destructor invocation and event counting in fdarray__filter ---
commit 586c1ed32764876e9ecee40639dd9f2a7fcfdcd5
Author: Ian Rogers <irogers@google.com>

perf record: Fix destructor invocation and event counting in fdarray__filter

This commit modifies the fdarray filtering logic to separate lifecycle and
masking logic. It removes the early bypass for nonfilterable file descriptors
so their destructors can run, and uses a non_perf_event flag to prevent
clearing their masks.

> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index 16a047f1906ee..92d9a3fb652c0 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -115,9 +115,6 @@ int fdarray__filter(struct fdarray *fda, short revents,
>  		return 0;
>  
>  	for (fd = 0; fd < fda->nr; ++fd) {
> -		if (fda->priv[fd].flags & fdarray_flag__nonfilterable)
> -			continue;
> -
>  		if (!fda->entries[fd].events)
>  			continue;
>  
> @@ -125,6 +122,9 @@ int fdarray__filter(struct fdarray *fda, short revents,
>  			if (entry_destructor)
>  				entry_destructor(fda, fd, arg);
>  
> +			if (fda->priv[fd].flags & fdarray_flag__non_perf_event)
> +				continue;
> +
>  			/*
>  			 * Set fd to -1 so poll() ignores this entry; otherwise

[Severity: High]
With the removal of the early continue for nonfilterable file descriptors,
does this regression cause the active event count to never reach zero? 

When profiling a target process that exits, auxiliary file descriptors like
control pipes usually do not match the revents mask (such as POLLHUP). They
now bypass the revents check block and unconditionally reach the counter
increment at the bottom of the loop:

fdarray__filter()
        ...
		if (fda->entries[fd].revents & revents) {
			...
			continue;
		}

		++nr;
	}

	return nr;
}

In the main event loop, __cmd_record() relies on this function returning 0
to transition to the draining state and terminate gracefully:

    if (fdarray__filter(&thread->pollfd, POLLERR | POLLHUP,
                        record__thread_munmap_filtered, NULL) == 0)
        draining = true;

Since nonfilterable and non-perf-event file descriptors unconditionally
increment nr when they don't have matching revents, could this cause
perf record and perf trace to hang indefinitely waiting in poll()
after the traced process exits?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716073753.567782-1-irogers@google.com?part=1