Re: [PATCH v9 2/9] perf c2c: add function view model skeleton

[Ian Rogers]

On Mon, Aug 17, 2026 at 2:40 AM Jiebin Sun <jiebin.sun@xxxxxxxxx> wrote:
>
> Add the initial common model for the c2c function view: model state and
> small helpers shared by the hierarchy construction and formatting added
> in later patches.
>
> Build the model from util/ so it remains independent of the TUI and
> command-private symbols.
>
> Signed-off-by: Jiebin Sun <jiebin.sun@xxxxxxxxx>
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> Cc: Ian Rogers <irogers@xxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: James Clark <james.clark@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Thomas Falcon <thomas.falcon@xxxxxxxxx>
> Reviewed-by: Tianyou Li <tianyou.li@xxxxxxxxx>
> Reviewed-by: Wangyang Guo <wangyang.guo@xxxxxxxxx>
> ---
>  tools/perf/util/Build          |  1 +
>  tools/perf/util/c2c-function.c | 66 ++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
>  create mode 100644 tools/perf/util/c2c-function.c
>
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 1dfd92cbe3b7..b26a0b1ddfa3 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -12,6 +12,7 @@ perf-util-y += block-info.o
>  perf-util-y += block-range.o
>  perf-util-y += build-id.o
>  perf-util-y += c2c.o
> +perf-util-y += c2c-function.o
>  perf-util-y += cacheline.o
>  perf-util-$(CONFIG_LIBCAPSTONE) += capstone.o
>  perf-util-y += config.o
> diff --git a/tools/perf/util/c2c-function.c b/tools/perf/util/c2c-function.c
> new file mode 100644
> index 000000000000..ca82425a28dc
> --- /dev/null
> +++ b/tools/perf/util/c2c-function.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * C2C function model - function-level cacheline sharing analysis
> + *
> + * Displays a 3-level hierarchy showing which functions share cachelines:
> + *   Level 1: Read-side functions sorted by Cycles % (estimated load cycles)
> + *   Level 2: Functions sampled writing the shared lines read by level 1
> + *   Level 3: The specific cachelines where the two functions contend
> + *
> + * Builds the hierarchy from the existing cacheline histograms
> + * (c2c_hist_entry->hists), reusing the shared c2c data structures.
> + */
> +
> +#include <errno.h>
> +#include <inttypes.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <tools/libc_compat.h> /* reallocarray */
> +#include <linux/list.h>
> +#include <linux/rbtree.h>
> +#include <linux/zalloc.h>
> +
> +#include "addr_location.h"
> +#include "c2c.h"
> +#include "cacheline.h"
> +#include "hist.h"
> +#include "map.h"
> +#include "mem-events.h"
> +#include "mem-info.h"
> +#include "sort.h"
> +#include "symbol.h"
> +#include "thread.h"

nit: the number of #includes is somewhat generous here. I presume
later patches will require these includes. To avoid everything
depending on everything else it would be nice to use forward
declarations when possible. For example, if the only reason for
including the header file was to use a struct's name where it is
passed as a pointer in a function declaration, ie in header files
prefer:

  struct map;
  int foo(struct map *m);

over

  #include "map.h"
  int foo(struct map *m);

> +
> +struct c2c_function_model {
> +       struct c2c_hists        function_hists;
> +       /* Total estimated cycles across all level-1 entries. */
> +       u64                     total_cycles;
> +       /* Source cacheline histograms; not owned here. */
> +       struct c2c_hists        *cl_hists;
> +       /* --coalesce field list, used to require iaddr. */
> +       const char              *cl_sort;
> +       /* Do not cap long symbol names. */
> +       bool                     symbol_full;
> +};
> +
> +static struct c2c_function_model c2c_ext __maybe_unused;
> +
> +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)
> +{
> +       /* 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;

Sashiko rightly flagged this as not being cross-platform compatible,
but this is a pre-existing issue that looks relatively easy to clean
up but only really impacts PowerPC and so is hard for me to test. I'll
try to do it anyway.

Thanks,
Ian

> +}
> +
> +static inline __maybe_unused u64 hist_entry__iaddr(struct hist_entry *he)
> +{
> +       if (he->mem_info)
> +               return mem_info__iaddr(he->mem_info)->addr;
> +       return he->ip;
> +}
> --
> 2.52.0
>