Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH v2] perf stat: Count kernel events in a group led by a tool event
@ 2026-11-11  6:04 Hang Zhengyang via B4 Relay
  2026-11-11  6:18 ` sashiko-bot
  2026-11-11 11:06 ` [PATCH v3] " TAKANO Mitsuhiro
  0 siblings, 2 replies; 5+ messages in thread
From: Hang Zhengyang via B4 Relay @ 2026-11-11  6:04 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark
  Cc: linux-perf-users, linux-kernel, TAKANO Mitsuhiro, Hang Zhengyang

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.  They are then on their own, so read them without
PERF_FORMAT_GROUP and open them disabled like a leader, so that -D,
--control and enable_on_exec still apply.  Keep reporting a broken
group when the leader failed to open.

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

The kernel events in such a group are no longer scheduled together.

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.
A tool event as a member, as in '{cycles,user_time}', is still not
counted; I'll send that separately.

Ian, the other way is to take tool events out of such groups in perf
stat with a warning, the way mismatched cpus are handled, which also
covers the member case.  I went with the group_fd = -1 that
0b18bced5444 describes; happy to switch if you'd rather split them.
---
Changes in v2:
- Open the members disabled like a leader, so -D, --control and
  enable_on_exec apply to them (sashiko)
- Report a broken group when the leader failed to open, as before, so
  perf record is unchanged (sashiko)
- Limit to tool event leaders: a hwmon leader can't enable its members
- Link to v1: https://lore.kernel.org/r/20261111-tool-pmu-leader-v1-1-6173b5569f59@gmail.com
---
 tools/perf/builtin-stat.c | 10 ++++++----
 tools/perf/util/evsel.c   |  8 ++++++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 37752d6bfef4..07d01028bdea 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -719,9 +719,10 @@ static int create_perf_stat_counter(struct evsel *evsel,
 	/*
 	 * The event is part of non trivial group, let's enable
 	 * the group read (for leader) and ID retrieval for all
-	 * members.
+	 * members.  Members of a tool event leader are opened and
+	 * read on their own.
 	 */
-	if (leader->core.nr_members > 1)
+	if (leader->core.nr_members > 1 && !evsel__is_tool(leader))
 		attr->read_format |= PERF_FORMAT_ID|PERF_FORMAT_GROUP;
 
 	attr->inherit = !config->no_inherit && list_empty(&evsel->bpf_counter_list);
@@ -748,9 +749,10 @@ static int create_perf_stat_counter(struct evsel *evsel,
 	/*
 	 * Disabling all counters initially, they will be enabled
 	 * either manually by us or by kernel via enable_on_exec
-	 * set later.
+	 * set later.  Members of a tool event leader are opened on
+	 * their own and enabled with it, so they need it too.
 	 */
-	if (evsel__is_group_leader(evsel)) {
+	if (evsel__is_group_leader(evsel) || evsel__is_tool(leader)) {
 		attr->disabled = 1;
 
 		if (target__enable_on_exec(&target))
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 560741d9599d..e3ed645cc289 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2378,6 +2378,14 @@ 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 tool event leader has no perf event to group with, so open
+	 * the member on its own, or report the group as broken (-2) if
+	 * the leader failed to open.
+	 */
+	if (evsel__is_tool(leader))
+		return leader->supported ? -1 : -2;
+
 	/*
 	 * 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>



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] perf stat: Count kernel events in a group led by a tool event
  2026-11-11  6:04 [PATCH v2] perf stat: Count kernel events in a group led by a tool event Hang Zhengyang via B4 Relay
@ 2026-11-11  6:18 ` sashiko-bot
  2026-11-11 11:06 ` [PATCH v3] " TAKANO Mitsuhiro
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-11  6:18 UTC (permalink / raw)
  To: Hang Zhengyang; +Cc: linux-perf-users

> 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.  They are then on their own, so read them without
> PERF_FORMAT_GROUP and open them disabled like a leader, so that -D,
> --control and enable_on_exec still apply.  Keep reporting a broken
> group when the leader failed to open.
> 
>   $ perf stat -a -e '{user_time,cycles,instructions}' -- sleep 0.05
>                    0      user_time
>             38257394      cycles
>             21228043      instructions
> 
> The kernel events in such a group are no longer scheduled together.
> 
> Fixes: 6828d6929b76 ("perf evsel: Refactor tool events")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Hang Zhengyang <hang.zhengyang1010@gmail.com>

Sashiko has reviewed this patch and found no issues.

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3] perf stat: Count kernel events in a group led by a tool event
  2026-11-11  6:04 [PATCH v2] perf stat: Count kernel events in a group led by a tool event Hang Zhengyang via B4 Relay
  2026-11-11  6:18 ` sashiko-bot
@ 2026-11-11 11:06 ` TAKANO Mitsuhiro
  2026-11-11 11:33   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: TAKANO Mitsuhiro @ 2026-11-11 11:06 UTC (permalink / raw)
  To: linux-perf-users, Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Hang Zhengyang, linux-kernel, TAKANO Mitsuhiro

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 read them on their own: the leader is not asked for counts
of events that are not in its kernel group.  A kernel event member is
opened disabled, as a leader is, so that it starts with the workload
and still honours -D and --control.  A tool event member is left as it
is: it counts from open and an explicit disable is what stops it, so
disabling it here would leave nobody to start it when perf relies on
enable_on_exec.

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

The kernel events in such a group are no longer scheduled together,
which is the price of counting them at all.  A tool event used as a
member, as in '{cycles,user_time}', is still not counted; that also
needs the leader's group read to be handled and is left for a
follow-up.

Fixes: 6828d6929b76 ("perf evsel: Refactor tool events")
Written with LLM assistants: the group_fd change and its testing by
Hang Zhengyang with Claude, the v3 condition and its measurement by
TAKANO Mitsuhiro with Hermes Agent.  perf was built here and
'perf test -v 10' passes with and without the change.
Signed-off-by: Hang Zhengyang <hang.zhengyang1010@gmail.com>
Signed-off-by: TAKANO Mitsuhiro <takano32@gmail.com>
---
v3: open only the kernel event members disabled.  A tool event member
    counts from open and an explicit disable is what stops it, so
    opening it disabled left nobody to start it when perf relies on
    enable_on_exec: 'perf stat -e "{user_time,system_time}" -- sleep
    0.2' went from ~2.7 ms of system_time to 0 on an arm64 KVM guest.
    The non-standard Assisted-by tag is replaced by a prose
    disclosure, and the patch is regenerated on 7.4-rc2.

    The libperf fix that stops the NULL dereference is a separate
    patch (<20261111101625.2204836-1-takano32@gmail.com>); this one
    does not depend on it.

 tools/perf/builtin-stat.c | 16 ++++++++++++----
 tools/perf/util/evsel.c   |  8 ++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 37752d6bfef4..57956f55ea09 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -719,9 +719,10 @@ static int create_perf_stat_counter(struct evsel *evsel,
 	/*
 	 * The event is part of non trivial group, let's enable
 	 * the group read (for leader) and ID retrieval for all
-	 * members.
+	 * members.  Members of a tool event leader are opened and
+	 * read on their own.
 	 */
-	if (leader->core.nr_members > 1)
+	if (leader->core.nr_members > 1 && !evsel__is_tool(leader))
 		attr->read_format |= PERF_FORMAT_ID|PERF_FORMAT_GROUP;
 
 	attr->inherit = !config->no_inherit && list_empty(&evsel->bpf_counter_list);
@@ -748,9 +749,16 @@ static int create_perf_stat_counter(struct evsel *evsel,
 	/*
 	 * Disabling all counters initially, they will be enabled
 	 * either manually by us or by kernel via enable_on_exec
-	 * set later.
+	 * set later.  A kernel event that a tool event leader has as
+	 * a member is not in the kernel's group either, so it has to
+	 * be opened disabled to start together with the workload.
+	 * A tool event member counts from open and is only stopped by
+	 * an explicit disable, so leaving it enabled here keeps its
+	 * count; disabling it would leave nobody to start it when
+	 * perf relies on enable_on_exec.
 	 */
-	if (evsel__is_group_leader(evsel)) {
+	if (evsel__is_group_leader(evsel) ||
+	    (evsel__is_tool(leader) && !evsel__is_tool(evsel))) {
 		attr->disabled = 1;
 
 		if (target__enable_on_exec(&target))
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d060fca8090f..7f514cde6840 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2370,6 +2370,14 @@ 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 tool event leader has no perf event to group with, so open
+	 * the member on its own, or report the group as broken (-2) if
+	 * the leader failed to open.
+	 */
+	if (evsel__is_tool(leader))
+		return leader->supported ? -1 : -2;
+
 	/*
 	 * Leader must be already processed/open,
 	 * if not it's a bug.

base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] perf stat: Count kernel events in a group led by a tool event
  2026-11-11 11:06 ` [PATCH v3] " TAKANO Mitsuhiro
@ 2026-11-11 11:33   ` sashiko-bot
  2026-11-12  3:49     ` Hang Zhengyang
  0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-11-11 11:33 UTC (permalink / raw)
  To: TAKANO Mitsuhiro, linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] perf stat: broken tool event enable/disable for -D,
  --control, and exec in create_perf_stat_counter()
--

commit 0e981d5b2ece1f32ac47445309993697c6cdb096
Author: Hang Zhengyang <hang.zhengyang1010@gmail.com>

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

This commit opens members of a group led by a tool event on their own
with group_fd = -1 and without PERF_FORMAT_GROUP, while opening kernel
event members initially disabled.

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 37752d6bfef45..57956f55ea09b 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
[ ... ]
> @@ -748,9 +749,16 @@ static int create_perf_stat_counter(struct evsel *evsel,
>  	/*
>  	 * Disabling all counters initially, they will be enabled
>  	 * either manually by us or by kernel via enable_on_exec
> -	 * set later.
> +	 * set later.  A kernel event that a tool event leader has as
> +	 * a member is not in the kernel's group either, so it has to
> +	 * be opened disabled to start together with the workload.
> +	 * A tool event member counts from open and is only stopped by
> +	 * an explicit disable, so leaving it enabled here keeps its
> +	 * count; disabling it would leave nobody to start it when
> +	 * perf relies on enable_on_exec.
>  	 */
> -	if (evsel__is_group_leader(evsel)) {
> +	if (evsel__is_group_leader(evsel) ||
> +	    (evsel__is_tool(leader) && !evsel__is_tool(evsel))) {
>  		attr->disabled = 1;
>  
>  		if (target__enable_on_exec(&target))

[Severity: Medium]
Does conditioning attr->disabled on leader vs. member rather than
target__enable_on_exec(&target) leave tool event members enabled during
-D / --control and leave tool event leaders disabled during exec
workloads?

In evsel__tool_pmu_open(), attr->disabled sets evsel->disabled = true and
start_time = INVALID_START_TIME; otherwise evsel->disabled is false and
start_time is sampled immediately at open.

When !target__enable_on_exec(&target) (such as with -D or --control),
enable_counters() calls evlist__enable(), and evsel__enable_cpu() enables
each member of a tool-led group via evsel__tool_pmu_enable_cpu():

tools/perf/util/tool_pmu.c:evsel__tool_pmu_enable_cpu() {
	if (!evsel->disabled)
		return 0;
    ...
}

Because a tool event member in a group like '{user_time,system_time}' is
skipped by !evsel__is_tool(evsel) and opened with attr->disabled == 0,
evsel__tool_pmu_enable_cpu() returns early without resetting start_time,
so the member counts continuously through the disabled delay window.

Conversely, when target__enable_on_exec(&target) is true (such as
'perf stat -e "{user_time,cycles}" -- sleep 0.2'), enable_counters() skips
evlist__enable() because kernel events are enabled on exec.  A tool event
leader still matches evsel__is_group_leader(evsel) and opens with
evsel->disabled = true; nothing enables it on exec, so
evsel__tool_pmu_read() returns 0.

Should tool events instead set attr->disabled = 1 only when
!target__enable_on_exec(&target), regardless of whether they are the
group leader or a member?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111110616.2249764-1-takano32@gmail.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] perf stat: Count kernel events in a group led by a tool event
  2026-11-11 11:33   ` sashiko-bot
@ 2026-11-12  3:49     ` Hang Zhengyang
  0 siblings, 0 replies; 5+ messages in thread
From: Hang Zhengyang @ 2026-11-12  3:49 UTC (permalink / raw)
  To: TAKANO Mitsuhiro, linux-perf-users
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, linux-kernel

On Wed, 11 Nov 2026 11:33:19 +0000, sashiko-bot@kernel.org wrote:
> [...] leave tool event members enabled during -D / --control and
> leave tool event leaders disabled during exec workloads?

Both happen without this patch too, with tool events outside any
group (perf-tools-next cc905a18a647):

  $ perf stat -e user_time,system_time -- dd if=/dev/zero of=/dev/null bs=1 count=300000
  -> user_time 0, system_time 0

  $ perf stat -D 400 -e user_time,system_time -- sh -c 'dd ... ; sleep 0.6'
  -> user_time 173 ms, system_time 137 ms, though dd ends within 400 ms

The first is what TAKANO's "perf stat: Start tool events when
enable_on_exec is used" fixes.

Thanks,
Hang

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-12  3:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-11  6:04 [PATCH v2] perf stat: Count kernel events in a group led by a tool event Hang Zhengyang via B4 Relay
2026-11-11  6:18 ` sashiko-bot
2026-11-11 11:06 ` [PATCH v3] " TAKANO Mitsuhiro
2026-11-11 11:33   ` sashiko-bot
2026-11-12  3:49     ` Hang Zhengyang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox