2023-09-30 10:21:11

by Ian Rogers

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

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 <[email protected]>
---
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

2023-10-03 04:47:15

by Namhyung Kim

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

Hi Ian,

On Fri, Sep 29, 2023 at 11:02 PM Ian Rogers <[email protected]> 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 <[email protected]>

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 <[email protected]>

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
>

2023-10-03 05:10:53

by Ian Rogers

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

On Mon, Oct 2, 2023 at 9:47 PM Namhyung Kim <[email protected]> wrote:
>
> Hi Ian,
>
> On Fri, Sep 29, 2023 at 11:02 PM Ian Rogers <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>

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
> >

2023-10-09 05:24:59

by Namhyung Kim

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

On Mon, Oct 2, 2023 at 10:10 PM Ian Rogers <[email protected]> wrote:
>
> On Mon, Oct 2, 2023 at 9:47 PM Namhyung Kim <[email protected]> wrote:
> >
> > Hi Ian,
> >
> > On Fri, Sep 29, 2023 at 11:02 PM Ian Rogers <[email protected]> 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 <[email protected]>
> >
> > 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 <[email protected]>
>
> 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!