Re: [PATCH] tools/perf/tests: Add system wide check for perf bench workload in all metric test

[kajoljain <kjain@linux.ibm.com>]

On 2/15/23 05:36, Ian Rogers wrote:
> On Tue, Feb 7, 2023 at 7:45 PM kajoljain <kjain@linux.ibm.com> wrote:
>>
>>
>>
>> On 2/6/23 10:10, Athira Rajeev wrote:
>>>
>>>
>>>> On 02-Feb-2023, at 10:14 PM, Kajol Jain <kjain@linux.ibm.com> wrote:
>>>>
>>>> Testcase stat_all_metrics.sh fails in powerpc:
>>>>
>>>> 92: perf all metrics test : FAILED!
>>>>
>>>> Logs with verbose:
>>>>
>>>> [command]# ./perf test 92 -vv
>>>> 92: perf all metrics test                                           :
>>>> --- start ---
>>>> test child forked, pid 13262
>>>> Testing BRU_STALL_CPI
>>>> Testing COMPLETION_STALL_CPI
>>>> ----
>>>> Testing TOTAL_LOCAL_NODE_PUMPS_P23
>>>> Metric 'TOTAL_LOCAL_NODE_PUMPS_P23' not printed in:
>>>> Error:
>>>> Invalid event (hv_24x7/PM_PB_LNS_PUMP23,chip=3/) in per-thread mode, enable system wide with '-a'.
>>>> Testing TOTAL_LOCAL_NODE_PUMPS_RETRIES_P01
>>>> Metric 'TOTAL_LOCAL_NODE_PUMPS_RETRIES_P01' not printed in:
>>>> Error:
>>>> Invalid event (hv_24x7/PM_PB_RTY_LNS_PUMP01,chip=3/) in per-thread mode, enable system wide with '-a'.
>>>> ----
>>>>
>>>> Based on above logs, we could see some of the hv-24x7 metric events fails,
>>>> and logs suggest to run the metric event with -a option.
>>>> This change happened after the commit a4b8cfcabb1d ("perf stat: Delay metric
>>>> parsing"), which delayed the metric parsing phase and now before metric parsing
>>>> phase perf tool identifies, whether target is system-wide or not. With this
>>>> change, perf_event_open will fails with workload monitoring for uncore events
>>>> as expected.
>>>>
>>>> The perf all metric test case fails as some of the hv-24x7 metric events
>>>> may need bigger workload to get the data. And the added perf bench
>>>> workload in 'perf all metric test case' will not run for hv-24x7 without
>>>> -a option.
>>>>
>>>> Fix this issue by adding system wide check for perf bench workload.
>>>>
>>>> Result with the patch changes in powerpc:
>>>>
>>>> 92: perf all metrics test : Ok
>>>>
>>>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>>>
>>> Looks good to me
>>>
>>> Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>
>> Hi Arnaldo,
>>    Let me know if patch looks fine to you.
>>
>> Thanks,
>> Kajol Jain
> 
> I ran into a similar issue but worked around it with:
> 
> ```
> --- a/tools/perf/tests/shell/stat_all_metrics.sh
> +++ b/tools/perf/tests/shell/stat_all_metrics.sh
> @@ -11,7 +11,7 @@ for m in $(perf list --raw-dump metrics); do
>     continue
>   fi
>   # Failed so try system wide.
> -  result=$(perf stat -M "$m" -a true 2>&1)
> +  result=$(perf stat -M "$m" -a sleep 0.01 2>&1)
>   if [[ "$result" =~ "${m:0:50}" ]]
>   then
>     continue
> ```
> 
> Running the synthesize benchmark is potentially slow, wdyt of the change above?

Hi Ian,
  Thanks for reviewing the patch. Yes we can change workload true to
sleep 0.01 as we need bigger workload for 24x7 and sleep 0.01 will also
work for 24x7 metric events.

I will send v2 patch with this change.

Thanks,
Kajol Jain

> 
> Thanks,
> Ian
> 
> 
>>>
>>>> ---
>>>> tools/perf/tests/shell/stat_all_metrics.sh | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
>>>> index 6e79349e42be..d49832a316d9 100755
>>>> --- a/tools/perf/tests/shell/stat_all_metrics.sh
>>>> +++ b/tools/perf/tests/shell/stat_all_metrics.sh
>>>> @@ -23,6 +23,13 @@ for m in $(perf list --raw-dump metrics); do
>>>>   then
>>>>     continue
>>>>   fi
>>>> +  # Failed again, possibly the event is uncore pmu event which will need
>>>> +  # system wide monitoring with workload, so retry with -a option
>>>> +  result=$(perf stat -M "$m" -a perf bench internals synthesize 2>&1)
>>>> +  if [[ "$result" =~ "${m:0:50}" ]]
>>>> +  then
>>>> +    continue
>>>> +  fi
>>>>   echo "Metric '$m' not printed in:"
>>>>   echo "$result"
>>>>   if [[ "$err" != "1" ]]
>>>> --
>>>> 2.39.0
>>>>
>>>