Re: [PATCH] perf/x86: Fix open counting event error

["Liang, Kan" <kan.liang@linux.intel.com>]

Hi Ravi,

Sorry for the late response. I was on vacation.

On 2025-04-25 6:12 a.m., Ravi Bangoria wrote:
> Hi Kan,
> 
>>>> Perf doesn't work at perf stat for hardware events:
>>>>
>>>>  $perf stat -- sleep 1
>>>>  Performance counter stats for 'sleep 1':
>>>>              16.44 msec task-clock                       #    0.016 CPUs utilized
>>>>                  2      context-switches                 #  121.691 /sec
>>>>                  0      cpu-migrations                   #    0.000 /sec
>>>>                 54      page-faults                      #    3.286 K/sec
>>>>    <not supported>	cycles
>>>>    <not supported>	instructions
>>>>    <not supported>	branches
>>>>    <not supported>	branch-misses
>>>
>>> Wondering if it is worth to add this in perf test. Something like
>>> below?
>>>
>>> --- a/tools/perf/tests/shell/stat.sh
>>> +++ b/tools/perf/tests/shell/stat.sh
>>> @@ -16,6 +16,24 @@ test_default_stat() {
>>>    echo "Basic stat command test [Success]"
>>>  }
>>>  
>>> +test_stat_count() {
>>> +  echo "stat count test"
>>> +
>>> +  if ! perf list | grep -q "cpu-cycles OR cycles"
>>> +  then
>>> +    echo "stat count test [Skipped cpu-cycles event missing]"
>>> +    return
>>> +  fi
>>> +
>>> +  if perf stat -e cycles true 2>&1 | grep -E -q "<not supported>"
>>> +  then
>>> +    echo "stat count test [Failed]"
>>> +    err=1
>>> +    return
>>> +  fi
>>> +  echo "stat count test [Success]"
>>> +}
>>> +
>>>  test_stat_record_report() {
>>>    echo "stat record and report test"
>>>    if ! perf stat record -o - true | perf stat report -i - 2>&1 | \
>>> @@ -201,6 +219,7 @@ test_hybrid() {
>>>  }
>>>  
>>>  test_default_stat
>>> +test_stat_count
>>
>> I think the perf stat default should always be supported, not just cycles.
>> Maybe we should add the check in test_default_stat?
> 
> Do you mean:
> 
>   if perf stat true 2>&1 | grep -E -q "<not supported>"
>     err=1
>

Yes, I assumed that all the events in the perf stat are always
available. But it seems the assumption is only true for bare metal.


> Isn't this ambiguous? Also, this fails on machines where HW pmu
> is not supported. For ex, on my qemu guest with `-cpu pmu=off`:
> 
>   $ ./perf list | grep "cpu-cycles OR cycles"
>   <empty output>
> 
>   $ ./perf stat true
>    Performance counter stats for 'true':
>                 0.42 msec task-clock:u                     #    0.470 CPUs utilized
>                    0      context-switches:u               #    0.000 /sec
>                    0      cpu-migrations:u                 #    0.000 /sec
>                   48      page-faults:u                    #  113.874 K/sec
>      <not supported>      cycles:u
> 
If taking the virtualization into account, the test_stat_count looks
good to me.

Thanks,
Kan