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

[Athira Rajeev]

> On 30 Oct 2024, at 5:29 AM, Namhyung Kim <namhyung at kernel.org> wrote:
> 
> Hello,
> 
> On Tue, Oct 22, 2024 at 07:31:56PM +0530, Athira Rajeev wrote:
>> 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.
> 
> 
>> 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

>> 
>> Signed-off-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
>> ---
>> 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