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

[Namhyung Kim <namhyung@kernel.org>]

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

> 
> Signed-off-by: Athira Rajeev <atrajeev@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
>