* [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid
@ 2025-02-12 18:54 Athira Rajeev
2025-02-13 3:34 ` Namhyung Kim
2025-02-14 12:43 ` Thomas Richter
0 siblings, 2 replies; 5+ messages in thread
From: Athira Rajeev @ 2025-02-12 18:54 UTC (permalink / raw)
To: acme, jolsa, adrian.hunter, irogers, namhyung
Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel,
hbathini, Aditya.Bodkhe1
"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 <atrajeev@linux.vnet.ibm.com>
---
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));
else
diff --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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid
2025-02-12 18:54 [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid Athira Rajeev
@ 2025-02-13 3:34 ` Namhyung Kim
2025-02-17 12:17 ` Athira Rajeev
2025-02-17 12:25 ` Athira Rajeev
2025-02-14 12:43 ` Thomas Richter
1 sibling, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2025-02-13 3:34 UTC (permalink / raw)
To: Athira Rajeev
Cc: acme, jolsa, adrian.hunter, irogers, linux-perf-users,
linuxppc-dev, maddy, kjain, disgoel, hbathini, Aditya.Bodkhe1
On Thu, Feb 13, 2025 at 12:24:38AM +0530, Athira Rajeev wrote:
> "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 <atrajeev@linux.vnet.ibm.com>
Please take a look at:
https://lore.kernel.org/r/20250212163859.1489916-1-james.clark@linaro.org
Thanks,
Namhyung
> ---
> 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));
> else
> diff --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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid
2025-02-12 18:54 [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid Athira Rajeev
2025-02-13 3:34 ` Namhyung Kim
@ 2025-02-14 12:43 ` Thomas Richter
1 sibling, 0 replies; 5+ messages in thread
From: Thomas Richter @ 2025-02-14 12:43 UTC (permalink / raw)
To: Athira Rajeev, acme, jolsa, adrian.hunter, irogers, namhyung
Cc: linux-perf-users, linuxppc-dev, maddy, kjain, disgoel, hbathini,
Aditya.Bodkhe1
On 2/12/25 19:54, Athira Rajeev wrote:
> "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 <atrajeev@linux.vnet.ibm.com>
for s390:
Tested-by: Thomas Richter <tmricht@linux.ibm.com>
--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid
2025-02-13 3:34 ` Namhyung Kim
@ 2025-02-17 12:17 ` Athira Rajeev
2025-02-17 12:25 ` Athira Rajeev
1 sibling, 0 replies; 5+ messages in thread
From: Athira Rajeev @ 2025-02-17 12:17 UTC (permalink / raw)
To: Namhyung Kim, Thomas Richter, James Clark
Cc: Athira Rajeev, Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter,
Ian Rogers, linux-perf-users, linuxppc-dev, maddy, kjain, disgoel,
hbathini, Aditya.Bodkhe1
[-- Attachment #1: Type: text/plain, Size: 5870 bytes --]
> On 13 Feb 2025, at 9:04 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Feb 13, 2025 at 12:24:38AM +0530, Athira Rajeev wrote:
>> "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 <atrajeev@linux.vnet.ibm.com>
>
> Please take a look at:
> https://lore.kernel.org/r/20250212163859.1489916-1-james.clark@linaro.org
>
> 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
>
>> ---
>> 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));
>> else
>> diff --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
[-- Attachment #2: Type: text/html, Size: 14342 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid
2025-02-13 3:34 ` Namhyung Kim
2025-02-17 12:17 ` Athira Rajeev
@ 2025-02-17 12:25 ` Athira Rajeev
1 sibling, 0 replies; 5+ messages in thread
From: Athira Rajeev @ 2025-02-17 12:25 UTC (permalink / raw)
To: Namhyung Kim, Thomas Richter, James Clark
Cc: Athira Rajeev, Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter,
Ian Rogers, linux-perf-users, linuxppc-dev, maddy, kjain, disgoel,
hbathini, Aditya.Bodkhe1
> On 13 Feb 2025, at 9:04 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Feb 13, 2025 at 12:24:38AM +0530, Athira Rajeev wrote:
>> "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 <atrajeev@linux.vnet.ibm.com>
>
> Please take a look at:
> https://lore.kernel.org/r/20250212163859.1489916-1-james.clark@linaro.org
>
> 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
>
>> ---
>> 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));
>> else
>> diff --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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-17 21:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-12 18:54 [PATCH] tools/perf: Add check to tool pmu tests to ensure if the event is valid Athira Rajeev
2025-02-13 3:34 ` Namhyung Kim
2025-02-17 12:17 ` Athira Rajeev
2025-02-17 12:25 ` Athira Rajeev
2025-02-14 12:43 ` Thomas Richter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).