Re: [PATCH v4] perf fdarray: Fix destructor invocation and event counting in fdarray__filter

On Mon, Aug 17, 2026 at 4:17 PM Namhyung Kim <[email protected]> wrote:
>
> 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.

So the fix is trying to address hangs I see in the TPEBS test. We
should be able to add a C unit test. I can do that while I wrangle
with Sashiko.

Thanks,
Ian

> 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 <[email protected]>
> > ---
> >  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
> >