Re: [PATCH] perf test: Parse perf stat Topdown events for aarch64

[Veronika Molnarova <vmolnaro@redhat.com>]

On 11/18/24 22:31, Ian Rogers wrote:
> On Mon, Nov 18, 2024 at 2:04 AM Veronika Molnarova <vmolnaro@redhat.com> wrote:
>>
>>
>>
>> On 10/29/24 16:25, Ian Rogers wrote:
>>> On Tue, Oct 29, 2024 at 7:44 AM <vmolnaro@redhat.com> wrote:
>>>>
>>>> From: Veronika Molnarova <vmolnaro@redhat.com>
>>>>
>>>> The 'perf stat' output on aarch64 machines with topdown events wasn't
>>>> counted for in the 'perf stat STD output linter' test case. Add the
>>>> topdown metric to the skip_metric list as it is done for topdown events
>>>> on other systems.
>>>>
>>>> The Topdown events are also disabled on aarch64 KVM guests because the
>>>> value of caps/slots is set to 0 due to the part of the system register
>>>> being a stub. This prevents the metric for the topdown events from being
>>>> computed, leaving the 'perf stat' topdown metric without any value at
>>>> all. Add the "TopdownL1" to the skip_metric list as well to handle this
>>>> possibility.
>>>>
>>>> Before aarch64:
>>>>
>>>>     100: perf stat STD output linter:
>>>>     --- start ---
>>>>     test child forked, pid 403305
>>>>     Checking STD output: no args Unknown event name in TopdownL1                 #      4.3 percent of slots  slots_lost_misspeculation_fraction
>>>>     ---- end(-1) ----
>>>>     100: perf stat STD output linter                                     : FAILED!
>>>>
>>>> Before aarch64 KVM:
>>>>
>>>>     100: perf stat STD output linter:
>>>>     --- start ---
>>>>     test child forked, pid 404671
>>>>     Checking STD output: no args Unknown event name in TopdownL1
>>>>     ---- end(-1) ----
>>>>     100: perf stat STD output linter                                     : FAILED!
>>>>
>>>> After:
>>>>
>>>>     100: perf stat STD output linter:
>>>>     --- start ---
>>>>     test child forked, pid 404777
>>>>     Checking STD output: no args [Success]
>>>>     Checking STD output: system wide [Success]
>>>>     Checking STD output: interval [Success]
>>>>     Checking STD output: per thread [Success]
>>>>     Checking STD output: per node [Success]
>>>>     Checking STD output: system wide no aggregation [Success]
>>>>     Checking STD output: per core [Success]
>>>>     Checking STD output: per cache instance [Success]
>>>>     Checking STD output: per cluster [Success]
>>>>     Checking STD output: per die [Success]
>>>>     Checking STD output: per socket [Success]
>>>>     ---- end(0) ----
>>>>     100: perf stat STD output linter                                     : Ok
>>>>
>>>> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
>>>>
>>>> ---
>>>>  tools/perf/tests/shell/stat+std_output.sh | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/perf/tests/shell/stat+std_output.sh b/tools/perf/tests/shell/stat+std_output.sh
>>>> index cbf2894b2c846a04..0f7967be60afdc7d 100755
>>>> --- a/tools/perf/tests/shell/stat+std_output.sh
>>>> +++ b/tools/perf/tests/shell/stat+std_output.sh
>>>> @@ -13,7 +13,7 @@ stat_output=$(mktemp /tmp/__perf_test.stat_output.std.XXXXX)
>>>>
>>>>  event_name=(cpu-clock task-clock context-switches cpu-migrations page-faults stalled-cycles-frontend stalled-cycles-backend cycles instructions branches branch-misses)
>>>>  event_metric=("CPUs utilized" "CPUs utilized" "/sec" "/sec" "/sec" "frontend cycles idle" "backend cycles idle" "GHz" "insn per cycle" "/sec" "of all branches")
>>>> -skip_metric=("stalled cycles per insn" "tma_" "retiring" "frontend_bound" "bad_speculation" "backend_bound")
>>>> +skip_metric=("stalled cycles per insn" "tma_" "retiring" "frontend_bound" "bad_speculation" "backend_bound" "TopdownL1" "percent of slots")
>>>
>>> I saw similar issues and sent:
>>> https://lore.kernel.org/lkml/20240417183219.1208493-1-irogers@google.com/
>>> but got distracted. A difference there is automating the generation of
>>> the skip_metric list rather than hard coding it. Hard coding may be
>>> the most pragmatic thing to do in the near term.
>>>
>>> Thanks,
>>> Ian
>>>
>> Automating the skip_metric list generation would be great, but for
>> the time being, we can stick with hard coding until there is such
>> possibility.
> 
> Acked-by: Ian Rogers <irogers@google.com>
> 
> Thanks,
> Ian
> 
Ping on this patch. Please let me know if it's missing anything.

Thanks,
Veronika