[PATCH v2] tools/perf/tests: Change true workload to sleep workload in all metric test for system wide check

[PATCH v2] tools/perf/tests: Change true workload to sleep workload in all metric test for system wide check

[Kajol Jain]

Testcase stat_all_metrics.sh fails in powerpc:

98: perf all metrics test : FAILED!

Logs with verbose:

[command]# ./perf test 98 -vv
 98: 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 with system wide monitoring to get the data.
Fix this issue by changing current system wide check from true workload to
sleep 0.01 workload.

Result with the patch changes in powerpc:

98: perf all metrics test : Ok

Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Disha Goel <disgoel@linux.ibm.com>
Suggested-by: Ian Rogers <irogers@google.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
Changelog:

v1->v2:
- Addressed review comments from Ian, by changing true workload
  to sleep workload in "perf all metric test". Rather then adding
  new system wide check with perf bench workload.
- Added Reviewed-by, Tested-by and Suggested-by tags.

 tools/perf/tests/shell/stat_all_metrics.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
index 6e79349e42be..22e9cb294b40 100755
--- 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
-- 
2.39.1

Re: [PATCH v2] tools/perf/tests: Change true workload to sleep workload in all metric test for system wide check

[Ian Rogers]

On Wed, Feb 15, 2023 at 1:38 AM Kajol Jain <kjain@linux.ibm.com> wrote:
>
> Testcase stat_all_metrics.sh fails in powerpc:
>
> 98: perf all metrics test : FAILED!
>
> Logs with verbose:
>
> [command]# ./perf test 98 -vv
>  98: 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 with system wide monitoring to get the data.
> Fix this issue by changing current system wide check from true workload to
> sleep 0.01 workload.
>
> Result with the patch changes in powerpc:
>
> 98: perf all metrics test : Ok
>
> Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Tested-by: Disha Goel <disgoel@linux.ibm.com>
> Suggested-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>

Tested-by: Ian Rogers <irogers@google.com>

The mention of a4b8cfcabb1d  can be moved to a Fixes tag so that this
is backported.

Thanks,
Ian

> ---
> Changelog:
>
> v1->v2:
> - Addressed review comments from Ian, by changing true workload
>   to sleep workload in "perf all metric test". Rather then adding
>   new system wide check with perf bench workload.
> - Added Reviewed-by, Tested-by and Suggested-by tags.
>
>  tools/perf/tests/shell/stat_all_metrics.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
> index 6e79349e42be..22e9cb294b40 100755
> --- 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
> --
> 2.39.1
>

Re: [PATCH v2] tools/perf/tests: Change true workload to sleep workload in all metric test for system wide check

[Arnaldo Carvalho de Melo]

Em Wed, Feb 15, 2023 at 08:08:20AM -0800, Ian Rogers escreveu:
> On Wed, Feb 15, 2023 at 1:38 AM Kajol Jain <kjain@linux.ibm.com> wrote:
> >
> > Testcase stat_all_metrics.sh fails in powerpc:
> >
> > 98: perf all metrics test : FAILED!
> >
> > Logs with verbose:
> >
> > [command]# ./perf test 98 -vv
> >  98: 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 with system wide monitoring to get the data.
> > Fix this issue by changing current system wide check from true workload to
> > sleep 0.01 workload.
> >
> > Result with the patch changes in powerpc:
> >
> > 98: perf all metrics test : Ok
> >
> > Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > Tested-by: Disha Goel <disgoel@linux.ibm.com>
> > Suggested-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> 
> Tested-by: Ian Rogers <irogers@google.com>
> 
> The mention of a4b8cfcabb1d  can be moved to a Fixes tag so that this
> is backported.

Done, thanks, applied.

- Arnaldo