Re: [PATCH v3 02/14] perf c2c: add function view browser skeleton

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing header fallback stubs for optional UI features in `tools/perf/c2c.h`
- [Low] Comment in `c2c-function.c` directly contradicts the `c2c_hist_entry` struct definition regarding per-entry state.
--

--- Patch [2]: [PATCH v3 02/14] perf c2c: add function view browser skeleton ---
commit 828f082bf7b49b53ad3bfe52de9accfa632ad4f5
Author: Jiebin Sun <jiebin.sun@intel.com>

perf c2c: add function view browser skeleton

This patch adds a skeleton for the c2c function view browser. It introduces
c2c-function.c with a stub perf_c2c__browse_function_view() entry point and
registers it in the build system.

> diff --git a/tools/perf/c2c.h b/tools/perf/c2c.h
> index 79dbd35184eda..6fc3fda40fb6c 100644
> --- a/tools/perf/c2c.h
> +++ b/tools/perf/c2c.h
> @@ -142,6 +142,7 @@ bool c2c_fmt_equal(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b);
>  
>  #ifdef HAVE_SLANG_SUPPORT
>  int perf_c2c__browse_cacheline(struct hist_entry *he);
> +int perf_c2c__browse_function_view(struct hists *hists);
>  #endif

[Severity: Low]
Should this feature guard include an #else block with dummy inline stubs?
The perf tools subsystem guidelines mention that header files should provide
fallback stubs (e.g., returning -ENOTSUPP) when a feature define is absent
to prevent build failures if callers miss the #ifdef guards.

> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> new file mode 100644
> index 0000000000000..c50c8ca825c10
> --- /dev/null
> +++ b/tools/perf/ui/browsers/c2c-function.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * C2C Function Browser - function-level cacheline sharing analysis
> + *
> + * Planned UI: 3-level hierarchy showing which functions share cachelines (not implemented yet):
> + *   Level 1: Primary functions sorted by Cycles % (estimated load cycles)
> + *   Level 2: Other functions sharing cachelines with the level-1 function
> + *   Level 3: Specific shared cachelines between each pair of functions
> + *
> + * Uses c2c_hist_entry->hists to build the hierarchy without adding any
> + * per-entry state to the existing c2c data structures.
> + */

[Severity: Low]
This isn't a bug, but is this comment accurate regarding per-entry state? 
It mentions avoiding adding any per-entry state to existing data structures,
but it looks like child_stores was added to struct c2c_hist_entry
specifically for this function view.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717020530.1645123-1-jiebin.sun@intel.com?part=2