Re: [PATCH v9 6/9] perf c2c: add function view hierarchy entry creation
Ian Rogers <[email protected]>
| Newsgroups | org.kernel.vger.linux-perf-users,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAP-5=fU5tM4vmu0PFJOOOb-caS6bs8=aRyfP2rr_L56TOLksgw@mail.gmail.com> |
On Mon, Aug 17, 2026 at 2:40 AM Jiebin Sun <[email protected]> wrote: > > Add the entry-creation layer: owned-reference child allocation and > insertion, and the level-1/2/3 lookup-or-create functions keyed by > function symbol (level 1 read-side, level 2 writer) and by the source > cacheline's existing index (level 3). > > Give synthetic children normal entry operations and acquire their thread > and map-symbol references. This lets the hierarchy teardown use > hist_entry__delete() for the common fields while the function-view free > callback handles the private child tree and containing allocation. > > Reuse cacheline_idx to preserve the source entry identity without adding > function-view-only state. Add c2c_function__find_cacheline() to locate the > original cacheline entry by the same index. > > These are driven by the hierarchy builder in the next patch and are > __maybe_unused until then. > > Signed-off-by: Jiebin Sun <[email protected]> > Cc: Adrian Hunter <[email protected]> > Cc: Alexander Shishkin <[email protected]> > Cc: Arnaldo Carvalho de Melo <[email protected]> > Cc: Dapeng Mi <[email protected]> > Cc: Ian Rogers <[email protected]> > Cc: Ingo Molnar <[email protected]> > Cc: James Clark <[email protected]> > Cc: Jiri Olsa <[email protected]> > Cc: Mark Rutland <[email protected]> > Cc: Namhyung Kim <[email protected]> > Cc: Peter Zijlstra <[email protected]> > Cc: Thomas Falcon <[email protected]> > Reviewed-by: Tianyou Li <[email protected]> > Reviewed-by: Wangyang Guo <[email protected]> Reviewed-by: Ian Rogers <[email protected]> Thanks, Ian > --- > tools/perf/util/c2c-function.c | 319 +++++++++++++++++++++++++++++++-- > tools/perf/util/c2c.h | 2 + > 2 files changed, 310 insertions(+), 11 deletions(-) > > diff --git a/tools/perf/util/c2c-function.c b/tools/perf/util/c2c-function.c > index aa6be7c72977..c005fb5454a7 100644 > --- a/tools/perf/util/c2c-function.c > +++ b/tools/perf/util/c2c-function.c > @@ -24,6 +24,7 @@ > #include "c2c.h" > #include "cacheline.h" > #include "debug.h" > +#include "dso.h" > #include "hist.h" > #include "map.h" > #include "mem-events.h" > @@ -51,12 +52,34 @@ static inline __maybe_unused u64 c2c_hitm_count(const struct c2c_stats *stats) > return stats->tot_hitm; > } > > -static inline __maybe_unused bool symbol_name_equal(struct symbol *a, struct symbol *b) > +static int64_t c2c_function_cmp(const struct map_symbol *left, > + const struct map_symbol *right) > { > - /* Two unknown symbols compare equal, matching cmp_null() in util/sort.c. */ > - if (!a || !b) > - return a == b; > - return arch__compare_symbol_names(a->name, b->name) == 0; > + const struct dso *left_dso = left->map ? map__dso(left->map) : NULL; > + const struct dso *right_dso = right->map ? map__dso(right->map) : NULL; > + int ret; > + > + if (!left_dso || !right_dso) { > + if (left_dso != right_dso) > + return left_dso ? 1 : -1; > + } else { > + /* > + * Use the same DSO name as _sort__dso_cmp() (short name unless > + * verbose), so this matches the DSO comparison the level-1 > + * entries are deduplicated by; otherwise same-basename DSOs > + * could be split or merged inconsistently across levels. > + */ > + const char *left_name = verbose > 0 ? > + dso__long_name(left_dso) : dso__short_name(left_dso); > + const char *right_name = verbose > 0 ? > + dso__long_name(right_dso) : dso__short_name(right_dso); > + > + ret = strcmp(left_name, right_name); > + if (ret) > + return ret; > + } > + > + return _sort__sym_cmp(left->sym, right->sym); > } > > static inline __maybe_unused u64 hist_entry__iaddr(struct hist_entry *he) > @@ -753,7 +776,7 @@ static void c2c_he__free_hierarchy(struct hist_entry *he); > /* > * Free a function-view histogram entry (hist_entry_ops::free). > */ > -static void __maybe_unused c2c_function_he_free(void *ptr) > +static void c2c_function_he_free(void *ptr) > { > struct hist_entry *he = ptr; > struct c2c_hist_entry *c2c_he; > @@ -768,11 +791,6 @@ static void __maybe_unused c2c_function_he_free(void *ptr) > > c2c_he__free_hierarchy(he); > > - zfree(&c2c_he->nodeset); > - zfree(&c2c_he->cpuset); > - zfree(&c2c_he->nodestr); > - zfree(&c2c_he->node_stats); > - > free(c2c_he); > } > > @@ -843,3 +861,282 @@ static int __maybe_unused c2c_he__prune_empty_writers(struct hist_entry *l1_he) > } > return surviving; > } > + > +static void *c2c_function_he_zalloc(size_t size) > +{ > + struct c2c_hist_entry *c2c_he = zalloc(sizeof(*c2c_he) + size); > + > + if (!c2c_he) > + return NULL; > + > + init_stats(&c2c_he->cstats.lcl_hitm); > + init_stats(&c2c_he->cstats.rmt_hitm); > + init_stats(&c2c_he->cstats.lcl_peer); > + init_stats(&c2c_he->cstats.rmt_peer); > + init_stats(&c2c_he->cstats.load); > + > + return &c2c_he->he; > +} > + > +/* Entry operations for function view */ > +static struct hist_entry_ops c2c_function_entry_ops = { > + .new = c2c_function_he_zalloc, > + .free = c2c_function_he_free, > +}; > + > +static struct c2c_hist_entry * > +c2c_child_entry__alloc(struct hist_entry *parent_he, struct hist_entry *src_he, > + int depth, u64 ip) > +{ > + struct c2c_hist_entry *child_c2c; > + struct hist_entry *child_he; > + > + /* Function-view children never own or display callchains. */ > + child_he = c2c_function_he_zalloc(0); > + if (!child_he) > + return NULL; > + > + child_c2c = container_of(child_he, struct c2c_hist_entry, he); > + child_he->ops = &c2c_function_entry_ops; > + map_symbol__copy(&child_he->ms, &src_he->ms); > + > + if (src_he->mem_info) { > + child_he->mem_info = mem_info__clone(src_he->mem_info); > + if (!child_he->mem_info) > + goto out_free; > + } > + > + child_he->thread = thread__get(src_he->thread); > + child_he->cpumode = src_he->cpumode; > + child_he->cpu = src_he->cpu; > + child_he->socket = src_he->socket; > + child_he->level = src_he->level; > + child_he->ip = ip; > + > + child_he->parent_he = parent_he; > + child_he->depth = depth; > + child_he->leaf = (depth >= 2); > + child_he->hists = &c2c_ext.function_hists.hists; > + child_he->filtered = false; > + child_he->unfolded = false; > + child_he->has_children = false; > + child_he->has_no_entry = false; > + child_he->nr_rows = 0; > + child_he->row_offset = 0; > + > + memset(&child_he->stat, 0, sizeof(child_he->stat)); > + child_he->hroot_in = RB_ROOT_CACHED; > + child_he->hroot_out = RB_ROOT_CACHED; > + INIT_LIST_HEAD(&child_he->pairs.node); > + child_he->hpp_list = &c2c_ext.function_hists.list; > + if (symbol_conf.cumulate_callchain) { > + child_he->stat_acc = calloc(1, sizeof(struct he_stat)); > + if (!child_he->stat_acc) > + goto out_free; > + } > + > + return child_c2c; > + > +out_free: > + hist_entry__delete(child_he); > + return NULL; > +} > + > +static void > +c2c_child_entry__insert(struct hist_entry *parent_he, struct hist_entry *child_he, > + struct rb_node **p, struct rb_node *rb_parent, bool leftmost) > +{ > + rb_link_node(&child_he->rb_node, rb_parent, p); > + rb_insert_color_cached(&child_he->rb_node, &parent_he->hroot_out, leftmost); > + > + parent_he->has_children = true; > + parent_he->leaf = false; > +} > + > +static __maybe_unused struct hist_entry * > +c2c_function_hists__level1_entry(struct symbol *sym, > + struct hist_entry *detail_he, > + struct thread *synthetic_thread) > +{ > + struct addr_location al; > + struct perf_sample sample = {}; > + struct mem_info *mi; > + struct hist_entry *he; > + /* > + * Key the level-1 entry by the function, not by a specific code > + * address: use the symbol start so every instruction address inside > + * the same function collapses into one entry. This makes level 1 a > + * true "function view" rather than a per-code-address view. > + */ > + u64 sym_start = (sym && detail_he->ms.map) ? > + map__unmap_ip(detail_he->ms.map, sym->start) : detail_he->ip; > + > + mi = mem_info__new(); > + if (!mi) > + return NULL; > + > + mem_info__iaddr(mi)->addr = sym_start; > + /* mem_info__put() will map_symbol__exit() these, so take refs. */ > + mem_info__iaddr(mi)->ms.thread = thread__get(detail_he->ms.thread); > + mem_info__iaddr(mi)->ms.map = map__get(detail_he->ms.map); > + mem_info__iaddr(mi)->ms.sym = sym; > + mem_info__daddr(mi)->addr = 0; > + > + addr_location__init(&al); > + al.thread = thread__get(synthetic_thread); > + al.map = map__get(detail_he->ms.map); > + al.sym = sym; > + al.addr = sym_start; > + al.level = detail_he->level; > + al.cpumode = detail_he->cpumode; > + al.cpu = 0; > + al.socket = 0; > + al.filtered = 0; > + al.latency = 0; > + > + /* > + * Synthetic sample: period/weight are placeholders only. The real > + * c2c counters live in c2c_hist_entry::stats and are added via > + * hist_entry__add_c2c_stats(); no function-view column or sort key > + * reads he->stat.period/nr_events, so the +1 that __hists__add_entry() > + * accrues on each dedup hit has no effect on what is displayed. > + */ > + sample.period = 1; > + sample.weight = 1; > + sample.ip = sym_start; > + sample.pid = thread__pid(synthetic_thread); > + sample.tid = thread__tid(synthetic_thread); > + sample.cpu = 0; > + > + /* Add entry - histogram handles dedup */ > + he = hists__add_entry_ops(&c2c_ext.function_hists.hists, > + &c2c_function_entry_ops, > + &al, NULL, NULL, mi, > + NULL, &sample, true); > + > + addr_location__exit(&al); > + mem_info__put(mi); > + > + if (he) > + he->hpp_list = &c2c_ext.function_hists.list; > + > + return he; > +} > + > +/* > + * Level 2: a function that writes a cacheline the level-1 function reads, > + * keyed by the DSO display name and symbol, consistently with perf's symbol > + * sort semantics. All code addresses and cachelines for the same writer > + * function aggregate into one row. > + */ > +static __maybe_unused struct c2c_hist_entry * > +c2c_function_hists__level2_entry(struct c2c_hist_entry *level1_c2c, > + struct symbol *sym, struct hist_entry *detail_he) > +{ > + struct hist_entry *level1_he = &level1_c2c->he; > + struct rb_node **p = &level1_he->hroot_out.rb_root.rb_node; > + struct rb_node *parent = NULL; > + struct c2c_hist_entry *level2_c2c; > + bool leftmost = true; > + > + while (*p) { > + struct hist_entry *iter = rb_entry(*p, struct hist_entry, rb_node); > + struct map_symbol key = detail_he->ms; > + int64_t cmp; > + > + key.sym = sym; > + parent = *p; > + cmp = c2c_function_cmp(&key, &iter->ms); > + > + if (cmp < 0) { > + p = &parent->rb_left; > + } else if (cmp > 0) { > + p = &parent->rb_right; > + leftmost = false; > + } else { > + return container_of(iter, struct c2c_hist_entry, he); > + } > + } > + > + /* Key by the function symbol start so all code addresses collapse. */ > + level2_c2c = c2c_child_entry__alloc(level1_he, detail_he, 1, > + (sym && detail_he->ms.map) ? > + map__unmap_ip(detail_he->ms.map, sym->start) : > + hist_entry__iaddr(detail_he)); > + if (!level2_c2c) > + return NULL; > + > + /* Key this level by the looked-up symbol, not detail_he's. */ > + level2_c2c->he.ms.sym = sym; > + if (level2_c2c->he.mem_info) > + mem_info__iaddr(level2_c2c->he.mem_info)->ms.sym = sym; > + > + c2c_child_entry__insert(level1_he, &level2_c2c->he, p, parent, leftmost); > + > + return level2_c2c; > +} > + > +/* Level 3: one source cacheline where the L1/L2 functions contend. */ > +static __maybe_unused struct c2c_hist_entry * > +c2c_function_hists__level3_entry(struct c2c_hist_entry *level2_c2c, > + struct c2c_hist_entry *cacheline_src_he) > +{ > + struct hist_entry *level2_he = &level2_c2c->he; > + struct rb_node **p = &level2_he->hroot_out.rb_root.rb_node; > + struct rb_node *parent = NULL; > + struct c2c_hist_entry *level3_c2c; > + bool leftmost = true; > + > + while (*p) { > + struct c2c_hist_entry *iter_c2c = > + rb_entry(*p, struct c2c_hist_entry, he.rb_node); > + > + parent = *p; > + if (cacheline_src_he->cacheline_idx < iter_c2c->cacheline_idx) { > + p = &parent->rb_left; > + } else if (cacheline_src_he->cacheline_idx > iter_c2c->cacheline_idx) { > + p = &parent->rb_right; > + leftmost = false; > + } else { > + return iter_c2c; > + } > + } > + > + level3_c2c = c2c_child_entry__alloc(level2_he, &cacheline_src_he->he, 2, > + hist_entry__iaddr(&cacheline_src_he->he)); > + if (!level3_c2c) > + return NULL; > + level3_c2c->cacheline_idx = cacheline_src_he->cacheline_idx; > + > + c2c_child_entry__insert(level2_he, &level3_c2c->he, p, parent, leftmost); > + > + return level3_c2c; > +} > + > +struct hist_entry *c2c_function__find_cacheline(struct hist_entry *he_selection) > +{ > + struct c2c_hist_entry *c2c_he; > + struct rb_node *nd; > + > + if (!c2c_ext.cl_hists || !he_selection || !he_selection->parent_he || > + !he_selection->parent_he->parent_he) > + return NULL; > + > + c2c_he = container_of(he_selection, struct c2c_hist_entry, he); > + > + for (nd = rb_first_cached(&c2c_ext.cl_hists->hists.entries); nd; > + nd = rb_next(nd)) { > + struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node); > + struct c2c_hist_entry *cacheline_he; > + > + if (he->filtered) > + continue; > + > + cacheline_he = container_of(he, struct c2c_hist_entry, he); > + if (cacheline_he->hists && > + cacheline_he->cacheline_idx == c2c_he->cacheline_idx) > + return he; > + } > + > + return NULL; > +} > diff --git a/tools/perf/util/c2c.h b/tools/perf/util/c2c.h > index bd0c9d1c9a1a..f95e636edc0f 100644 > --- a/tools/perf/util/c2c.h > +++ b/tools/perf/util/c2c.h > @@ -98,4 +98,6 @@ struct c2c_fmt { > void c2c_fmt_free(struct perf_hpp_fmt *fmt); > bool c2c_fmt_equal(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b); > > +struct hist_entry *c2c_function__find_cacheline(struct hist_entry *he); > + > #endif /* __PERF_UTIL_C2C_H */ > -- > 2.52.0 >