Re: [PATCH v4 0/2] perf tool_pmu: Support enable/disable for tool PMU events

[Ian Rogers]

On Mon, May 18, 2026 at 6:41 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> A regression in perf stat was reported where tool PMU events (like
> duration_time used in CPUs_utilized metric) incorrectly included the
> delay period when using the delay option (-D).
>
> This series fixes the regression by making tool PMU events
> (duration_time, user_time, system_time) behave more like regular
> counters by implementing proper enable and disable support. They now
> correctly accumulate values only when enabled.
>
> The first patch implements the core enable/disable support for tool PMU
> events, and the second patch adds a shell test to verify that
> duration_time correctly excludes the delay period.

Would be nice to get this reported bug fix landed.

Thanks,
Ian

> Changes in v4:
> - Update evsel->disabled immediately after the leader's own enable/disable
>   succeeds in evsel__enable() / evsel__disable(), preventing state
>   inconsistency on early return if a member fails.
> - Remove evsel->disabled state changes from within the CPU loops in
>   evsel__tool_pmu_enable_cpu() and evsel__tool_pmu_disable_cpu() to
>   prevent __evlist__disable() from skipping disable_cpu calls for other
>   CPUs.
> - Make *start_time = INVALID_START_TIME unconditional in
>   evsel__tool_pmu_disable_cpu() to ensure safe inactive state invalidation.
> - Address a checkpatch warning regarding unnecessary braces for a single
>   statement if block.
>
> Changes in v3:
> - Refine group handling: only manually enable/disable group members when
>   the leader or member is a non-perf-event open PMU, as the kernel allows
>   grouping of software and hardware PMUs.
> - Fix a file descriptor leak in evsel__tool_pmu_open() on error paths by
>   explicitly closing the successfully opened fd before exiting.
> - Synchronize the 'disabled' flag for all group members in enable/disable
>   paths (both per-CPU and batch loops) to prevent stale disabled flags.
> - Add explicit early exits to evsel__tool_pmu_enable() and disable() based
>   on evsel->disabled to protect internal metric state.
> - Add an upper bound check in test_stat_delay to verify that the delay was
>   actually excluded.
>
> Changes in v2:
> - Implement evsel__tool_pmu_enable() and evsel__tool_pmu_disable() to
>   avoid ioctl failures in batch evsel__enable() and evsel__disable()
>   functions.
> - Correctly iterate and enable/disable tool PMU events configured as
>   non-leader members of event groups.
> - Correct the lseek() arguments order in the read_stat helper:
>   lseek(fd, 0, SEEK_SET) instead of lseek(fd, SEEK_SET, 0).
> - Introduce INVALID_START_TIME (~0ULL) to prevent erroneous large delta
>   accumulation in evsel__tool_pmu_read() if /proc/<pid>/stat fails to
>   read in enable_cpu (e.g., process exited).
> - Improve test parsing to use LC_ALL=C and cut to be robust against
>   different locales, and use awk to dynamically compare duration_time to
>   time elapsed with a 200ms tolerance (avoiding loaded CI false failures).
>   Also added a lower-bound check.
> - Fix style warnings from checkpatch.pl (line lengths, braces, and blank
>   lines).
>
> Ian Rogers (2):
>   perf tool_pmu: Make tool PMU events respect enable/disable
>   perf tests: Add test for stat delay option with duration_time
>
>  tools/perf/tests/shell/stat.sh |  53 +++++++
>  tools/perf/util/evlist.c       |  10 +-
>  tools/perf/util/evsel.c        | 197 ++++++++++++++++++++------
>  tools/perf/util/evsel.h        |  15 +-
>  tools/perf/util/tool_pmu.c     | 250 ++++++++++++++++++++++++++-------
>  tools/perf/util/tool_pmu.h     |   4 +
>  6 files changed, 430 insertions(+), 99 deletions(-)
>
> --
> 2.54.0.631.ge1b05301d1-goog
>