Thread (11 messages) 11 messages, 4 authors, 2024-12-05

Re: [PATCH] tools/perf/tests/expr: Make the system_tsc_freq test only for intel

From: Athira Rajeev <hidden>
Date: 2024-11-04 04:17:57
Also in: linux-perf-users

On 30 Oct 2024, at 5:29 AM, Namhyung Kim [off-list ref] wrote:

Hello,

On Tue, Oct 22, 2024 at 07:31:56PM +0530, Athira Rajeev wrote:
quoted
The "Simple expression parser" test fails on powerpc
as below:

parsing metric: #system_tsc_freq
Unrecognized literal '#system_tsc_freq'literal: #system_tsc_freq = nan
syntax error
FAILED tests/expr.c:247 #system_tsc_freq
---- end(-1) ----
7: Simple expression parser  : FAILED!

In the test, system_tsc_freq is checked as below:

if (is_intel)
   TEST_ASSERT_VAL("#system_tsc_freq > 0", val > 0);
else

But commit 609aa2667f67 ("perf tool_pmu: Switch to standard
pmu functions and json descriptions")' changed condition in
Probably need to put it as Fixes: tag.

quoted
tool_pmu__skip_event so that system_tsc_freq event should
only appear on x86

+#if !defined(__i386__) && !defined(__x86_64__)
+       /* The system_tsc_freq event should only appear on x86. */
+       if (strcasecmp(name, "system_tsc_freq") == 0)
+               return true;
+#endif

After this commit, the testcase breaks for expr__parse of
system_tsc_freq in powerpc case. Fix the testcase to have
complete system_tsc_freq test within "is_intel" check.
Ian, are you ok with this?

Thanks,
Namhyung
Hi Ian

If the change looks good to you, I will send a V2 with Fixes tag added. Please share your review comments

Hi James, Thomas

Looking for help to test since in non-intel platform, this test will fail without the patch

Thanks
Athira
quoted
Signed-off-by: Athira Rajeev <redacted>
---
tools/perf/tests/expr.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index e3aa9d4fcf3a..eb3bd68fc4ce 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -244,11 +244,10 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
if (num_dies) // Some platforms do not have CPU die support, for example s390
TEST_ASSERT_VAL("#num_dies >= #num_packages", num_dies >= num_packages);

- TEST_ASSERT_VAL("#system_tsc_freq", expr__parse(&val, ctx, "#system_tsc_freq") == 0);
- if (is_intel)
+ if (is_intel) {
+ TEST_ASSERT_VAL("#system_tsc_freq", expr__parse(&val, ctx, "#system_tsc_freq") == 0);
TEST_ASSERT_VAL("#system_tsc_freq > 0", val > 0);
- else
- TEST_ASSERT_VAL("#system_tsc_freq == 0", fpclassify(val) == FP_ZERO);
+ }

/*
 * Source count returns the number of events aggregating in a leader
-- 
2.43.5

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help