Re: [PATCH v8 5/6] perf tools: Show memory region in perf-script subcommand
From: sashiko-bot@kernel.org
Date: 2026-09-10 19:53:56
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 [off-list ref] 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/ (local) [ ... ]
quoted hunk ↗ jump to hunk
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