[PATCH v1] perf test: Avoid system wide when not privileged

[PATCH v1] perf test: Avoid system wide when not privileged

[Ian Rogers]

Switch the test program to sleep that makes more sense for system wide
events. Only enable system wide when root or not paranoid. This avoids
failures under some testing conditions like ARM cloud.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/stat_all_metricgroups.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/shell/stat_all_metricgroups.sh b/tools/perf/tests/shell/stat_all_metricgroups.sh
index cb35e488809a..f3e305649e2c 100755
--- a/tools/perf/tests/shell/stat_all_metricgroups.sh
+++ b/tools/perf/tests/shell/stat_all_metricgroups.sh
@@ -4,9 +4,21 @@
 
 set -e
 
-for m in $(perf list --raw-dump metricgroups); do
+function ParanoidAndNotRoot()
+{
+  [ "$(id -u)" != 0 ] && [ "$(cat /proc/sys/kernel/perf_event_paranoid)" -gt $1 ]
+}
+
+system_wide_flag="-a"
+if ParanoidAndNotRoot 0
+then
+  system_wide_flag=""
+fi
+
+for m in $(perf list --raw-dump metricgroups)
+do
   echo "Testing $m"
-  perf stat -M "$m" -a true
+  perf stat -M "$m" $system_wide_flag sleep 0.01
 done
 
 exit 0
-- 
2.42.0.582.g8ccd20d70d-goog

Re: [PATCH v1] perf test: Avoid system wide when not privileged

[Namhyung Kim]

Hi Ian,

On Fri, Sep 29, 2023 at 11:02 PM Ian Rogers <irogers@google.com> wrote:
>
> Switch the test program to sleep that makes more sense for system wide
> events. Only enable system wide when root or not paranoid. This avoids
> failures under some testing conditions like ARM cloud.
>
> Signed-off-by: Ian Rogers <irogers@google.com>

While I'm ok with the change, I wonder why it doesn't have the same
code structure as in the stat_all_metrics.sh which checks the actual result.

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/tests/shell/stat_all_metricgroups.sh | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/shell/stat_all_metricgroups.sh b/tools/perf/tests/shell/stat_all_metricgroups.sh
> index cb35e488809a..f3e305649e2c 100755
> --- a/tools/perf/tests/shell/stat_all_metricgroups.sh
> +++ b/tools/perf/tests/shell/stat_all_metricgroups.sh
> @@ -4,9 +4,21 @@
>
>  set -e
>
> -for m in $(perf list --raw-dump metricgroups); do
> +function ParanoidAndNotRoot()
> +{
> +  [ "$(id -u)" != 0 ] && [ "$(cat /proc/sys/kernel/perf_event_paranoid)" -gt $1 ]
> +}
> +
> +system_wide_flag="-a"
> +if ParanoidAndNotRoot 0
> +then
> +  system_wide_flag=""
> +fi
> +
> +for m in $(perf list --raw-dump metricgroups)
> +do
>    echo "Testing $m"
> -  perf stat -M "$m" -a true
> +  perf stat -M "$m" $system_wide_flag sleep 0.01
>  done
>
>  exit 0
> --
> 2.42.0.582.g8ccd20d70d-goog
>

Re: [PATCH v1] perf test: Avoid system wide when not privileged

[Ian Rogers]

On Mon, Oct 2, 2023 at 9:47 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Fri, Sep 29, 2023 at 11:02 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Switch the test program to sleep that makes more sense for system wide
> > events. Only enable system wide when root or not paranoid. This avoids
> > failures under some testing conditions like ARM cloud.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> While I'm ok with the change, I wonder why it doesn't have the same
> code structure as in the stat_all_metrics.sh which checks the actual result.
>
> Acked-by: Namhyung Kim <namhyung@kernel.org>

The metric group isn't printed when we print the metrics within it.
This could be changed.

Thanks,
Ian

> Thanks,
> Namhyung
>
> > ---
> >  tools/perf/tests/shell/stat_all_metricgroups.sh | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/stat_all_metricgroups.sh b/tools/perf/tests/shell/stat_all_metricgroups.sh
> > index cb35e488809a..f3e305649e2c 100755
> > --- a/tools/perf/tests/shell/stat_all_metricgroups.sh
> > +++ b/tools/perf/tests/shell/stat_all_metricgroups.sh
> > @@ -4,9 +4,21 @@
> >
> >  set -e
> >
> > -for m in $(perf list --raw-dump metricgroups); do
> > +function ParanoidAndNotRoot()
> > +{
> > +  [ "$(id -u)" != 0 ] && [ "$(cat /proc/sys/kernel/perf_event_paranoid)" -gt $1 ]
> > +}
> > +
> > +system_wide_flag="-a"
> > +if ParanoidAndNotRoot 0
> > +then
> > +  system_wide_flag=""
> > +fi
> > +
> > +for m in $(perf list --raw-dump metricgroups)
> > +do
> >    echo "Testing $m"
> > -  perf stat -M "$m" -a true
> > +  perf stat -M "$m" $system_wide_flag sleep 0.01
> >  done
> >
> >  exit 0
> > --
> > 2.42.0.582.g8ccd20d70d-goog
> >

Re: [PATCH v1] perf test: Avoid system wide when not privileged

[Namhyung Kim]

On Mon, Oct 2, 2023 at 10:10 PM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Oct 2, 2023 at 9:47 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hi Ian,
> >
> > On Fri, Sep 29, 2023 at 11:02 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > Switch the test program to sleep that makes more sense for system wide
> > > events. Only enable system wide when root or not paranoid. This avoids
> > > failures under some testing conditions like ARM cloud.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> >
> > While I'm ok with the change, I wonder why it doesn't have the same
> > code structure as in the stat_all_metrics.sh which checks the actual result.
> >
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
>
> The metric group isn't printed when we print the metrics within it.
> This could be changed.

Ok, we can change it later.  Applied to perf-tools-next, thanks!