Re: [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid
From: Athira Rajeev <hidden>
Date: 2025-02-17 21:01:48
Also in:
linux-perf-users
On 13 Feb 2025, at 9:04 AM, Namhyung Kim [off-list ref] wrote: On Thu, Feb 13, 2025 at 12:24:38AM +0530, Athira Rajeev wrote:quoted
"Tool PMU" tests fails on powerpc as below: 12.1: Parsing without PMU name: --- start --- test child forked, pid 48492 Using CPUID 0x00800200 Attempt to add: tool/duration_time/ ..after resolving event: tool/config=0x1/ duration_time -> tool/duration_time/ Attempt to add: tool/user_time/ ..after resolving event: tool/config=0x2/ user_time -> tool/user_time/ Attempt to add: tool/system_time/ ..after resolving event: tool/config=0x3/ system_time -> tool/system_time/ Attempt to add: tool/has_pmem/ ..after resolving event: tool/config=0x4/ has_pmem -> tool/has_pmem/ Attempt to add: tool/num_cores/ ..after resolving event: tool/config=0x5/ num_cores -> tool/num_cores/ Attempt to add: tool/num_cpus/ ..after resolving event: tool/config=0x6/ num_cpus -> tool/num_cpus/ Attempt to add: tool/num_cpus_online/ ..after resolving event: tool/config=0x7/ num_cpus_online -> tool/num_cpus_online/ Attempt to add: tool/num_dies/ ..after resolving event: tool/config=0x8/ num_dies -> tool/num_dies/ Attempt to add: tool/num_packages/ ..after resolving event: tool/config=0x9/ num_packages -> tool/num_packages/ ---- unexpected signal (11) ---- 12.1: Parsing without PMU name : FAILED! Same fail is observed for "Parsing with PMU name" as well. The testcase loops through events in tool_pmu__for_each_event() and access event name using "tool_pmu__event_to_str()". Here tool_pmu__event_to_str returns null for "slots" event and "system_tsc_freq" event. These two events are only applicable for arm64 and x86 respectively. So the function tool_pmu__event_to_str() skips for unsupported events and returns null. This null value is causing testcase fail. To address this in "Tool PMU" testcase, add a helper function tool_pmu__all_event_to_str() which returns the name for all events mapping to the tool_pmu_event index including the skipped ones. So that even if its a skipped event, the helper function helps to resolve the tool_pmu_event index to its mapping event name. Update the testcase to check for null event names before proceeding the test. Signed-off-by: Athira Rajeev <redacted>Please take a look at: https://lore.kernel.org/r/20250212163859.1489916-1-james.clark@linaro.org (local) Thanks, Namhyung
Hi, Sure thanks for the fix James Thomas, Thanks for testing this patch. But James already fixed this with a different patch and it is part of perf-tools-next https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/commit/?h=perf-tools-next&id=615ec00b06f78912c370b372426190768402a5b9 Please test with latest perf-tools-next Thanks Athira
quoted
--- tools/perf/tests/tool_pmu.c | 12 ++++++++++++ tools/perf/util/tool_pmu.c | 17 +++++++++++++++++ tools/perf/util/tool_pmu.h | 1 + 3 files changed, 30 insertions(+)diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c index 187942b749b7..e468e5fb3c73 100644 --- a/tools/perf/tests/tool_pmu.c +++ b/tools/perf/tests/tool_pmu.c@@ -19,6 +19,18 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu)return TEST_FAIL; } + /* + * if tool_pmu__event_to_str returns NULL, Check if the event is + * valid for the platform. + * Example: + * slots event is only on arm64. + * system_tsc_freq event is only on x86. + */ + if (!tool_pmu__event_to_str(ev) && tool_pmu__skip_event(tool_pmu__all_event_to_str(ev))) { + ret = TEST_OK; + goto out; + } + if (with_pmu) snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev)); elsediff --git a/tools/perf/util/tool_pmu.c b/tools/perf/util/tool_pmu.c index 3a68debe7143..572422797f6e 100644 --- a/tools/perf/util/tool_pmu.c +++ b/tools/perf/util/tool_pmu.c@@ -60,6 +60,15 @@ int tool_pmu__num_skip_events(void)return num; } +/* + * tool_pmu__event_to_str returns only supported event names. + * For events which are supposed to be skipped in the platform, + * return NULL + * + * tool_pmu__all_event_to_str returns the name for all + * events mapping to the tool_pmu_event index including the + * skipped ones. + */ const char *tool_pmu__event_to_str(enum tool_pmu_event ev) { if ((ev > TOOL_PMU__EVENT_NONE && ev < TOOL_PMU__EVENT_MAX) &&@@ -69,6 +78,14 @@ const char *tool_pmu__event_to_str(enum tool_pmu_event ev)return NULL; } +const char *tool_pmu__all_event_to_str(enum tool_pmu_event ev) +{ + if (ev > TOOL_PMU__EVENT_NONE && ev < TOOL_PMU__EVENT_MAX) + return tool_pmu__event_names[ev]; + + return NULL; +} + enum tool_pmu_event tool_pmu__str_to_event(const char *str) { int i;diff --git a/tools/perf/util/tool_pmu.h b/tools/perf/util/tool_pmu.h index a60184859080..da76808ae71e 100644 --- a/tools/perf/util/tool_pmu.h +++ b/tools/perf/util/tool_pmu.h@@ -30,6 +30,7 @@ enum tool_pmu_event {for ((ev) = TOOL_PMU__EVENT_DURATION_TIME; (ev) < TOOL_PMU__EVENT_MAX; ev++) const char *tool_pmu__event_to_str(enum tool_pmu_event ev); +const char *tool_pmu__all_event_to_str(enum tool_pmu_event ev); enum tool_pmu_event tool_pmu__str_to_event(const char *str); bool tool_pmu__skip_event(const char *name); int tool_pmu__num_skip_events(void); -- 2.43.5