* [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match
2026-08-24 6:37 ` [PATCH v1 0/7] perf ui: Implement hybrid event merging for heterogeneous systems Ian Rogers
@ 2026-08-24 6:37 ` Ian Rogers
2026-08-24 6:53 ` sashiko-bot
2026-08-24 22:49 ` Andi Kleen
2026-08-24 6:37 ` [PATCH v1 2/7] perf ui hist: Add support for aggregated total_period and merging entries cleanly Ian Rogers
` (5 subsequent siblings)
6 siblings, 2 replies; 31+ messages in thread
From: Ian Rogers @ 2026-08-24 6:37 UTC (permalink / raw)
To: andi; +Cc: acme, ak, ak, linux-perf-users, namhyung, Ian Rogers
Add logic to dynamically identify mergeable events spawned from the same
wildcard alias via first_wildcard_match, breaking reliance on hardcoded
PMU metrics or type IDs.
Signed-off-by: Ian Rogers <irogers@google.com>
Assisted-by: Antigravity:gemini-3.1-pro
---
tools/perf/util/evlist.c | 208 ++++++++++++++++++++++++++++++++++
tools/perf/util/evlist.h | 2 +
tools/perf/util/evsel.c | 2 +
tools/perf/util/hist.h | 1 +
tools/perf/util/symbol_conf.h | 1 +
5 files changed, 214 insertions(+)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index c3d784727810..431759e05e26 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include "util/pmu.h"
/*
* Copyright (C) 2011, Red Hat Inc, Arnaldo Carvalho de Melo <acme@redhat.com>
*
@@ -6,7 +7,9 @@
* copyright notes.
*/
#include "evlist.h"
+#include "hist.h"
+#include <stdio.h>
#include <errno.h>
#include <inttypes.h>
#include <signal.h>
@@ -125,12 +128,21 @@ struct evlist *evlist__new_default(const struct target *target, bool sample_call
if (err)
goto out_err;
} else {
+ struct evsel *leader = NULL;
while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
snprintf(buf, sizeof(buf), "%s/cycles/%s", pmu->name,
can_profile_kernel ? "P" : "Pu");
err = parse_event(evlist, buf);
if (err)
goto out_err;
+ if (!leader)
+ leader = evlist__last(evlist);
+ else {
+ struct evsel *last = evlist__last(evlist);
+
+ if (last != leader)
+ last->first_wildcard_match = leader;
+ }
}
}
@@ -148,6 +160,202 @@ struct evlist *evlist__new_default(const struct target *target, bool sample_call
return NULL;
}
+
+/**
+ * is_pmu_core_len - Check if a given string prefix matches a core PMU name.
+ * @name: The string to check.
+ * @len: The length of the PMU name prefix in the string.
+ *
+ * This function is used instead of the global `is_pmu_core()` from pmu.h
+ * because it operates natively on substrings without requiring null-termination
+ * (e.g. strndup allocations) when parsing event names like "cpu_core/cycles/".
+ */
+static bool is_pmu_core_len(const char *name, size_t len)
+{
+ struct perf_pmu *pmu = NULL;
+
+ while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
+ if (!strncmp(name, pmu->name, len) && strlen(pmu->name) == len)
+ return true;
+ }
+ return false;
+}
+
+/*
+ * evlist__can_merge_hybrid - check if hybrid events can be merged.
+ * @evlist: The evlist to check.
+ *
+ * This code is valid for perf record, top, etc. as the event parsing
+ * will set first_wildcard_match. The perf.data case (e.g. perf report)
+ * recomputes the first_wildcard_match in the case there are none by
+ * falling back to string matches only in the case of core events on
+ * hybrid systems.
+ */
+bool evlist__can_merge_hybrid(struct evlist *evlist)
+{
+ struct evsel *pos;
+ unsigned int nr = 0;
+ bool has_wildcard = false;
+
+ evlist__for_each_entry(evlist, pos) {
+ if (evsel__is_dummy_event(pos))
+ continue;
+ if (pos->first_wildcard_match)
+ has_wildcard = true;
+ nr++;
+ }
+
+ if (!has_wildcard) {
+ evlist__for_each_entry(evlist, pos) {
+ const char *pos_name;
+ char *pos_match;
+ struct evsel *peer;
+
+ if (evsel__is_dummy_event(pos) || pos->first_wildcard_match)
+ continue;
+
+ pos_name = evsel__name(pos);
+ pos_match = strchr(pos_name, '/');
+ if (!pos_match)
+ continue;
+
+ /* If evsel->core.is_pmu_core missing in report, fallback to prefix */
+ if (!evsel__is_hybrid(pos)) {
+ if (!is_pmu_core_len(pos_name, pos_match - pos_name) ||
+ perf_pmus__num_core_pmus() <= 1)
+ continue;
+ }
+
+ peer = pos;
+ list_for_each_entry_continue(peer, &evlist->core.entries, core.node) {
+ const char *peer_name;
+ char *peer_match;
+
+ if (evsel__is_dummy_event(peer) || peer->first_wildcard_match)
+ continue;
+
+ peer_name = evsel__name(peer);
+ peer_match = strchr(peer_name, '/');
+ if (!peer_match)
+ continue;
+
+ if (!evsel__is_hybrid(peer)) {
+ if (!is_pmu_core_len(peer_name, peer_match - peer_name) ||
+ perf_pmus__num_core_pmus() <= 1)
+ continue;
+ }
+
+ if (!strcmp(pos_match, peer_match)) {
+ peer->first_wildcard_match = pos;
+ has_wildcard = true;
+ }
+ }
+ }
+ }
+
+ return has_wildcard && (nr > 1);
+}
+
+/*
+ * evlist__merge_hybrid - group hybrid events logically together.
+ * @evlist: The evlist containing events to merge.
+ *
+ * Iterates through the evlist and logically merges associated hybrid events
+ * by assigning their first_wildcard_match as their core group leader,
+ * modifying their presentation into a single merged histogram view.
+ */
+void evlist__merge_hybrid(struct evlist *evlist, bool refresh_hists)
+{
+ struct evsel *pos, *tmp;
+ int idx = 0;
+
+ evlist__for_each_entry_safe(evlist, tmp, pos) {
+ if (evsel__is_dummy_event(pos))
+ continue;
+
+ if (pos->first_wildcard_match) {
+ struct evsel *leader = evsel__leader(pos->first_wildcard_match);
+ struct evsel *old_leader = evsel__leader(pos);
+
+ if (old_leader != leader) {
+ struct evsel *member;
+
+ if (old_leader != pos)
+ old_leader->core.nr_members--;
+ pos->core.leader = &leader->core;
+ /* Base is 1 to natively represent the leader */
+ if (leader->core.nr_members == 0)
+ leader->core.nr_members = 1;
+ leader->core.nr_members++;
+
+ /* Assign stranded members to the new leader as well */
+ for_each_group_member(member, pos) {
+ if (!member->first_wildcard_match) {
+ member->core.leader = &leader->core;
+ leader->core.nr_members++;
+ }
+ }
+ }
+ }
+ }
+
+ {
+ struct list_head new_list;
+ struct evsel *member, *mtmp;
+
+ INIT_LIST_HEAD(&new_list);
+
+ while (!list_empty(&evlist->core.entries)) {
+ pos = list_first_entry(&evlist->core.entries, struct evsel, core.node);
+ list_move_tail(&pos->core.node, &new_list);
+
+ list_for_each_entry_safe(member, mtmp, &evlist->core.entries, core.node) {
+ if (member->core.leader == &pos->core)
+ list_move_tail(&member->core.node, &new_list);
+ }
+ }
+ list_splice_init(&new_list, &evlist->core.entries);
+ }
+
+ evlist__for_each_entry(evlist, pos)
+ pos->core.idx = idx++;
+
+ /* Set merge_entries flag on leaders */
+ evlist__for_each_entry(evlist, pos) {
+ if (evsel__is_dummy_event(pos))
+ continue;
+ if (pos->core.leader == &pos->core && pos->core.nr_members > 1) {
+ evsel__hists(pos)->merge_entries = true;
+ symbol_conf.event_group = true;
+ symbol_conf.hybrid_merge = true;
+ }
+ }
+
+ if (!refresh_hists)
+ return;
+
+ evlist__for_each_entry(evlist, pos) {
+ /* Match histograms dynamically since parsing happened before group toggling */
+ if (symbol_conf.event_group && !evsel__is_group_leader(pos)) {
+ struct hists *leader_hists = evsel__hists(evsel__leader(pos));
+ struct hists *hists = evsel__hists(pos);
+
+ hists__match(leader_hists, hists);
+ hists__link(leader_hists, hists);
+ }
+ }
+
+ /* Now that links are formed, safely resort the active tree so the UI renders accurately */
+ if (symbol_conf.event_group) {
+ evlist__for_each_entry(evlist, pos) {
+ if (evsel__is_dummy_event(pos) || !evsel__is_group_leader(pos))
+ continue;
+ if (pos->core.nr_members > 1)
+ hists__output_resort(evsel__hists(pos), NULL);
+ }
+ }
+}
+
struct evlist *evlist__new_dummy(void)
{
struct evlist *evlist = evlist__new();
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 838e263b76f3..12f3fd7dad9b 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -331,6 +331,8 @@ static inline void evlist__set_selected(struct evlist *evlist, struct evsel *evs
struct evlist *evlist__new(void);
struct evlist *evlist__new_default(const struct target *target, bool sample_callchains);
+bool evlist__can_merge_hybrid(struct evlist *evlist);
+void evlist__merge_hybrid(struct evlist *evlist, bool refresh_hists);
struct evlist *evlist__new_dummy(void);
struct evlist *evlist__get(struct evlist *evlist);
void evlist__put(struct evlist *evlist);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d4cb455f4a7d..3f56a0e6f9d6 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2052,6 +2052,8 @@ static void evsel__exit(struct evsel *evsel)
evsel__free_config_terms(evsel);
cgroup__put(evsel->cgrp);
perf_evsel__exit(&evsel->core);
+ if (evsel->first_wildcard_match)
+ evsel->first_wildcard_match = NULL;
zfree(&evsel->group_name);
zfree(&evsel->name);
#ifdef HAVE_LIBTRACEEVENT
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index b830cbe7f95b..ea79628bbc6b 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -130,6 +130,7 @@ struct hists {
struct hists_stats stats;
u64 event_stream;
u16 col_len[HISTC_NR_COLS];
+ bool merge_entries;
bool has_callchains;
int socket_filter;
struct perf_hpp_list *hpp_list;
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index 0dee5aa6a534..2bdd96fe886f 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -28,6 +28,7 @@ enum a2l_style {
#define MAX_A2L_STYLE (A2L_STYLE_CMD + 1)
struct symbol_conf {
+ bool hybrid_merge;
bool nanosecs;
unsigned short priv_size;
bool try_vmlinux_path,
--
2.55.0.766.g2966f0265a-goog
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match
2026-08-24 6:37 ` [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match Ian Rogers
@ 2026-08-24 6:53 ` sashiko-bot
2026-08-24 22:49 ` Andi Kleen
1 sibling, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-08-24 6:53 UTC (permalink / raw)
To: Ian Rogers; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `first_wildcard_match` field in `struct evsel` is assigned as a raw pointer without incrementing the reference count of the target `evsel`, risking a use-after-free if the target is freed.
--
--- Patch [1]: [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match ---
commit fefc17e1de9b340818e3dce0b74337b9595b7aa3
Author: Ian Rogers <irogers@google.com>
perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match
Add logic to dynamically identify mergeable events spawned from the same
wildcard alias via first_wildcard_match, breaking reliance on hardcoded
PMU metrics or type IDs.
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index c3d7847278106..431759e05e260 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
[ ... ]
> @@ -125,12 +128,21 @@ struct evlist *evlist__new_default(const struct target *target, bool sample_call
> if (err)
> goto out_err;
> } else {
> + struct evsel *leader = NULL;
> while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> snprintf(buf, sizeof(buf), "%s/cycles/%s", pmu->name,
> can_profile_kernel ? "P" : "Pu");
> err = parse_event(evlist, buf);
> if (err)
> goto out_err;
> + if (!leader)
> + leader = evlist__last(evlist);
> + else {
> + struct evsel *last = evlist__last(evlist);
> +
> + if (last != leader)
> + last->first_wildcard_match = leader;
[Severity: High]
Should we be taking a reference here with evsel__get()? Assigning the leader
as a raw pointer without a reference count could lead to a use-after-free if
the leader event is removed or freed before this member event.
> + }
> }
> }
[ ... ]
> @@ -148,6 +160,202 @@ struct evlist *evlist__new_default(const struct target *target, bool sample_call
[ ... ]
> + if (!evsel__is_hybrid(peer)) {
> + if (!is_pmu_core_len(peer_name, peer_match - peer_name) ||
> + perf_pmus__num_core_pmus() <= 1)
> + continue;
> + }
> +
> + if (!strcmp(pos_match, peer_match)) {
> + peer->first_wildcard_match = pos;
[Severity: High]
Similar to above, could this raw pointer assignment leave a dangling pointer
in peer->first_wildcard_match if the matching event (pos) is freed later?
Using evsel__get() and evsel__put() might be needed for reference counting.
> + has_wildcard = true;
> + }
> + }
> + }
> + }
> +
> + return has_wildcard && (nr > 1);
> +}
[ ... ]
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d4cb455f4a7d4..3f56a0e6f9d65 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -2052,6 +2052,8 @@ static void evsel__exit(struct evsel *evsel)
> evsel__free_config_terms(evsel);
> cgroup__put(evsel->cgrp);
> perf_evsel__exit(&evsel->core);
> + if (evsel->first_wildcard_match)
> + evsel->first_wildcard_match = NULL;
[Severity: High]
If first_wildcard_match is holding a reference, would we need evsel__put()
here instead of just setting it to NULL? Also, this only clears its own
pointer, but doesn't it leave dangling pointers in other events that might
have this evsel set as their first_wildcard_match?
> zfree(&evsel->group_name);
> zfree(&evsel->name);
> #ifdef HAVE_LIBTRACEEVENT
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824063744.1533837-1-irogers@google.com?part=1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match
2026-08-24 6:37 ` [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match Ian Rogers
2026-08-24 6:53 ` sashiko-bot
@ 2026-08-24 22:49 ` Andi Kleen
2026-08-25 4:01 ` Ian Rogers
1 sibling, 1 reply; 31+ messages in thread
From: Andi Kleen @ 2026-08-24 22:49 UTC (permalink / raw)
To: Ian Rogers; +Cc: acme, ak, ak, linux-perf-users, namhyung
On Sun, Aug 23, 2026 at 11:37:38PM -0700, Ian Rogers wrote:
> Add logic to dynamically identify mergeable events spawned from the same
> wildcard alias via first_wildcard_match, breaking reliance on hardcoded
> PMU metrics or type IDs.
I'm not fully sure what first wildcard match is. That means just
matching on the name? FWIW I think it's ok for common cases, but there
are definitely also cases where the same event doesn't quite mean the
same on hybrid cores.
but anyways as long as it works for "cycles" it's fine because
that's nearly everyone uses with perf top.
-Andi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match
2026-08-24 22:49 ` Andi Kleen
@ 2026-08-25 4:01 ` Ian Rogers
0 siblings, 0 replies; 31+ messages in thread
From: Ian Rogers @ 2026-08-25 4:01 UTC (permalink / raw)
To: Andi Kleen, acme, namhyung; +Cc: ak, ak, linux-perf-users
On Mon, Aug 24, 2026 at 3:49 PM Andi Kleen <andi@firstfloor.org> wrote:
>
> On Sun, Aug 23, 2026 at 11:37:38PM -0700, Ian Rogers wrote:
> > Add logic to dynamically identify mergeable events spawned from the same
> > wildcard alias via first_wildcard_match, breaking reliance on hardcoded
> > PMU metrics or type IDs.
>
> I'm not fully sure what first wildcard match is. That means just
> matching on the name? FWIW I think it's ok for common cases, but there
> are definitely also cases where the same event doesn't quite mean the
> same on hybrid cores.
So you've pretty much got it from the name, which I'm glad about. The
first wildcard match originally came from events like data_read on
memory controller PMUs. We have 1 evsel per PMU, and the first PMU the
wildcard-ing (originally done with fnmatch) hits is special due to the
aggregation of counts in perf stat. Now it doesn't make sense to say
aggregate memory controller events with data_read say from a storage
device PMU, so there is fix up of things done in
parse_events__sort_events_and_fix_groups:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n2126
parse_events__sort_events_and_fix_groups is too complicated but a
large part of that is down to perf metric events and we've covered it
in tests. I think we should revisit the one PMU per evsel at some
future date and change it to multiple PMUs per evsel, as that more
closely matches the command line parsing. The code is slowly evolving,
and I believe the memory controller PMU aggregation logic I carried
forward originated with Kan Liang.
One thing this series suffers from is determining the first wildcard
match in `perf report`, as that data isn't recorded in the perf.data
file. In that case, the 'M' logic tries to infer the first wildcard
match from the event names. If someone did `perf record -e
cpu_atom/cycles/,cpu_core/cycles/ ...` then this would look identical
to `perf record -e cycles` and so this would be an argument in keeping
the existing non-merging logic first.
> but anyways as long as it works for "cycles" it's fine because
> that's nearly everyone uses with perf top.
Well, I think that may be misguided, as users might think cycles is a
proxy for wall clock time. The lower frequency and IPC on an e-core
mean the lower counts aren't representative of wall clock time. For
this reason, I've included individual percentage breakdowns for each
event along with the total. However, since we are sorting by total,
things may still be misleading, so I've added the text in the tips.txt
file.
Anyway, this series involves a lot of work and isn't as user-friendly
as I'd hoped. For example, pressing escape from the merged view
doesn't unmerge events in perf top. I think this change is preferable
to your change because it is more generic in how events are handled
and supports `perf report`. That said, if I'm about to go 12 rounds
with Sashiko over these changes, it would be nice to have some
positivity from people who could lend the patches their tags. For me,
the status quo is tolerable, and my reason for sending these changes
was to show the direction I think the code should take. Sending a v2
of these changes is low on my priority list.
Thanks,
Ian
> -Andi
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v1 2/7] perf ui hist: Add support for aggregated total_period and merging entries cleanly
2026-08-24 6:37 ` [PATCH v1 0/7] perf ui: Implement hybrid event merging for heterogeneous systems Ian Rogers
2026-08-24 6:37 ` [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match Ian Rogers
@ 2026-08-24 6:37 ` Ian Rogers
2026-08-24 6:53 ` sashiko-bot
2026-08-24 6:37 ` [PATCH v1 3/7] perf ui browsers: Implement interactive 'M' keystroke to toggle hybrid event merging Ian Rogers
` (4 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Ian Rogers @ 2026-08-24 6:37 UTC (permalink / raw)
To: andi; +Cc: acme, ak, ak, linux-perf-users, namhyung, Ian Rogers
Implement cross-PMU histogram sorting logic and properly scale the
global total_period for symmetric percentage ceilings across differing
core types, resolving issues with --percent-limit.
Signed-off-by: Ian Rogers <irogers@google.com>
Assisted-by: Antigravity:gemini-3.1-pro
---
tools/perf/ui/hist.c | 49 ++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/hist.h | 5 +++++
2 files changed, 54 insertions(+)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index e58327595d37..09f0baa808e0 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -287,6 +287,24 @@ static int __hpp__sort(struct hist_entry *a, struct hist_entry *b,
return __hpp__group_sort_idx(a, b, get_field,
symbol_conf.group_sort_idx);
}
+ /*
+ * Relies on merge_entries being only enabled if there are
+ * only matching events. If that is ever relaxed will need
+ * more logic here.
+ */
+ if (a->hists->merge_entries && b->hists->merge_entries) {
+ u64 val_a = get_field(a), val_b = get_field(b);
+ struct hist_entry *pair;
+
+ list_for_each_entry(pair, &a->pairs.head, pairs.node)
+ val_a += get_field(pair);
+ list_for_each_entry(pair, &b->pairs.head, pairs.node)
+ val_b += get_field(pair);
+
+ ret = field_cmp(val_a, val_b);
+ if (ret)
+ return ret;
+ }
ret = field_cmp(get_field(a), get_field(b));
if (ret || !symbol_conf.event_group)
@@ -1271,3 +1289,34 @@ int perf_hpp__alloc_mem_stats(struct perf_hpp_list *list, struct evlist *evlist)
}
return 0;
}
+
+float hist_entry__get_percent_limit_merged(struct hist_entry *he)
+{
+ struct hist_entry *pair;
+ u64 period = he->stat.period;
+ u64 total_period = hists__total_period(he->hists);
+ struct evsel *evsel = hists_to_evsel(he->hists);
+ struct evsel *pos;
+
+ /* Accumulate global total_period across all merged hists */
+ for_each_group_member(pos, evsel) {
+ total_period += hists__total_period(evsel__hists(pos));
+ }
+
+ /* Accumulate symbol specific period across pairs */
+ list_for_each_entry(pair, &he->pairs.head, pairs.node) {
+ period += pair->stat.period;
+ }
+
+ if (unlikely(total_period == 0))
+ return 0;
+
+ if (symbol_conf.cumulate_callchain) {
+ period = he->stat_acc->period;
+ list_for_each_entry(pair, &he->pairs.head, pairs.node) {
+ period += pair->stat_acc->period;
+ }
+ }
+
+ return period * 100.0 / total_period;
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index ea79628bbc6b..6cb7059e8405 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -434,6 +434,8 @@ void hists__match(struct hists *leader, struct hists *other);
int hists__link(struct hists *leader, struct hists *other);
int hists__unlink(struct hists *hists);
+float hist_entry__get_percent_limit_merged(struct hist_entry *he);
+
static inline float hist_entry__get_percent_limit(struct hist_entry *he)
{
u64 period = he->stat.period;
@@ -445,6 +447,9 @@ static inline float hist_entry__get_percent_limit(struct hist_entry *he)
if (symbol_conf.cumulate_callchain)
period = he->stat_acc->period;
+ if (he->hists->merge_entries)
+ return hist_entry__get_percent_limit_merged(he);
+
return period * 100.0 / total_period;
}
--
2.55.0.766.g2966f0265a-goog
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v1 2/7] perf ui hist: Add support for aggregated total_period and merging entries cleanly
2026-08-24 6:37 ` [PATCH v1 2/7] perf ui hist: Add support for aggregated total_period and merging entries cleanly Ian Rogers
@ 2026-08-24 6:53 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-08-24 6:53 UTC (permalink / raw)
To: Ian Rogers; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] hist_entry__get_percent_limit incorrectly early-returns 0 for merged histograms when the group leader has no samples, causing valid entries to be hidden.
- [Medium] __hpp__sort_acc does not aggregate fields across pairs when merge_entries is true, causing incorrect sorting for accumulated callchains.
--
--- Patch [2]: [PATCH v1 2/7] perf ui hist: Add support for aggregated total_period and merging entries cleanly ---
commit 7077e4a680da4bc6b3a68f49b2a79f0a03e770ef
Author: Ian Rogers <irogers@google.com>
perf ui hist: Add support for aggregated total_period and merging entries cleanly
This commit implements cross-PMU histogram sorting logic to support merging
entries cleanly. It scales the global total_period to fix symmetric percentage
ceilings across different core types. This resolves issues with --percent-limit.
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index e58327595d37d..09f0baa808e0a 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -287,6 +287,24 @@ static int __hpp__sort(struct hist_entry *a, struct hist_entry *b,
> return __hpp__group_sort_idx(a, b, get_field,
> symbol_conf.group_sort_idx);
> }
> + /*
> + * Relies on merge_entries being only enabled if there are
> + * only matching events. If that is ever relaxed will need
> + * more logic here.
> + */
> + if (a->hists->merge_entries && b->hists->merge_entries) {
> + u64 val_a = get_field(a), val_b = get_field(b);
> + struct hist_entry *pair;
> +
> + list_for_each_entry(pair, &a->pairs.head, pairs.node)
> + val_a += get_field(pair);
> + list_for_each_entry(pair, &b->pairs.head, pairs.node)
> + val_b += get_field(pair);
> +
> + ret = field_cmp(val_a, val_b);
> + if (ret)
> + return ret;
> + }
[Severity: Medium]
Does __hpp__sort_acc() also need this merge_entries aggregation loop?
When sorting accumulated fields for hybrid events, hpp__sort_overhead_acc()
delegates to __hpp__sort_acc(). Since __hpp__sort_acc() lacks this
paired-summing loop, will the default `--children` sorting order evaluate only
the leader's he->stat_acc->period instead of the aggregated global samples?
[ ... ]
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index ea79628bbc6b3..6cb7059e8405f 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
[ ... ]
> @@ -445,6 +447,9 @@ static inline float hist_entry__get_percent_limit(struct hist_entry *he)
> if (symbol_conf.cumulate_callchain)
> period = he->stat_acc->period;
>
> + if (he->hists->merge_entries)
> + return hist_entry__get_percent_limit_merged(he);
> +
> return period * 100.0 / total_period;
> }
[Severity: Medium]
Will this new check be bypassed if the group leader recorded no samples?
Earlier in hist_entry__get_percent_limit(), there is an early return:
static inline float hist_entry__get_percent_limit(struct hist_entry *he)
{
u64 period = he->stat.period;
u64 total_period = hists__total_period(he->hists);
if (unlikely(total_period == 0))
return 0;
...
If total_period is zero (e.g., the leader PMU on a hybrid core had 0 samples
for a workload), the function returns early. Could this cause valid merged
histogram entries to be hidden from the UI because the new merge_entries check
is never reached?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824063744.1533837-1-irogers@google.com?part=2
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v1 3/7] perf ui browsers: Implement interactive 'M' keystroke to toggle hybrid event merging
2026-08-24 6:37 ` [PATCH v1 0/7] perf ui: Implement hybrid event merging for heterogeneous systems Ian Rogers
2026-08-24 6:37 ` [PATCH v1 1/7] perf evlist: Implement evlist__can_merge_hybrid using first_wildcard_match Ian Rogers
2026-08-24 6:37 ` [PATCH v1 2/7] perf ui hist: Add support for aggregated total_period and merging entries cleanly Ian Rogers
@ 2026-08-24 6:37 ` Ian Rogers
2026-08-24 6:49 ` sashiko-bot
2026-08-24 6:37 ` [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge Ian Rogers
` (3 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Ian Rogers @ 2026-08-24 6:37 UTC (permalink / raw)
To: andi; +Cc: acme, ak, ak, linux-perf-users, namhyung, Ian Rogers
Map the 'M' keystroke globally across the interface to toggle boolean state
dynamically rebuilding hybrid core groups independently.
This allows cleanly separating or aggregating hybrid core histograms.
Signed-off-by: Ian Rogers <irogers@google.com>
Assisted-by: Antigravity:gemini-3.1-pro
---
tools/perf/ui/browsers/hists.c | 48 +++++++++++++++++++++++++++++-----
tools/perf/util/evsel.c | 12 +++++++++
tools/perf/util/evsel.h | 2 ++
3 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index c15874a491b4..07cb06834d0f 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2840,10 +2840,10 @@ add_script_opt(struct hist_browser *browser,
return n;
j = sprintf(tstr, " in ");
j += timestamp__scnprintf_usec(he->time, tstr + j,
- sizeof tstr - j);
+ sizeof(tstr) - j);
j += sprintf(tstr + j, "-");
timestamp__scnprintf_usec(he->time + symbol_conf.time_quantum,
- tstr + j, sizeof tstr - j);
+ tstr + j, sizeof(tstr) - j);
ret = add_script_opt_2(time_act, time_optstr, thread, sym, tstr);
if (ret > 0) {
time_act->time = he->time;
@@ -3580,8 +3580,11 @@ static int perf_evsel_menu__run(struct evsel_menu *menu,
int delay_secs = hbt ? hbt->refresh : 0;
int key;
- if (ui_browser__show(&menu->b, title,
- "ESC: exit, ENTER|->: Browse histograms") < 0)
+ const char *help_msg = evlist__can_merge_hybrid(evlist) ?
+ "ESC: exit, ENTER|->: Browse histograms, M: Merge hybrid events" :
+ "ESC: exit, ENTER|->: Browse histograms";
+
+ if (ui_browser__show(&menu->b, title, help_msg) < 0)
return -1;
while (1) {
@@ -3636,6 +3639,16 @@ static int perf_evsel_menu__run(struct evsel_menu *menu,
goto out;
case K_ESC:
default:
+ if (key == 'M') {
+ if (evlist__can_merge_hybrid(evlist)) {
+ if (!symbol_conf.hybrid_merge)
+ evlist__merge_hybrid(evlist, true);
+ symbol_conf.hybrid_merge =
+ !symbol_conf.hybrid_merge;
+ ui_browser__hide(&menu->b);
+ return K_RELOAD;
+ }
+ }
continue;
}
case K_LEFT:
@@ -3649,6 +3662,15 @@ static int perf_evsel_menu__run(struct evsel_menu *menu,
case CTRL('c'):
goto out;
default:
+ if (key == 'M') {
+ if (evlist__can_merge_hybrid(evlist)) {
+ if (!symbol_conf.hybrid_merge)
+ evlist__merge_hybrid(evlist, true);
+ symbol_conf.hybrid_merge = !symbol_conf.hybrid_merge;
+ ui_browser__hide(&menu->b);
+ return K_RELOAD;
+ }
+ }
ui_browser__warn_unhandled_hotkey(&menu->b, key, delay_secs, NULL);
continue;
}
@@ -3675,6 +3697,8 @@ static int __evlist__tui_browse_hists(struct evlist *evlist, int nr_entries, con
bool warn_lost_event)
{
struct evsel *pos;
+ int ret;
+
struct evsel_menu menu = {
.b = {
.entries = &evlist__core(evlist)->entries,
@@ -3699,8 +3723,11 @@ static int __evlist__tui_browse_hists(struct evlist *evlist, int nr_entries, con
menu.b.width = line_len;
}
- return perf_evsel_menu__run(&menu, nr_entries, help,
+ ret = perf_evsel_menu__run(&menu, nr_entries, help,
hbt, warn_lost_event);
+
+
+ return ret;
}
static bool evlist__single_entry(struct evlist *evlist)
@@ -3720,10 +3747,14 @@ static bool evlist__single_entry(struct evlist *evlist)
return false;
}
+
int evlist__tui_browse_hists(struct evlist *evlist, const char *help, struct hist_browser_timer *hbt,
float min_pcnt, struct perf_env *env, bool warn_lost_event)
{
int nr_entries = evlist__nr_entries(evlist);
+ int ret;
+
+retry:
if (evlist__single_entry(evlist)) {
single_entry: {
@@ -3747,8 +3778,13 @@ single_entry: {
goto single_entry;
}
- return __evlist__tui_browse_hists(evlist, nr_entries, help, hbt, min_pcnt, env,
+ ret = __evlist__tui_browse_hists(evlist, nr_entries, help, hbt, min_pcnt, env,
warn_lost_event);
+ if (ret == K_RELOAD) {
+ nr_entries = evlist__nr_entries(evlist);
+ goto retry;
+ }
+ return ret;
}
static int block_hists_browser__title(struct hist_browser *browser, char *bf,
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3f56a0e6f9d6..a4760cfb7582 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -4758,3 +4758,15 @@ void evsel__warn_user_requested_cpus(struct evsel *evsel, struct perf_cpu_map *u
perf_cpu_map__put(intersect);
perf_cpu_map__put(online);
}
+
+struct evsel *evsel__new_dummy(void)
+{
+ struct perf_event_attr attr = {
+ .type = PERF_TYPE_SOFTWARE,
+ .config = PERF_COUNT_SW_DUMMY,
+ .size = sizeof(attr),
+ .freq = 0,
+ .sample_period = 1,
+ };
+ return evsel__new(&attr);
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index d9ecc6628217..6cf07120da9f 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -15,6 +15,8 @@
#include "symbol_conf.h"
+struct evsel *evsel__new_dummy(void);
+
struct bperf_follower_bpf;
struct bperf_leader_bpf;
struct bpf_counter_ops;
--
2.55.0.766.g2966f0265a-goog
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v1 3/7] perf ui browsers: Implement interactive 'M' keystroke to toggle hybrid event merging
2026-08-24 6:37 ` [PATCH v1 3/7] perf ui browsers: Implement interactive 'M' keystroke to toggle hybrid event merging Ian Rogers
@ 2026-08-24 6:49 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-08-24 6:49 UTC (permalink / raw)
To: Ian Rogers; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Toggling hybrid merging off via 'M' fails to reverse structural changes, leaving the evlist permanently merged.
- [High] If merging hybrid events results in a single event group, the 'M' key becomes unreachable, and pressing it causes a silent exit.
- [High] Intercepting K_RELOAD inside the UI browser breaks critical caller-level session reloads.
- [High] Dynamically calling `evlist__merge_hybrid` from the TUI thread modifies `evlist->core.entries` and rbtrees without locks, causing race conditions.
- [Low] The newly introduced function `evsel__new_dummy()` is dead code.
--
--- Patch [3]: [PATCH v1 3/7] perf ui browsers: Implement interactive 'M' keystroke to toggle hybrid event merging ---
commit 761e0204fddbaaaabd4cf218b352b3c2ddbaf235
Author: Ian Rogers <irogers@google.com>
perf ui browsers: Implement interactive 'M' keystroke to toggle hybrid event merging
Map the 'M' keystroke globally across the interface to toggle boolean state
dynamically rebuilding hybrid core groups independently.
This allows cleanly separating or aggregating hybrid core histograms.
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index c15874a491b43..07cb06834d0f4 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
[ ... ]
> @@ -3636,6 +3639,16 @@ static int perf_evsel_menu__run(struct evsel_menu *menu,
> goto out;
> case K_ESC:
> default:
> + if (key == 'M') {
> + if (evlist__can_merge_hybrid(evlist)) {
> + if (!symbol_conf.hybrid_merge)
> + evlist__merge_hybrid(evlist, true);
> + symbol_conf.hybrid_merge =
> + !symbol_conf.hybrid_merge;
[Severity: High]
Does toggling hybrid merging off via 'M' fail to reverse the structural
changes?
When symbol_conf.hybrid_merge is toggled from true to false, the state is
updated and K_RELOAD is returned, but there is no call to an unmerge function.
Since evlist__merge_hybrid() irreversibly mutates pos->core.leader and the
doubly linked evlist->core.entries, wouldn't the histograms remain permanently
merged under the hood?
[Severity: High]
Is it safe to dynamically call evlist__merge_hybrid() from the TUI thread
without locks?
In perf top, a background process_thread continuously mutates evlist and its
child rbtrees while holding hists->lock. This call locklessly manipulates the
doubly linked evlist->core.entries and modifies hists nodes. If the background
thread accesses the evlist concurrently, could this cause list corruption or
crashes?
> + ui_browser__hide(&menu->b);
> + return K_RELOAD;
> + }
> + }
> continue;
> }
> case K_LEFT:
[ ... ]
> @@ -3720,10 +3747,14 @@ static bool evlist__single_entry(struct evlist *evlist)
> return false;
> }
>
> +
> int evlist__tui_browse_hists(struct evlist *evlist, const char *help, struct hist_browser_timer *hbt,
> float min_pcnt, struct perf_env *env, bool warn_lost_event)
> {
> int nr_entries = evlist__nr_entries(evlist);
> + int ret;
> +
> +retry:
>
> if (evlist__single_entry(evlist)) {
> single_entry: {
> @@ -3747,8 +3778,13 @@ single_entry: {
> goto single_entry;
> }
>
> - return __evlist__tui_browse_hists(evlist, nr_entries, help, hbt, min_pcnt, env,
> + ret = __evlist__tui_browse_hists(evlist, nr_entries, help, hbt, min_pcnt, env,
> warn_lost_event);
> + if (ret == K_RELOAD) {
> + nr_entries = evlist__nr_entries(evlist);
> + goto retry;
[Severity: High]
Does this logic trap the user if merging hybrid events results in a single
event group?
If evlist__merge_hybrid() coalesces all events into a single group, nr_entries
will evaluate to 1. On K_RELOAD, this loop goes to retry, which then jumps
directly to the single_entry block and bypasses the perf_evsel_menu__run() menu
loop. Since the single entry view lacks the 'M' handler, pressing 'M' again
would act as an unhandled hotkey and cause a silent exit.
[Severity: High]
Does intercepting K_RELOAD inside the UI browser break critical caller-level
session reloads?
Callers like display_thread_tui() in builtin-top.c previously relied on
receiving K_RELOAD to execute full session re-initialization (e.g., zeroing
histograms). By swallowing the signal with this goto retry loop, does this
force an internal redraw with stale structures instead of yielding control to
the caller?
> + }
> + return ret;
> }
>
> static int block_hists_browser__title(struct hist_browser *browser, char *bf,
[ ... ]
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 3f56a0e6f9d65..a4760cfb7582a 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -4758,3 +4758,15 @@ void evsel__warn_user_requested_cpus(struct evsel *evsel, struct perf_cpu_map *u
> perf_cpu_map__put(intersect);
> perf_cpu_map__put(online);
> }
> +
> +struct evsel *evsel__new_dummy(void)
> +{
> + struct perf_event_attr attr = {
> + .type = PERF_TYPE_SOFTWARE,
> + .config = PERF_COUNT_SW_DUMMY,
> + .size = sizeof(attr),
> + .freq = 0,
> + .sample_period = 1,
> + };
> + return evsel__new(&attr);
> +}
[Severity: Low]
Is this newly introduced function evsel__new_dummy() dead code?
It is defined and exported here, but it doesn't appear to be called anywhere in
the patch series.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824063744.1533837-1-irogers@google.com?part=3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge
2026-08-24 6:37 ` [PATCH v1 0/7] perf ui: Implement hybrid event merging for heterogeneous systems Ian Rogers
` (2 preceding siblings ...)
2026-08-24 6:37 ` [PATCH v1 3/7] perf ui browsers: Implement interactive 'M' keystroke to toggle hybrid event merging Ian Rogers
@ 2026-08-24 6:37 ` Ian Rogers
2026-08-24 6:52 ` sashiko-bot
2026-08-24 22:40 ` Andi Kleen
2026-08-24 6:37 ` [PATCH v1 5/7] perf Documentation: Add tip for hybrid event merging Ian Rogers
` (2 subsequent siblings)
6 siblings, 2 replies; 31+ messages in thread
From: Ian Rogers @ 2026-08-24 6:37 UTC (permalink / raw)
To: andi; +Cc: acme, ak, ak, linux-perf-users, namhyung, Ian Rogers
Add --hybrid-merge flag to explicitly trigger event merging
non-interactively
for both perf-top and perf-report.
Signed-off-by: Ian Rogers <irogers@google.com>
Assisted-by: Antigravity:gemini-3.1-pro
---
tools/perf/Documentation/perf-report.txt | 5 +++++
tools/perf/Documentation/perf-top.txt | 5 +++++
tools/perf/builtin-report.c | 6 ++++++
tools/perf/builtin-top.c | 9 ++++++++-
tools/perf/util/symbol.c | 1 +
5 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 22f87eaa3279..b7775b9260b7 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -573,6 +573,11 @@ include::itrace.txt[]
When displaying traceevent output, do not use print fmt or plugins.
-H::
+--hybrid-merge::
+ Merge matching events from all hybrid core PMUs into one
+ display. For example, if a wildcard expands to run on both p-cores and
+ e-cores, this aggregates them into a single view.
+
--hierarchy::
Enable hierarchical output. In the hierarchy mode, each sort key groups
samples based on the criteria and then sub-divide it using the lower
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index af3e4230c72f..05fe65cd77c5 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -43,6 +43,11 @@ Default is to monitor all CPUS.
encoding with the layout of the event control registers as described
by entries in /sys/bus/event_source/devices/cpu/format/*.
+--hybrid-merge::
+ Merge matching events from all hybrid core PMUs into one
+ display. For example, if a wildcard expands to run on both p-cores and
+ e-cores, this aggregates them into a single view.
+
--filter=<filter>::
Event filter. This option should follow an event selector (-e). For
syntax see linkperf:perf-record[1].
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 60d1f166629e..2fc81f4517e9 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1113,6 +1113,10 @@ static int __cmd_report(struct report *rep)
evlist__for_each_entry(session->evlist, pos)
rep->nr_entries += evsel__hists(pos)->nr_entries;
+ if (symbol_conf.hybrid_merge && evlist__can_merge_hybrid(session->evlist)) {
+ evlist__merge_hybrid(session->evlist, false);
+ }
+
if (use_browser == 0) {
if (verbose > 3)
perf_session__fprintf(session, stdout);
@@ -1446,6 +1450,8 @@ int cmd_report(int argc, const char **argv)
parse_branch_mode),
OPT_BOOLEAN(0, "branch-history", &branch_call_mode,
"add last branch records to call history"),
+ OPT_BOOLEAN(0, "hybrid-merge", &symbol_conf.hybrid_merge,
+ "merge the same event across hybrid core PMUs"),
OPT_STRING(0, "objdump", &objdump_path, "path",
"objdump binary to use for disassembly and annotations"),
OPT_STRING(0, "addr2line", &addr2line_path, "path",
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 570410599f1b..cf7ac2712696 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -320,7 +320,7 @@ static void perf_top__resort_hists(struct perf_top *t)
static void perf_top__print_sym_table(struct perf_top *top)
{
- char bf[160];
+ char bf[512];
int printed = 0;
const int win_width = top->winsize.ws_col - 1;
struct evsel *evsel = top->sym_evsel;
@@ -1321,6 +1321,9 @@ static int __cmd_top(struct perf_top *top)
if (ret)
return ret;
+ if (symbol_conf.hybrid_merge && evlist__can_merge_hybrid(top->evlist))
+ evlist__merge_hybrid(top->evlist, false);
+
top->session->evlist = top->evlist;
perf_session__set_id_hdr_size(top->session);
@@ -1490,6 +1493,8 @@ int cmd_top(int argc, const char **argv)
OPT_CALLBACK('e', "event", &parse_events_option_args, "event",
"event selector. use 'perf list' to list available events",
parse_events_option),
+ OPT_BOOLEAN(0, "hybrid-merge", &symbol_conf.hybrid_merge,
+ "merge the same event across hybrid core PMUs"),
OPT_CALLBACK(0, "filter", &top.evlist, "filter",
"event filter", parse_filter),
OPT_U64('c', "count", &opts->user_interval, "event period to sample"),
@@ -1724,6 +1729,8 @@ int cmd_top(int argc, const char **argv)
evlist__put(def_evlist);
}
+
+
status = evswitch__init(&top.evswitch, top.evlist, stderr);
if (status)
goto out_put_evlist;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 94f9c8faedda..5cbb899e8fbd 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -73,6 +73,7 @@ struct symbol_conf symbol_conf = {
.symfs = "",
.symfs_layout_flat = false,
.event_group = true,
+ .hybrid_merge = false,
.inline_name = true,
.res_sample = 0,
.addr2line_timeout_ms = 5 * 1000,
--
2.55.0.766.g2966f0265a-goog
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge
2026-08-24 6:37 ` [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge Ian Rogers
@ 2026-08-24 6:52 ` sashiko-bot
2026-08-24 22:40 ` Andi Kleen
1 sibling, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-08-24 6:52 UTC (permalink / raw)
To: Ian Rogers; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] `perf top --hybrid-merge` fails to enable events on non-leader hybrid PMUs (e.g., e-cores) when a target is specified, resulting in zero samples from those PMUs.
- [Medium] The `--hybrid-merge` option relies on the host's sysfs to detect hybrid PMUs, breaking cross-platform offline analysis in `perf report`.
--
--- Patch [4]: [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge ---
commit 81cddf1e4d0d6711f1be8b496730e2ec2a6af8cd
Author: Ian Rogers <irogers@google.com>
perf tools: Expose opt-in --hybrid-merge
Add --hybrid-merge flag to explicitly trigger event merging non-interactively
for both perf-top and perf-report.
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 60d1f166629e0..2fc81f4517e9d 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1113,6 +1113,10 @@ static int __cmd_report(struct report *rep)
> evlist__for_each_entry(session->evlist, pos)
> rep->nr_entries += evsel__hists(pos)->nr_entries;
>
> + if (symbol_conf.hybrid_merge && evlist__can_merge_hybrid(session->evlist)) {
[Severity: Medium]
Will this break cross-platform offline analysis for perf report?
When a user runs perf report --hybrid-merge on a non-hybrid host to analyze
data recorded on a hybrid host, evlist__can_merge_hybrid() calls
perf_pmus__num_core_pmus(). That function scans the host machine's sysfs
instead of the perf.data topology:
tools/perf/util/pmus.c:perf_pmus__num_core_pmus() {
...
while ((pmu = perf_pmus__scan_core(pmu)) != NULL)
count++;
...
}
Because this relies on host sysfs rather than dynamically inspecting the
perf_env from the session, it seems it will return 1 on non-hybrid hosts,
causing the merge to be skipped entirely.
> + evlist__merge_hybrid(session->evlist, false);
> + }
> +
> if (use_browser == 0) {
> if (verbose > 3)
> perf_session__fprintf(session, stdout);
[ ... ]
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 570410599f1b3..cf7ac2712696e 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1321,6 +1321,9 @@ static int __cmd_top(struct perf_top *top)
> if (ret)
> return ret;
>
> + if (symbol_conf.hybrid_merge && evlist__can_merge_hybrid(top->evlist))
> + evlist__merge_hybrid(top->evlist, false);
> +
[Severity: Medium]
Does merging events here cause e-core events to remain disabled when a target
is specified?
If a target is specified (like -p <pid>), events are initially opened in a
disabled state. When evlist__merge_hybrid() logically reparents e-core events
to the p-core event leader in user-space, they lose their leader status.
Later in __cmd_top(), evlist__enable() is called, which relies on
__evlist__enable():
tools/perf/util/evlist.c:__evlist__enable() {
...
if (!evsel__is_group_leader(pos) || !pos->core.fd)
continue;
...
}
Since the e-core event is no longer a group leader in user-space, it gets
skipped. Because PMU events cannot cross PMU boundaries in the same hardware
group, the p-core's enable ioctl won't enable the e-core event, meaning we
could drop all samples from the e-cores.
> top->session->evlist = top->evlist;
> perf_session__set_id_hdr_size(top->session);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824063744.1533837-1-irogers@google.com?part=4
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge
2026-08-24 6:37 ` [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge Ian Rogers
2026-08-24 6:52 ` sashiko-bot
@ 2026-08-24 22:40 ` Andi Kleen
2026-08-25 3:33 ` Ian Rogers
1 sibling, 1 reply; 31+ messages in thread
From: Andi Kleen @ 2026-08-24 22:40 UTC (permalink / raw)
To: Ian Rogers; +Cc: acme, ak, ak, linux-perf-users, namhyung
On Sun, Aug 23, 2026 at 11:37:41PM -0700, Ian Rogers wrote:
> Add --hybrid-merge flag to explicitly trigger event merging
> non-interactively
> for both perf-top and perf-report.
Well it should be default on.
I don't want another option I have to always specify like
--no-children.
-Andi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge
2026-08-24 22:40 ` Andi Kleen
@ 2026-08-25 3:33 ` Ian Rogers
2026-08-25 22:19 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 31+ messages in thread
From: Ian Rogers @ 2026-08-25 3:33 UTC (permalink / raw)
To: Andi Kleen; +Cc: acme, ak, ak, linux-perf-users, namhyung
On Mon, Aug 24, 2026 at 3:40 PM Andi Kleen <andi@firstfloor.org> wrote:
>
> On Sun, Aug 23, 2026 at 11:37:41PM -0700, Ian Rogers wrote:
> > Add --hybrid-merge flag to explicitly trigger event merging
> > non-interactively
> > for both perf-top and perf-report.
>
> Well it should be default on.
>
> I don't want another option I have to always specify like
> --no-children.
I agree we should be able to change defaults. Just over 2 years ago, I sent out:
https://lore.kernel.org/linux-perf-users/20240516222159.3710131-1-irogers@google.com/
That switches the histogram behavior in perf top to zero after every
refresh rather than decaying the values. If you filter perf top by
user, it can be common to get no samples. Note that around this time,
we fixed this behavior to work really nicely by using BPF instead of
horrible /proc scanning. In the default decay mode, a symbol that ran
in the past will appear in perf top even if the process with that
symbol has terminated. This happens frequently in user mode, resulting
in many ghost symbols and lingering processes. The zero mode, which
ironically matches top's behavior, isn't the default because it would
be a noticeable change in the defaults.
Anyway, this is a long winded way of saying that Namhyung and Arnaldo
pick the defaults. I didn't choose to change the defaults in this
series because I think if you test the changes, you'll see that
hitting 'M' at startup really isn't that much of a chore and it gives
a clear signal that you are doing something special. Perf top also
hints at the 'M' key along the bottom of the screen when hybrid events
are present. If this is too much to live with, I think we could make
it a config file value. I think this matches the principle of least
surprise, while the decay mode default, I'd argue, is the opposite.
Thanks,
Ian
> -Andi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge
2026-08-25 3:33 ` Ian Rogers
@ 2026-08-25 22:19 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 31+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-25 22:19 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, ak, ak, linux-perf-users, namhyung,
Linux Kernel Mailing List, Adrian Hunter, Jiri Olsa, Ingo Molnar,
Thomas Gleixner, James Clark
On Mon, Aug 24, 2026 at 08:33:02PM -0700, Ian Rogers wrote:
> On Mon, Aug 24, 2026 at 3:40 PM Andi Kleen <andi@firstfloor.org> wrote:
> > On Sun, Aug 23, 2026 at 11:37:41PM -0700, Ian Rogers wrote:
> > > Add --hybrid-merge flag to explicitly trigger event merging
> > > non-interactively
> > > for both perf-top and perf-report.
> > Well it should be default on.
> > I don't want another option I have to always specify like
> > --no-children.
Picking defaults is difficult...
Linus doesn't like the buildid cache, so he can pick something different
than the default of saving binaries that will be useful immediately
after when doing annotation, etc.
I don't like having to specify --no-children either, but lots of people
like it.
> I agree we should be able to change defaults. Just over 2 years ago, I sent out:
> https://lore.kernel.org/linux-perf-users/20240516222159.3710131-1-irogers@google.com/
> That switches the histogram behavior in perf top to zero after every
> refresh rather than decaying the values. If you filter perf top by
See? Ian has his preferences, like Andi, how come?
> user, it can be common to get no samples. Note that around this time,
> we fixed this behavior to work really nicely by using BPF instead of
> horrible /proc scanning. In the default decay mode, a symbol that ran
> in the past will appear in perf top even if the process with that
> symbol has terminated. This happens frequently in user mode, resulting
> in many ghost symbols and lingering processes. The zero mode, which
> ironically matches top's behavior, isn't the default because it would
> be a noticeable change in the defaults.
> Anyway, this is a long winded way of saying that Namhyung and Arnaldo
> pick the defaults. I didn't choose to change the defaults in this
I wish I could pick defaults wisely (or that the maintainer du jour do
it), its a hard call, and when I _do_ pick some default eventually I end
up very disappointed with myself when I find out that my thinking was
flawed.
> series because I think if you test the changes, you'll see that
> hitting 'M' at startup really isn't that much of a chore and it gives
> a clear signal that you are doing something special. Perf top also
> hints at the 'M' key along the bottom of the screen when hybrid events
> are present. If this is too much to live with, I think we could make
I don't think this is too much to live with and I I like being able to
have this switch, perhaps having a "do it permanently" after one thinks
it is the right default, should be made available.
> it a config file value. I think this matches the principle of least
> surprise, while the decay mode default, I'd argue, is the opposite.
This hybrid situation is one that is indeed a source of way too many
problems with how to properly aggregate counters from different types of
cores, having a way to go back and forth from aggregated (taking into
account freqs and whatnot) and the current per event type, which is
utterly sub-optimal for a system wide view seems to be an improvement.
Doing it just for cycles seems a step in the right direction, doing it
in a way that takes into account all types of events seems like a worthy
goal, lets make some sort of progress here with, I think, Ian's attempt
at taking into account all sorts of events while making the simple case
Andi cares to work, can we go with that?
- Arnaldo
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v1 5/7] perf Documentation: Add tip for hybrid event merging
2026-08-24 6:37 ` [PATCH v1 0/7] perf ui: Implement hybrid event merging for heterogeneous systems Ian Rogers
` (3 preceding siblings ...)
2026-08-24 6:37 ` [PATCH v1 4/7] perf tools: Expose opt-in --hybrid-merge Ian Rogers
@ 2026-08-24 6:37 ` Ian Rogers
2026-08-24 6:40 ` sashiko-bot
2026-08-24 6:37 ` [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations Ian Rogers
2026-08-24 6:37 ` [PATCH v1 7/7] perf test: Expand top tests for --hybrid-merge Ian Rogers
6 siblings, 1 reply; 31+ messages in thread
From: Ian Rogers @ 2026-08-24 6:37 UTC (permalink / raw)
To: andi; +Cc: acme, ak, ak, linux-perf-users, namhyung, Ian Rogers
Add informative text outlining the IPC imbalances associated with
merging cross-hybrid core events like cycles.
Signed-off-by: Ian Rogers <irogers@google.com>
Assisted-by: Antigravity:gemini-3.1-pro
---
tools/perf/Documentation/tips.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/Documentation/tips.txt b/tools/perf/Documentation/tips.txt
index ebf12a8c5db5..1c7b309fe6e8 100644
--- a/tools/perf/Documentation/tips.txt
+++ b/tools/perf/Documentation/tips.txt
@@ -66,3 +66,4 @@ For latency profiling, try: perf record/report --latency
For parallelism histogram, try: perf report --hierarchy --sort latency,parallelism,comm,symbol
To analyze particular parallelism levels, try: perf report --latency --parallelism=32-64
To see how parallelism changes over time, try: perf report -F time,latency,parallelism --time-quantum=1s
+When merging events like cycles, different core frequencies and instructions per cycle mean the counts may not fairly reflect time spent in a function.
--
2.55.0.766.g2966f0265a-goog
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations
2026-08-24 6:37 ` [PATCH v1 0/7] perf ui: Implement hybrid event merging for heterogeneous systems Ian Rogers
` (4 preceding siblings ...)
2026-08-24 6:37 ` [PATCH v1 5/7] perf Documentation: Add tip for hybrid event merging Ian Rogers
@ 2026-08-24 6:37 ` Ian Rogers
2026-08-24 6:53 ` sashiko-bot
2026-08-24 6:37 ` [PATCH v1 7/7] perf test: Expand top tests for --hybrid-merge Ian Rogers
6 siblings, 1 reply; 31+ messages in thread
From: Ian Rogers @ 2026-08-24 6:37 UTC (permalink / raw)
To: andi; +Cc: acme, ak, ak, linux-perf-users, namhyung, Ian Rogers
Forcefully calculate proportional visual allocations only natively for
the 'Total' merge target header instead of uniformly ballooning array
spacings. Unpack strings directly into the primary column width fn.
Signed-off-by: Ian Rogers <irogers@google.com>
Assisted-by: Antigravity:gemini-3.1-pro
---
tools/perf/ui/hist.c | 86 ++++++++++++++++++++++++++++++++--------
tools/perf/util/evlist.c | 9 +++--
tools/perf/util/hist.h | 8 +++-
3 files changed, 82 insertions(+), 21 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 09f0baa808e0..02c188100b09 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -57,8 +57,9 @@ struct hpp_fmt_value {
};
static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
- hpp_field_fn get_field, const char *fmt, int len,
- hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype)
+ hpp_field_fn get_field, const char *fmtstr, int len,
+ hpp_snprint_fn print_fn, enum perf_hpp_fmt_type fmtype,
+ struct perf_hpp_fmt *fmt)
{
int ret = 0;
struct hists *hists = he->hists;
@@ -98,14 +99,44 @@ static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
}
}
- for (i = 0; i < nr_members; i++) {
- if (symbol_conf.skip_empty &&
- values[i].hists->stats.nr_samples == 0)
- continue;
+ if (he->hists->merge_entries) {
+ u64 total_val = 0;
+ u64 total_samples = 0;
+ u64 total_period = 0;
+
+ for (i = 0; i < nr_members; i++) {
+ total_val += values[i].val;
+ total_samples += values[i].samples;
+ total_period += fmtype == PERF_HPP_FMT_TYPE__PERCENT ?
+ hists__total_period(values[i].hists) :
+ hists__total_latency(values[i].hists);
+ }
+
+ if (fmtype == PERF_HPP_FMT_TYPE__PERCENT || fmtype == PERF_HPP_FMT_TYPE__LATENCY) {
+ double percent = 0.0;
+
+ if (total_period)
+ percent = 100.0 * total_val / total_period;
+ ret += hpp__call_print_fn(hpp, print_fn, fmtstr, len, percent);
+ } else if (fmtype == PERF_HPP_FMT_TYPE__AVERAGE) {
+ double avg = total_samples ? (1.0 * total_val / total_samples) : 0;
- ret += __hpp__fmt_print(hpp, values[i].hists, values[i].val,
- values[i].samples, fmt, len,
- print_fn, fmtype);
+ ret += hpp__call_print_fn(hpp, print_fn, fmtstr, len, avg);
+ } else {
+ ret += hpp__call_print_fn(hpp, print_fn, fmtstr, len, total_val);
+ }
+ }
+
+ if (!he->hists->merge_entries || &fmt->list == he->hists->hpp_list->fields.next) {
+ for (i = 0; i < nr_members; i++) {
+ if (symbol_conf.skip_empty &&
+ values[i].hists->stats.nr_samples == 0)
+ continue;
+
+ ret += __hpp__fmt_print(hpp, values[i].hists, values[i].val,
+ values[i].samples, fmtstr, len,
+ print_fn, fmtype);
+ }
}
free(values);
@@ -129,7 +160,7 @@ int hpp__fmt(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
if (symbol_conf.field_sep) {
return __hpp__fmt(hpp, he, get_field, fmtstr, 1,
- print_fn, fmtype);
+ print_fn, fmtype, fmt);
}
if (fmtype == PERF_HPP_FMT_TYPE__PERCENT || fmtype == PERF_HPP_FMT_TYPE__LATENCY)
@@ -137,7 +168,7 @@ int hpp__fmt(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
else
len -= 1;
- return __hpp__fmt(hpp, he, get_field, fmtstr, len, print_fn, fmtype);
+ return __hpp__fmt(hpp, he, get_field, fmtstr, len, print_fn, fmtype, fmt);
}
int hpp__fmt_acc(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
@@ -399,10 +430,16 @@ static int hpp__width_fn(struct perf_hpp_fmt *fmt,
int nr = 0;
struct evsel *pos;
- for_each_group_evsel(pos, evsel) {
- if (!symbol_conf.skip_empty ||
- evsel__hists(pos)->stats.nr_samples)
- nr++;
+ if (hists->merge_entries && &fmt->list != hists->hpp_list->fields.next) {
+ nr = 1;
+ } else {
+ for_each_group_evsel(pos, evsel) {
+ if (!symbol_conf.skip_empty ||
+ evsel__hists(pos)->stats.nr_samples)
+ nr++;
+ }
+ if (hists->merge_entries && &fmt->list == hists->hpp_list->fields.next)
+ nr++; /* Add 1 extra unit of width generically for the 'Total' */
}
len = max(len, nr * fmt->len);
@@ -421,8 +458,25 @@ static int hpp__header_fn(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
int len = hpp__width_fn(fmt, hpp, hists);
const char *hdr = "";
- if (line == hists->hpp_list->nr_header_lines - 1)
+ if (line == hists->hpp_list->nr_header_lines - 1) {
hdr = fmt->name;
+ if (hists->merge_entries && &fmt->list == hists->hpp_list->fields.next) {
+ char buf[1024];
+ int w = 0;
+ int f_len = fmt->user_len ?: fmt->len;
+ struct evsel *pos, *evsel = hists_to_evsel(hists);
+
+ w += scnprintf(buf + w, sizeof(buf) - w, "%*.*s", f_len, f_len, fmt->name);
+ for_each_group_evsel(pos, evsel) {
+ if (symbol_conf.skip_empty &&
+ evsel__hists(pos)->stats.nr_samples == 0)
+ continue;
+ w += scnprintf(buf + w, sizeof(buf) - w, " %*.*s",
+ f_len - 1, f_len - 1, evsel__name(pos));
+ }
+ return scnprintf(hpp->buf, hpp->size, "%*s", len, buf);
+ }
+ }
return scnprintf(hpp->buf, hpp->size, "%*s", len, hdr);
}
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 431759e05e26..4c928ef5c985 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -324,10 +324,13 @@ void evlist__merge_hybrid(struct evlist *evlist, bool refresh_hists)
evlist__for_each_entry(evlist, pos) {
if (evsel__is_dummy_event(pos))
continue;
+
if (pos->core.leader == &pos->core && pos->core.nr_members > 1) {
evsel__hists(pos)->merge_entries = true;
symbol_conf.event_group = true;
symbol_conf.hybrid_merge = true;
+ if (!pos->group_name)
+ pos->group_name = strdup("Merged hybrid events");
}
}
@@ -345,12 +348,10 @@ void evlist__merge_hybrid(struct evlist *evlist, bool refresh_hists)
}
}
- /* Now that links are formed, safely resort the active tree so the UI renders accurately */
+ /* Resort the output dynamically since group parsing bypassed this before */
if (symbol_conf.event_group) {
evlist__for_each_entry(evlist, pos) {
- if (evsel__is_dummy_event(pos) || !evsel__is_group_leader(pos))
- continue;
- if (pos->core.nr_members > 1)
+ if (pos->core.leader == &pos->core && pos->core.nr_members > 1)
hists__output_resort(evsel__hists(pos), NULL);
}
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 6cb7059e8405..5a0cfa8ea264 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -339,7 +339,13 @@ static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
static inline void hist_entry__add_pair(struct hist_entry *pair,
struct hist_entry *he)
{
- list_add_tail(&pair->pairs.node, &he->pairs.head);
+ struct list_head *pos;
+
+ list_for_each(pos, &he->pairs.head) {
+ if (pos == &pair->pairs.node)
+ return; /* Already paired */
+ }
+ list_move_tail(&pair->pairs.node, &he->pairs.head);
}
struct hist_entry *hists__add_entry(struct hists *hists,
--
2.55.0.766.g2966f0265a-goog
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations
2026-08-24 6:37 ` [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations Ian Rogers
@ 2026-08-24 6:53 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-08-24 6:53 UTC (permalink / raw)
To: Ian Rogers; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Truncation of the `buf` array causes severe visual misalignment of columns when printing group event headers.
--
--- Patch [6]: [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations ---
commit fb91b42310113a183352643a9b299c122e218ea8
Author: Ian Rogers <irogers@google.com>
perf ui hist: Format group headers iteratively based on proportional visual allocations
Forcefully calculate proportional visual allocations only natively for
the 'Total' merge target header instead of uniformly ballooning array
spacings. Unpack strings directly into the primary column width fn.
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index 09f0baa808e0a..02c188100b098 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
[ ... ]
> @@ -421,8 +458,25 @@ static int hpp__header_fn(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> int len = hpp__width_fn(fmt, hpp, hists);
> const char *hdr = "";
>
> - if (line == hists->hpp_list->nr_header_lines - 1)
> + if (line == hists->hpp_list->nr_header_lines - 1) {
> hdr = fmt->name;
> + if (hists->merge_entries && &fmt->list == hists->hpp_list->fields.next) {
> + char buf[1024];
> + int w = 0;
> + int f_len = fmt->user_len ?: fmt->len;
> + struct evsel *pos, *evsel = hists_to_evsel(hists);
> +
> + w += scnprintf(buf + w, sizeof(buf) - w, "%*.*s", f_len, f_len, fmt->name);
> + for_each_group_evsel(pos, evsel) {
> + if (symbol_conf.skip_empty &&
> + evsel__hists(pos)->stats.nr_samples == 0)
> + continue;
> + w += scnprintf(buf + w, sizeof(buf) - w, " %*.*s",
> + f_len - 1, f_len - 1, evsel__name(pos));
> + }
> + return scnprintf(hpp->buf, hpp->size, "%*s", len, buf);
[Severity: Medium]
If the event group has many members (e.g., from a hybrid merge), can the
concatenated header string exceed the 1024 byte limit of buf array?
If the array truncates the string, but len is calculated from the total width
of all members, won't this cause the %*s specifier to right-justify the
truncated string within the larger len field?
Could this result in padding the string with leading spaces and cause severe
visual misalignment of columns in the UI output?
> + }
> + }
>
> return scnprintf(hpp->buf, hpp->size, "%*s", len, hdr);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824063744.1533837-1-irogers@google.com?part=6
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v1 7/7] perf test: Expand top tests for --hybrid-merge
2026-08-24 6:37 ` [PATCH v1 0/7] perf ui: Implement hybrid event merging for heterogeneous systems Ian Rogers
` (5 preceding siblings ...)
2026-08-24 6:37 ` [PATCH v1 6/7] perf ui hist: Format group headers iteratively based on proportional visual allocations Ian Rogers
@ 2026-08-24 6:37 ` Ian Rogers
2026-08-24 6:55 ` sashiko-bot
6 siblings, 1 reply; 31+ messages in thread
From: Ian Rogers @ 2026-08-24 6:37 UTC (permalink / raw)
To: andi; +Cc: acme, ak, ak, linux-perf-users, namhyung, Ian Rogers
Add shell test coverage for top --hybrid-merge flag to verify
correct behavior specifically on topologies featuring hybrid events.
It safely relies on fallback logic inside perf top.
Signed-off-by: Ian Rogers <irogers@google.com>
Assisted-by: Antigravity:gemini-3.1-pro
---
tools/perf/tests/shell/top.sh | 65 +++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 11 deletions(-)
diff --git a/tools/perf/tests/shell/top.sh b/tools/perf/tests/shell/top.sh
index ad7fccd09025..49820bb2d6b2 100755
--- a/tools/perf/tests/shell/top.sh
+++ b/tools/perf/tests/shell/top.sh
@@ -35,26 +35,27 @@ test_basic_perf_top() {
# Use -d 1 to avoid flooding output
# Use -e cpu-clock to ensure we get samples
# Use sleep to keep stdin open but silent, preventing EOF loop or interactive spam
- if ! sleep 10 | timeout 5s perf top --stdio -d 1 -e cpu-clock -p $PID > "${log_file}" 2>&1; then
- retval=$?
- if [ $retval -ne 124 ] && [ $retval -ne 0 ]; then
- echo "Basic perf top test [Failed: perf top failed to start or run (ret=$retval)]"
- head -n 50 "${log_file}"
- kill $PID
- wait $PID 2>/dev/null || true
- err=1
- return
- fi
+ sleep 10 | timeout 5s perf top --stdio -d 1 -e cpu-clock \
+ -p $PID > "${log_file}" 2>&1 || retval=$?
+ if [ "${retval:-0}" -ne 124 ] && [ "${retval:-0}" -ne 0 ]; then
+ echo "Basic perf top test [Failed: perf top failed to start or run (ret=$retval)]"
+ head -n 50 "${log_file}"
+ kill $PID
+ wait $PID 2>/dev/null || true
+ err=1
+ return
fi
kill $PID
wait $PID 2>/dev/null || true
+ sync
+
# Check for some sample data (percentage)
if ! grep -E -q "[0-9]+\.[0-9]+%" "${log_file}"; then
echo "Basic perf top test [Failed: no sample percentage found]"
head -n 50 "${log_file}"
- err=1
+ err=2
return
fi
@@ -69,6 +70,48 @@ test_basic_perf_top() {
echo "Basic perf top test [Success]"
}
+test_hybrid_merge_perf_top() {
+ echo "Perf top hybrid merge test"
+
+ perf test -w thloop 20 &
+ PID=$!
+
+ # Allow it to start
+ sleep 0.1
+
+ # Run without explicitly requesting -e cycles so heavily virtualized
+ # environments can seamlessly fall back to cpu-clock while real
+ # hybrid hardware will naturally cover the merge logic.
+ sleep 10 | timeout 5s perf top -e cpu-clock \
+ --stdio --hybrid-merge -d 1 -p $PID > "${log_file}" 2>&1 || retval=$?
+ if [ "${retval:-0}" -ne 124 ] && [ "${retval:-0}" -ne 0 ]; then
+ echo "Perf top hybrid merge test [Failed: run err=$retval]"
+ head -n 50 "${log_file}"
+ kill $PID
+ wait $PID 2>/dev/null || true
+ err=1
+ return
+ fi
+ sync
+
+ kill $PID
+ wait $PID 2>/dev/null || true
+
+ # Wait a tiny bit for the file system to catch up on the logs
+ sync
+
+ # Check for some sample data (percentage)
+ if ! grep -E -q "[0-9]+\.[0-9]+%" "${log_file}"; then
+ echo "Perf top hybrid merge test [Failed: no sample percentage found]"
+ head -n 50 "${log_file}"
+ err=2
+ return
+ fi
+
+ echo "Perf top hybrid merge test [Success]"
+}
+
test_basic_perf_top
+test_hybrid_merge_perf_top
cleanup
exit $err
--
2.55.0.766.g2966f0265a-goog
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v1 7/7] perf test: Expand top tests for --hybrid-merge
2026-08-24 6:37 ` [PATCH v1 7/7] perf test: Expand top tests for --hybrid-merge Ian Rogers
@ 2026-08-24 6:55 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-08-24 6:55 UTC (permalink / raw)
To: Ian Rogers; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The hybrid merge test explicitly requests the `cpu-clock` event, contradicting its design to test `--hybrid-merge` on hardware events.
- [Medium] Unnecessary system-wide `sync` commands are used to wait for local file data, causing potential CI performance degradation.
- [Medium] The global `retval` variable is conditionally assigned but not initialized before use, leaking state between consecutive tests.
--
--- Patch [7]: [PATCH v1 7/7] perf test: Expand top tests for --hybrid-merge ---
commit 83b4f9c7a4d40ea456ef8050c21d34919fb655e1
Author: Ian Rogers <irogers@google.com>
perf test: Expand top tests for --hybrid-merge
This commit adds shell test coverage for the top --hybrid-merge flag to
verify correct behavior on topologies featuring hybrid events. It relies
on fallback logic inside perf top.
> diff --git a/tools/perf/tests/shell/top.sh b/tools/perf/tests/shell/top.sh
> index ad7fccd09025d..49820bb2d6b25 100755
> --- a/tools/perf/tests/shell/top.sh
> +++ b/tools/perf/tests/shell/top.sh
[ ... ]
> @@ -35,26 +35,27 @@ test_basic_perf_top() {
[ ... ]
> + sleep 10 | timeout 5s perf top --stdio -d 1 -e cpu-clock \
> + -p $PID > "${log_file}" 2>&1 || retval=$?
> + if [ "${retval:-0}" -ne 124 ] && [ "${retval:-0}" -ne 0 ]; then
[Severity: Medium]
Is it possible for retval to leak state between consecutive tests?
Since retval is conditionally assigned here but not initialized before use, a
successful command execution might leave retval with a value from a previous
test. If test_basic_perf_top fails and leaves a non-zero retval, a subsequent
success in test_hybrid_merge_perf_top could falsely inherit the non-zero
value and incorrectly report a failure.
[ ... ]
> kill $PID
> wait $PID 2>/dev/null || true
>
> + sync
> +
[Severity: Medium]
Does this sync command introduce an unnecessary system-wide delay?
Once the process writes to a file descriptor and exits, the data is
immediately available in the VFS page cache for grep. The sync command
commits all dirty pages system-wide to physical storage rather than just
flushing the process buffers.
[ ... ]
> +test_hybrid_merge_perf_top() {
[ ... ]
> + # Run without explicitly requesting -e cycles so heavily virtualized
> + # environments can seamlessly fall back to cpu-clock while real
> + # hybrid hardware will naturally cover the merge logic.
> + sleep 10 | timeout 5s perf top -e cpu-clock \
> + --stdio --hybrid-merge -d 1 -p $PID > "${log_file}" 2>&1 || retval=$?
[Severity: Medium]
Could the explicit -e cpu-clock argument here bypass the hybrid PMU logic
that this function is intended to test?
The inline comment suggests the test runs without requesting cycles to allow
natural hardware merge logic to trigger, yet the command hardcodes cpu-clock,
which is a software event. Software events do not utilize hardware PMUs,
meaning the --hybrid-merge code path for merging hybrid hardware events
might never be exercised.
[ ... ]
> + kill $PID
> + wait $PID 2>/dev/null || true
> +
> + # Wait a tiny bit for the file system to catch up on the logs
> + sync
> +
[Severity: Medium]
Is this sync command needed to wait for the file system to catch up?
Similar to the basic test, the data written to the log file should already
be available in the page cache once the process exits. Forcing a system-wide
sync might stall the CI environment unnecessarily.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824063744.1533837-1-irogers@google.com?part=7
^ permalink raw reply [flat|nested] 31+ messages in thread