Re: [PATCH v8 6/9] perf c2c: add function view hierarchy entry creation

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Aug 10, 2026 at 01:26:44PM +0800, Jiebin Sun 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. The browser can later use the same index to find
> the original cacheline entry.
> 
> These are driven by the hierarchy builder in the next patch and are
> __maybe_unused until then.
> 
> Signed-off-by: Jiebin Sun <jiebin.sun@intel.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: James Clark <james.clark@linaro.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Falcon <thomas.falcon@intel.com>
> Reviewed-by: Tianyou Li <tianyou.li@intel.com>
> Reviewed-by: Wangyang Guo <wangyang.guo@intel.com>
> ---
>  tools/perf/ui/browsers/c2c-function.c | 293 +++++++++++++++++++++++++-
>  1 file changed, 282 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> index 4bf8406fde55..4099d4c7e7b4 100644
> --- a/tools/perf/ui/browsers/c2c-function.c
> +++ b/tools/perf/ui/browsers/c2c-function.c
> @@ -28,6 +28,7 @@
>  #include "../../util/addr_location.h"
>  #include "../../util/cacheline.h"
>  #include "../../util/debug.h"
> +#include "../../util/dso.h"
>  #include "../../util/hist.h"
>  #include "../../util/map.h"
>  #include "../../util/mem-events.h"
> @@ -57,12 +58,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;

What if both DSOs are NULL?

Thanks,
Namhyung


> +	} 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);
>  }