Re: [PATCH v8 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] Unnecessary full header inclusion of `session.h` in `mem-events.h` instead of a forward declaration.
--

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

perf tools: Show memory region in perf-script subcommand

This commit adds a new memory region field to the perf-script output, sourced
from the mem_region field of the memory information data source. It also
incorporates fallback logic for pipe mode scenarios where feature bits might
not be available.

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]
Does this unnecessarily pull in the full session.h header when a forward
declaration would suffice?

According to the perf tools subsystem guidelines, it is preferred to use
forward declarations (e.g., "struct perf_session;") in header files instead
of full header inclusions when only structure pointer handles are referenced. 
Since the new perf_script__meminfo_scnprintf() function prototype only needs a
pointer to struct perf_session, could this include be replaced with a forward
declaration to reduce header dependencies and namespace pollution? The full
include can then be placed directly in the .c files that dereference it.

>  
>  struct perf_mem_event {
>  	bool		supported;
[ ... ]

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