From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: acme@kernel.org, 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: Re: [PATCH v4] perf fdarray: Fix destructor invocation and event counting in fdarray__filter
Date: Tue, 18 Aug 2026 08:17:27 +0900 [thread overview]
Message-ID: <aoOWh-e5BR2PX27Q@google.com> (raw)
In-Reply-To: <20260817230431.424646-1-irogers@google.com>
On Mon, Aug 17, 2026 at 04:04:31PM -0700, Ian Rogers wrote:
> When processing POLLHUP or POLLERR for an event in fdarray__filter, the
> function invokes its destructor callback. However, the exact behavior
> around unhandled POLLHUP events on control pipe descriptors caused
> premature termination due to thread and evlist pollfd index mismatches.
> This occurred because they were skipped by the early nonfilterable continue.
Do you have a concrete scenario to check this behavior so that we can
verify the fix? It'd be great if we can add a test case.
Thanks,
Namhyung
>
> Address this by refining the early continue filter to only skip system-wide
> perf events (which are nonfilterable but not non_perf_event). Control
> descriptors (non_perf_event) now fall through to appropriately have their
> fd value unset to -1 on POLLHUP while avoiding their destructors. Finally,
> maintain the invariant that the active event counter (nr) increments strictly
> and only for completely filterable events.
>
> 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/lib/api/fd/array.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index 67b73481df27..3681ad7c6527 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -116,14 +116,23 @@ 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)
> + /*
> + * System-wide perf events are nonfilterable but not non_perf_event.
> + * We want to skip them entirely and never process revents on them.
> + */
> + if ((fda->priv[fd].flags & fdarray_flag__nonfilterable) &&
> + !(fda->priv[fd].flags & fdarray_flag__non_perf_event))
> continue;
>
> if (!fda->entries[fd].events)
> continue;
>
> if (fda->entries[fd].revents & revents) {
> - if (entry_destructor)
> + /*
> + * Control descriptors are non_perf_event and don't need
> + * their perf-specific destructors triggered.
> + */
> + if (entry_destructor && !(fda->priv[fd].flags & fdarray_flag__non_perf_event))
> entry_destructor(fda, fd, arg);
>
> /*
> @@ -136,7 +145,8 @@ int fdarray__filter(struct fdarray *fda, short revents,
> continue;
> }
>
> - ++nr;
> + if (!(fda->priv[fd].flags & fdarray_flag__nonfilterable))
> + ++nr;
> }
>
> return nr;
> --
> 2.55.0.699.gb54405d56f-goog
>
next prev parent reply other threads:[~2026-08-17 23:17 UTC|newest]
Thread overview: 20+ 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 [this message]
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
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=aoOWh-e5BR2PX27Q@google.com \
--to=namhyung@kernel.org \
--cc=abyssmystery@gmail.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.