From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Thomas Falcon <thomas.falcon@intel.com>,
Dapeng Mi <dapeng1.mi@linux.intel.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] perf parse-events: Make X modifier more respectful of groups
Date: Fri, 17 Oct 2025 16:03:57 -0700 [thread overview]
Message-ID: <20251017230357.15663-1-irogers@google.com> (raw)
Events with an X modifier were reordered within a group, for example
slots was made the leader in:
```
$ perf record -e '{cpu/mem-stores/ppu,cpu/slots/uX}' -- sleep 1
```
Fix by making `dont_regroup` evsels always use their index for
sorting. Make the cur_leader, when fixing the groups, be that of
`dont_regroup` evsel so that the `dont_regroup` evsel doesn't become a
leader.
On a tigerlake this patch corrects this and meets expectations in:
```
$ perf stat -e '{cpu/mem-stores/,cpu/slots/uX}' -a -- sleep 0.1
Performance counter stats for 'system wide':
83,458,652 cpu/mem-stores/
2,720,854,880 cpu/slots/uX
0.103780587 seconds time elapsed
$ perf stat -e 'slots,slots:X' -a -- sleep 0.1
Performance counter stats for 'system wide':
732,042,247 slots (48.96%)
643,288,155 slots:X (51.04%)
0.102731018 seconds time elapsed
```
Closes: https://lore.kernel.org/lkml/18f20d38-070c-4e17-bc90-cf7102e1e53d@linux.intel.com/
Fixes: 035c17893082 ("perf parse-events: Add 'X' modifier to exclude an event from being regrouped")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/parse-events.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3aec86aebdc6..0c0dc20b1c13 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1973,14 +1973,18 @@ static int evlist__cmp(void *_fg_idx, const struct list_head *l, const struct li
* event's index is used. An index may be forced for events that
* must be in the same group, namely Intel topdown events.
*/
- if (*force_grouped_idx != -1 && arch_evsel__must_be_in_group(lhs)) {
+ if (lhs->dont_regroup) {
+ lhs_sort_idx = lhs_core->idx;
+ } else if (*force_grouped_idx != -1 && arch_evsel__must_be_in_group(lhs)) {
lhs_sort_idx = *force_grouped_idx;
} else {
bool lhs_has_group = lhs_core->leader != lhs_core || lhs_core->nr_members > 1;
lhs_sort_idx = lhs_has_group ? lhs_core->leader->idx : lhs_core->idx;
}
- if (*force_grouped_idx != -1 && arch_evsel__must_be_in_group(rhs)) {
+ if (rhs->dont_regroup) {
+ rhs_sort_idx = rhs_core->idx;
+ } else if (*force_grouped_idx != -1 && arch_evsel__must_be_in_group(rhs)) {
rhs_sort_idx = *force_grouped_idx;
} else {
bool rhs_has_group = rhs_core->leader != rhs_core || rhs_core->nr_members > 1;
@@ -2078,10 +2082,10 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
*/
idx = 0;
list_for_each_entry(pos, list, core.node) {
- const struct evsel *pos_leader = evsel__leader(pos);
+ struct evsel *pos_leader = evsel__leader(pos);
const char *pos_pmu_name = pos->group_pmu_name;
const char *cur_leader_pmu_name;
- bool pos_force_grouped = force_grouped_idx != -1 &&
+ bool pos_force_grouped = force_grouped_idx != -1 && !pos->dont_regroup &&
arch_evsel__must_be_in_group(pos);
/* Reset index and nr_members. */
@@ -2095,8 +2099,8 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
* groups can't span PMUs.
*/
if (!cur_leader || pos->dont_regroup) {
- cur_leader = pos;
- cur_leaders_grp = &pos->core;
+ cur_leader = pos->dont_regroup ? pos_leader : pos;
+ cur_leaders_grp = &cur_leader->core;
if (pos_force_grouped)
force_grouped_leader = pos;
}
--
2.51.0.858.gf9c4a03a3a-goog
next reply other threads:[~2025-10-17 23:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-17 23:03 Ian Rogers [this message]
2025-10-20 3:00 ` Mi, Dapeng
2025-10-21 2:55 ` Namhyung Kim
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=20251017230357.15663-1-irogers@google.com \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=dapeng1.mi@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=thomas.falcon@intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®