Perf test 7 fails on s390 (perf expr: Add metric literals for topology)

Perf test 7 fails on s390 (perf expr: Add metric literals for topology)

[Thomas Richter]

Commit fdf1e29b6118 ("perf expr: Add metric literals for topology.")
fails on s390:
 
 # ./perf test -Fv 7
   ...
 # FAILED tests/expr.c:173 #num_dies >= #num_packages
   ---- end ----
   Simple expression parser: FAILED!
 #                    
Investigating this issue leads to function:
   static bool has_die_topology(void)
   {                                
        char filename[MAXPATHLEN];
        struct utsname uts;
                                                
        if (uname(&uts) < 0)    
                return false;

        if (strncmp(uts.machine, "x86_64", 6)) 
                return false;  
                                                             
        ....
   }
which always return false on s390.
When I get this right, there is no build up of CPU topology with
dies on s390. The the #num_dies value in above test is always zero
and lower than the #num_packages field.

I am also suprised by the architecture specific code in a common
code function. Wouldn't it be better to define a weak function with
archticture specfic overrides?

Is this intended?
What is the recommended approach for other architectures?
Should I skip this test on s390?
Should s390 invent die for CPUs mapping?

Thanks a lot for your help and advice.

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: Perf test 7 fails on s390 (perf expr: Add metric literals for topology)

[Ian Rogers]

On Mon, Nov 22, 2021 at 8:04 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>
> Commit fdf1e29b6118 ("perf expr: Add metric literals for topology.")
> fails on s390:
>
>  # ./perf test -Fv 7
>    ...
>  # FAILED tests/expr.c:173 #num_dies >= #num_packages
>    ---- end ----
>    Simple expression parser: FAILED!
>  #
> Investigating this issue leads to function:
>    static bool has_die_topology(void)
>    {
>         char filename[MAXPATHLEN];
>         struct utsname uts;
>
>         if (uname(&uts) < 0)
>                 return false;
>
>         if (strncmp(uts.machine, "x86_64", 6))
>                 return false;
>
>         ....
>    }
> which always return false on s390.
> When I get this right, there is no build up of CPU topology with
> dies on s390. The the #num_dies value in above test is always zero
> and lower than the #num_packages field.
>
> I am also suprised by the architecture specific code in a common
> code function. Wouldn't it be better to define a weak function with
> archticture specfic overrides?
>
> Is this intended?
> What is the recommended approach for other architectures?
> Should I skip this test on s390?
> Should s390 invent die for CPUs mapping?
>
> Thanks a lot for your help and advice.

Hi Thomas,

sorry to hear of this breakage and it was in no way intentional. The
'dies' ABI is supposedly stable:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/stable/sysfs-devices-system-cpu
hence having a test. It looks like something to dig into for s390. It
seems preferable to fix s390 than to add weak symbol workarounds.

Thanks,
Ian

> --
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
> --
> Vorsitzender des Aufsichtsrats: Gregor Pillen
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294