[PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up

Ian Rogers posted 5 patches 3 months, 2 weeks ago
tools/perf/tests/builtin-test.c | 69 +++++++++++++++++++++++++++++++++
tools/perf/tests/code-reading.c |  7 ----
tools/perf/tests/hwmon_pmu.c    | 11 +++---
tools/perf/util/dso.c           |  4 ++
tools/perf/util/hwmon_pmu.c     | 38 +++++++++++++-----
tools/perf/util/hwmon_pmu.h     |  4 +-
tools/perf/util/pmus.c          |  2 +-
tools/perf/util/pmus.h          |  2 +-
tools/perf/util/symbol.c        |  1 +
9 files changed, 112 insertions(+), 26 deletions(-)
[PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
Posted by Ian Rogers 3 months, 2 weeks ago
Some recent patches showed we were leaking file descriptors:
https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/

When a test is forked the file descriptors >3 can be closed and then
any file descriptors >3 left after the test are leaks. Add this
checking to the forked test code. Prior to that clean up some file
descriptor usage so we can assert that only file descriptors are
cleaned up. Sometimes the file descriptor being held open is the
result of a memory leak, so fix those.

Ian Rogers (5):
  perf dso: Add missed dso__put to dso__load_kcore
  perf test code-reading: Avoid a leak of cpus and threads
  perf hwmon_pmu: Hold path rather than fd
  perf dso: With ref count checking, avoid dso_data holding dso live
  perf test: In forked mode add check that fds aren't leaked

 tools/perf/tests/builtin-test.c | 69 +++++++++++++++++++++++++++++++++
 tools/perf/tests/code-reading.c |  7 ----
 tools/perf/tests/hwmon_pmu.c    | 11 +++---
 tools/perf/util/dso.c           |  4 ++
 tools/perf/util/hwmon_pmu.c     | 38 +++++++++++++-----
 tools/perf/util/hwmon_pmu.h     |  4 +-
 tools/perf/util/pmus.c          |  2 +-
 tools/perf/util/pmus.h          |  2 +-
 tools/perf/util/symbol.c        |  1 +
 9 files changed, 112 insertions(+), 26 deletions(-)

-- 
2.50.0.714.g196bf9f422-goog
Re: [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
Posted by Namhyung Kim 3 months, 1 week ago
On Tue, Jun 24, 2025 at 12:03:20PM -0700, Ian Rogers wrote:
> Some recent patches showed we were leaking file descriptors:
> https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/
> 
> When a test is forked the file descriptors >3 can be closed and then
> any file descriptors >3 left after the test are leaks. Add this
> checking to the forked test code. Prior to that clean up some file
> descriptor usage so we can assert that only file descriptors are
> cleaned up. Sometimes the file descriptor being held open is the
> result of a memory leak, so fix those.

Interesting, I can see a few more tests are failing with this.  But we
can figure it out later.

Thanks,
Namhyung

> 
> Ian Rogers (5):
>   perf dso: Add missed dso__put to dso__load_kcore
>   perf test code-reading: Avoid a leak of cpus and threads
>   perf hwmon_pmu: Hold path rather than fd
>   perf dso: With ref count checking, avoid dso_data holding dso live
>   perf test: In forked mode add check that fds aren't leaked
> 
>  tools/perf/tests/builtin-test.c | 69 +++++++++++++++++++++++++++++++++
>  tools/perf/tests/code-reading.c |  7 ----
>  tools/perf/tests/hwmon_pmu.c    | 11 +++---
>  tools/perf/util/dso.c           |  4 ++
>  tools/perf/util/hwmon_pmu.c     | 38 +++++++++++++-----
>  tools/perf/util/hwmon_pmu.h     |  4 +-
>  tools/perf/util/pmus.c          |  2 +-
>  tools/perf/util/pmus.h          |  2 +-
>  tools/perf/util/symbol.c        |  1 +
>  9 files changed, 112 insertions(+), 26 deletions(-)
> 
> -- 
> 2.50.0.714.g196bf9f422-goog
>
Re: [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
Posted by Ian Rogers 3 months, 1 week ago
On Wed, Jul 2, 2025 at 7:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Tue, Jun 24, 2025 at 12:03:20PM -0700, Ian Rogers wrote:
> > Some recent patches showed we were leaking file descriptors:
> > https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/
> >
> > When a test is forked the file descriptors >3 can be closed and then
> > any file descriptors >3 left after the test are leaks. Add this
> > checking to the forked test code. Prior to that clean up some file
> > descriptor usage so we can assert that only file descriptors are
> > cleaned up. Sometimes the file descriptor being held open is the
> > result of a memory leak, so fix those.
>
> Interesting, I can see a few more tests are failing with this.  But we
> can figure it out later.

That's cool. I was a little disappointed that just the dso kcore leak
was found by this. I was also surprised that the dso kcore memory leak
hadn't shown up with leak sanitizer and reference count checking. Let
me know if there is anything more I need to do to the patch series.

Thanks,
Ian
Re: [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
Posted by Namhyung Kim 3 months ago
On Wed, Jul 02, 2025 at 08:03:08PM -0700, Ian Rogers wrote:
> On Wed, Jul 2, 2025 at 7:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Tue, Jun 24, 2025 at 12:03:20PM -0700, Ian Rogers wrote:
> > > Some recent patches showed we were leaking file descriptors:
> > > https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/
> > >
> > > When a test is forked the file descriptors >3 can be closed and then
> > > any file descriptors >3 left after the test are leaks. Add this
> > > checking to the forked test code. Prior to that clean up some file
> > > descriptor usage so we can assert that only file descriptors are
> > > cleaned up. Sometimes the file descriptor being held open is the
> > > result of a memory leak, so fix those.
> >
> > Interesting, I can see a few more tests are failing with this.  But we
> > can figure it out later.
> 
> That's cool. I was a little disappointed that just the dso kcore leak
> was found by this. I was also surprised that the dso kcore memory leak
> hadn't shown up with leak sanitizer and reference count checking. Let
> me know if there is anything more I need to do to the patch series.

Applied to perf-tools-next, thanks!

Best Regards,
Namhyung