Linux Perf Users
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: irogers@google.com, acme@kernel.org, namhyung@kernel.org
Cc: abyssmystery@gmail.com, adrian.hunter@intel.com,
	james.clark@linaro.org,  jolsa@kernel.org,
	linux-kernel@vger.kernel.org,  linux-perf-users@vger.kernel.org,
	mingo@redhat.com, peterz@infradead.org
Subject: [PATCH v7] perf record: Fix unhandled POLLHUP on non_perf_event descriptors
Date: Thu, 20 Aug 2026 13:02:14 -0700	[thread overview]
Message-ID: <20260820200214.2865570-1-irogers@google.com> (raw)
In-Reply-To: <20260820164937.2734066-1-irogers@google.com>

When processing POLLHUP on a non_perf_event control descriptor,
evlist__finalize_ctlfd() is invoked to finalize the setup, correctly
setting the core evlist's poll array file descriptor to -1. However,
this finalized teardown state is never propagated back to the
individual thread's local replica of the pollfd array.

Consequently, on the next iteration of the main recording loop,
record__update_evlist_pollfd_from_thread() performs a strict
equivalence check between the core evlist's array and the thread's
localized poll array, detecting that the fd values no longer match.
This causes an immediate -EINVAL abort and a premature teardown.

Fix the underlying logic within
record__update_evlist_pollfd_from_thread() to sustainably propagate
the finalized teardown statuses (-1) originating from the core evlist
back to the thread's localized poll structure. This correctly
maintains synchronization and entirely prevents the unhandled
index mismatch crashes.

Additionally, add a unit test that explicitly validates that
fdarray__filter() preserves its invariants regarding
fdarray_flag__nonfilterable items to guard against regressions.

Fixes: fb4751e79c45 ("perf record: Fix teardown hang on system-wide multi-threaded sessions")
Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-record.c | 15 +++++++++++++++
 tools/perf/tests/fdarray.c  | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index a57987851cf0..ad81458989b5 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1169,6 +1169,21 @@ static int record__update_evlist_pollfd_from_thread(struct record *rec,
 		int e_pos = rec->index_map[i].evlist_pollfd_index;
 		int t_pos = rec->index_map[i].thread_pollfd_index;
 
+		if (e_entries[e_pos].fd == -1 || e_entries[e_pos].events == 0) {
+			/*
+			 * If the control file descriptor was closed, then evlist__ctlfd_process()
+			 * will have called evlist__finalize_ctlfd() on the PREVIOUS loop iteration
+			 * to cleanly set the core evlist's e_entries[e_pos].fd to -1.
+			 *
+			 * Since nonfilterable items are skipped by fdarray__filter(), the
+			 * thread's local t_entries[t_pos] retains its original state.
+			 * We must explicitly propagate the finalized -1 state to t_entries
+			 * BEFORE evaluating the strict equivalence check below.
+			 */
+			t_entries[t_pos].fd = -1;
+			t_entries[t_pos].events = 0;
+		}
+
 		if (e_entries[e_pos].fd != t_entries[t_pos].fd ||
 		    e_entries[e_pos].events != t_entries[t_pos].events) {
 			pr_err("Thread and evlist pollfd index mismatch\n");
diff --git a/tools/perf/tests/fdarray.c b/tools/perf/tests/fdarray.c
index 40983c3574b1..23860edb8ef0 100644
--- a/tools/perf/tests/fdarray.c
+++ b/tools/perf/tests/fdarray.c
@@ -80,6 +80,38 @@ static int test__fdarray__filter(struct test_suite *test __maybe_unused, int sub
 		goto out_delete;
 	}
 
+	fdarray__init_revents(fda, POLLHUP);
+	fda->priv[2].flags = fdarray_flag__nonfilterable;
+
+	pr_debug("\nfiltering all but fda->entries[2] (nonfilterable):");
+	fdarray__fprintf_prefix(fda, "before", stderr);
+
+	/*
+	 * Note: fdarray__filter() in tools/lib/api/fd/array.c evaluates the
+	 * fdarray_flag__nonfilterable flag at the very top of its loop via an
+	 * early continue. Therefore, it completely skips all processing for this
+	 * descriptor, guaranteeing its fd, events, and revents fields remain
+	 * entirely untouched by the filter mask evaluation below it.
+	 */
+	nr_fds = fdarray__filter(fda, POLLHUP, NULL, NULL);
+	fdarray__fprintf_prefix(fda, " after", stderr);
+
+	if (nr_fds != 0) {
+		pr_debug("\nfdarray__filter()=%d != 0, should be 0\n",
+			 nr_fds);
+		goto out_delete;
+	}
+	if (fda->entries[2].fd == -1) {
+		pr_debug("\nfdarray__filter() illegally modified nonfilterable fd!");
+		goto out_delete;
+	}
+	if (fda->entries[2].revents != POLLHUP) {
+		pr_debug("\nfdarray__filter() illegally modified nonfilterable revents!");
+		goto out_delete;
+	}
+
+	fda->priv[2].flags = 0; /* reset flags */
+
 	pr_debug("\n");
 
 	err = 0;
-- 
2.55.0.766.g2966f0265a-goog


  parent reply	other threads:[~2026-08-20 20:02 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10  5:39 [PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions Ian Rogers
2026-07-10  5:39 ` [PATCH v1 2/2] perf cap: If capability is missing still perform root test Ian Rogers
2026-07-10  5:47   ` sashiko-bot
2026-07-10 22:24     ` Namhyung Kim
2026-07-16  7:40       ` [PATCH v2] perf cap: Remove used_root parameter and simplify capability checks Ian Rogers
2026-07-23  5:06         ` [PATCH v3] " Ian Rogers
2026-07-24  5:47           ` Namhyung Kim
2026-07-10  5:56 ` [PATCH v1 1/2] perf record: Fix teardown hang on system-wide multi-threaded sessions sashiko-bot
2026-07-10 22:02 ` Namhyung Kim
2026-07-12  6:56 ` (subset) " Namhyung Kim
2026-07-16  7:37   ` [PATCH v2] perf record: Fix destructor invocation and event counting in fdarray__filter Ian Rogers
2026-07-16  8:00     ` sashiko-bot
2026-07-23  5:04     ` [PATCH v3] " Ian Rogers
2026-07-23  5:27       ` sashiko-bot
2026-08-17 23:04       ` [PATCH v4] perf fdarray: " Ian Rogers
2026-08-17 23:17         ` Namhyung Kim
2026-08-17 23:30           ` Ian Rogers
2026-08-17 23:28         ` sashiko-bot
2026-08-18  0:31         ` [PATCH v5] perf record: Fix unhandled POLLHUP on non_perf_event descriptors Ian Rogers
2026-08-18  0:43           ` sashiko-bot
2026-08-20 16:49           ` [PATCH v6] " Ian Rogers
2026-08-20 17:00             ` sashiko-bot
2026-08-20 20:02             ` Ian Rogers [this message]
2026-08-20 20:17               ` [PATCH v7] " sashiko-bot
2026-08-20 21:37               ` [PATCH v8] " Ian Rogers
2026-08-20 21:47                 ` sashiko-bot
2026-08-20 22:00                   ` Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260820200214.2865570-1-irogers@google.com \
    --to=irogers@google.com \
    --cc=abyssmystery@gmail.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox