All of lore.kernel.org
 help / color / mirror / Atom feed
From: TAKANO Mitsuhiro <takano32@gmail.com>
To: linux-perf-users@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Hang Zhengyang <hang.zhengyang1010@gmail.com>,
	linux-kernel@vger.kernel.org,
	TAKANO Mitsuhiro <takano32@gmail.com>
Subject: [PATCH v3] perf stat: Count kernel events in a group led by a tool event
Date: Wed, 11 Nov 2026 11:06:16 +0000	[thread overview]
Message-ID: <20261111110616.2249764-1-takano32@gmail.com> (raw)
In-Reply-To: <20261111-tool-pmu-leader-v2-1-3faa0fb98c51@gmail.com>

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


  parent reply	other threads:[~2026-11-11 11:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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:04 ` Hang Zhengyang
2026-11-11  6:18 ` sashiko-bot
2026-11-11 11:06 ` TAKANO Mitsuhiro [this message]
2026-11-11 11:33   ` [PATCH v3] " sashiko-bot
2026-11-12  3:49     ` Hang Zhengyang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261111110616.2249764-1-takano32@gmail.com \
    --to=takano32@gmail.com \
    --cc=acme@kernel.org \
    --cc=hang.zhengyang1010@gmail.com \
    --cc=irogers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.