Re: [PATCH v9 5/6] perf tools: Show memory region in perf-script subcommand

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf: unnecessary session.h inclusion in mem-events.h
--

commit f3a6814df7d1c56b77a8b31b8b171d285c37db7e
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf tools: Show memory region in perf-script subcommand

Show the memory region in perf-script subcommand. Memory region is found
in the mem_region field of the memory information data source.

Link: https://lore.kernel.org/all/20260114011750.350569-1-dapeng1.mi@linux.intel.com/

> diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
> index daa22748f9fea..4ebb8109fc3c4 100644
> --- a/tools/perf/util/mem-events.h
> +++ b/tools/perf/util/mem-events.h
> @@ -4,6 +4,7 @@
>  
>  #include <stdbool.h>
>  #include <linux/types.h>
> +#include "session.h"

[Severity: Low]
Is this full header inclusion necessary here?

Since the only usage of session.h in mem-events.h is the struct perf_session
pointer in the perf_script__meminfo_scnprintf function prototype below,
could this be replaced with a forward declaration (struct perf_session;)
instead? This would reduce namespace pollution and avoid unnecessary build
dependencies.

>  
>  struct perf_mem_event {
>  	bool		supported;

[ ... ]

> @@ -47,7 +48,8 @@ int perf_mem__snp_scnprintf(char *out, size_t sz, const struct mem_info *mem_inf
>  int perf_mem__lck_scnprintf(char *out, size_t sz, const struct mem_info *mem_info);
>  int perf_mem__blk_scnprintf(char *out, size_t sz, const struct mem_info *mem_info);
>  
> -int perf_script__meminfo_scnprintf(char *bf, size_t size, const struct mem_info *mem_info);
> +int perf_script__meminfo_scnprintf(char *bf, size_t size, const struct mem_info *mem_info,
> +				   struct perf_session *session);
>  
>  struct c2c_stats {
>  	u32	nr_entries;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914224650.52982-1-thomas.falcon@intel.com?part=5