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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.