[PATCH] perf stat: Count kernel events in a group led by a tool event

[PATCH] perf stat: Count kernel events in a group led by a tool event

[Hang Zhengyang via B4 Relay]

From: Hang Zhengyang <hang.zhengyang1010@gmail.com>

A tool event like user_time has no perf event in the kernel, but
get_group_fd() still passes its fd, the /proc/stat fd, as the group_fd
of the members.  The kernel rejects each member with EBADF and nothing
is counted:

  $ perf stat -a -e '{user_time,cycles,instructions}' -- sleep 0.05
                   0      user_time
     <not supported>      cycles
     <not supported>      instructions

Open the members with group_fd = -1, as the enable/disable code already
assumes, and don't give them PERF_FORMAT_GROUP since they are now read
on their own:

  $ perf stat -a -e '{user_time,cycles,instructions}' -- sleep 0.05
                   0      user_time
            42637254      cycles
            28279395      instructions

The kernel events in such a group are no longer scheduled together.
perf only builds these groups when the user writes them; metrics move
tool events out of their groups.

Fixes: 6828d6929b76 ("perf evsel: Refactor tool events")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Hang Zhengyang <hang.zhengyang1010@gmail.com>
---
This goes on top of TAKANO's segfault fix, as asked in that thread.

With -a, this counted before 6828d6929b76 too, because the cpu mismatch
check split the group up, so the kernel events weren't scheduled
together then either.  Ian, is that fine, or would you rather have the
first kernel event lead the rest?
---
 tools/perf/builtin-stat.c | 2 +-
 tools/perf/util/evsel.c   | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 37752d6bfef4..59890916dcc4 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -721,7 +721,7 @@ static int create_perf_stat_counter(struct evsel *evsel,
 	 * the group read (for leader) and ID retrieval for all
 	 * members.
 	 */
-	if (leader->core.nr_members > 1)
+	if (leader->core.nr_members > 1 && !evsel__is_non_perf_event_open_pmu(leader))
 		attr->read_format |= PERF_FORMAT_ID|PERF_FORMAT_GROUP;
 
 	attr->inherit = !config->no_inherit && list_empty(&evsel->bpf_counter_list);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 560741d9599d..5a919303562a 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2378,6 +2378,10 @@ static int get_group_fd(struct evsel *evsel, int cpu_map_idx, int thread)
 	if (!evsel->supported || evsel__is_group_leader(evsel))
 		return -1;
 
+	/* A userspace PMU leader has no perf event to group with */
+	if (evsel__is_non_perf_event_open_pmu(leader))
+		return -1;
+
 	/*
 	 * Leader must be already processed/open,
 	 * if not it's a bug.

---
base-commit: cc905a18a647a8e3ad42cd1596baa0e08fba26eb
change-id: 20261111-tool-pmu-leader-d25fe8153599
prerequisite-message-id: <20261023085629.1694941-1-takano32@gmail.com>
prerequisite-patch-id: b92ce8b0c6248a4af182670458912b760b98a410

Best regards,
-- 
Hang Zhengyang <hang.zhengyang1010@gmail.com>

Re: [PATCH] perf stat: Count kernel events in a group led by a tool event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] perf stat: missing disabled/enable_on_exec in
  create_perf_stat_counter()
- [Medium] perf evsel: get_group_fd() ignores !leader->supported for
  userspace PMU
- [Medium] perf stat: incomplete enable/disable for members under tool
  leader
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] perf stat: mixed group read and disabled flaw in
  create_perf_stat_counter() [1]
--

commit 498a1414507f8fb0fa9be7cd1e97fec9e69bc393
Author: Hang Zhengyang <hang.zhengyang1010@gmail.com>

perf stat: Count kernel events in a group led by a tool event

This commit opens kernel member events with group_fd = -1 and omits
PERF_FORMAT_GROUP when their group leader is a non-perf_event_open PMU
event, allowing them to be opened and read individually.

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 37752d6bfef45..59890916dcc4a 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -721,7 +721,7 @@ static int create_perf_stat_counter(struct evsel *evsel,
>  	 * the group read (for leader) and ID retrieval for all
>  	 * members.
>  	 */
> -	if (leader->core.nr_members > 1)
> +	if (leader->core.nr_members > 1 && !evsel__is_non_perf_event_open_pmu(leader))
>  		attr->read_format |= PERF_FORMAT_ID|PERF_FORMAT_GROUP;
>  
>  	attr->inherit = !config->no_inherit && list_empty(&evsel->bpf_counter_list);

[Severity: Medium]
Should create_perf_stat_counter() (and evsel__config()) also set
attr->disabled = 1 and attr->enable_on_exec = 1 when
evsel__is_non_perf_event_open_pmu(leader) is true?

tools/perf/builtin-stat.c:create_perf_stat_counter() {
	...
	if (evsel__is_group_leader(evsel)) {
		attr->disabled = 1;

		if (target__enable_on_exec(&target))
			attr->enable_on_exec = 1;
	}
	...
}

Because non-leader members of a group led by a tool event have
evsel__is_group_leader(evsel) == false while get_group_fd() now returns
-1, sys_perf_event_open() opens them as standalone kernel events with
attr->disabled = 0 and attr->enable_on_exec = 0.  This regression causes
those counters to start counting immediately inside sys_perf_event_open(),
recording events during setup, across initial delays (-D), and before
execvp() when profiling a child workload.

[Severity: Medium]
This problem wasn't introduced by this patch, but when a kernel PMU event
leads a mixed group containing a tool PMU member [1], does
PERF_FORMAT_GROUP still fail to read the group?

evsel__group_read_nr_members() and evsel__group_has_tpebs() only exclude
retire_lat events and do not exclude evsel__is_non_perf_event_open_pmu()
members, so evsel__group_read_size() and evsel__process_group_data()
expect more members than the kernel group contains and fail the read.  In
addition, because create_perf_stat_counter() only sets attr->disabled = 1
on the group leader, a subordinate tool PMU member opens with
attr->disabled = 0 in evsel__tool_pmu_open() and begins timing immediately
before evlist__enable() runs.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 560741d9599d2..5a919303562a0 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -2378,6 +2378,10 @@ static int get_group_fd(struct evsel *evsel, int cpu_map_idx, int thread)
>  	if (!evsel->supported || evsel__is_group_leader(evsel))
>  		return -1;
>  
> +	/* A userspace PMU leader has no perf event to group with */
> +	if (evsel__is_non_perf_event_open_pmu(leader))
> +		return -1;
> +
>  	/*
>  	 * Leader must be already processed/open,
>  	 * if not it's a bug.

[Severity: Medium]
What happens in get_group_fd() if the userspace PMU leader failed to open
and leader->supported is false?

Returning -1 instead of -2 bypasses the broken-leader check in
evsel__open_cpu(), so the member events still open with core.leader
pointing to the unsupported leader.  In perf stat, __run_perf_stat() frees
leader->core.fd and in perf record (e.g. perf record -e
'{user_time,cycles}'), record__open() removes the failed leader via
evlist__remove(), while __evlist__enable() and __evlist__disable() skip
the non-leader members and leave them impossible to enable or disable.

[Severity: Medium]
Can kernel members on CPUs 1..N-1 be enabled or disabled when the group
leader is a tool event like duration_time?

perf_pmu__reads_only_on_cpu_idx0() is true for duration_time, so
__perf_evlist__propagate_maps() restricts leader->core.cpus to CPU index 0
while kernel members keep all target CPUs.  Because __evlist__enable() and
__evlist__disable() skip non-leader events (!evsel__is_group_leader(pos))
and only delegate to members from evsel__enable_cpu() and
evsel__disable_cpu() on the leader's CPUs, member counters opened on CPUs
1..N-1 are never enabled or disabled.

[1] https://sashiko.dev/#/bug/linux-ff5d792a-1b0a-41d1-9760-725926f4f860

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-tool-pmu-leader-v1-1-6173b5569f59@gmail.com?part=1