Test every metric and metricgroup with 'true' as a workload.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/tests/shell/stat_all_metricgroups.sh | 12 ++++++++++++
tools/perf/tests/shell/stat_all_metrics.sh | 16 ++++++++++++++++
2 files changed, 28 insertions(+)
create mode 100755 tools/perf/tests/shell/stat_all_metricgroups.sh
create mode 100755 tools/perf/tests/shell/stat_all_metrics.sh
diff --git a/tools/perf/tests/shell/stat_all_metricgroups.sh b/tools/perf/tests/shell/stat_all_metricgroups.sh
new file mode 100755
index 000000000000..de24d374ce24
--- /dev/null
+++ b/tools/perf/tests/shell/stat_all_metricgroups.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+# perf all metricgroups test
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+for m in $(perf list --raw-dump metricgroups); do
+ echo "Testing $m"
+ perf stat -M "$m" true
+done
+
+exit 0
diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
new file mode 100755
index 000000000000..81b19ba27e68
--- /dev/null
+++ b/tools/perf/tests/shell/stat_all_metrics.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+# perf all metrics test
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+for m in `perf list --raw-dump metrics`; do
+ echo "Testing $m"
+ result=$(perf stat -M "$m" true)
+ if [[ "$result" =~ "$m" ]]; then
+ echo "Metric not printed: $m"
+ exit 1
+ fi
+done
+
+exit 0
--
2.33.0.309.g3052b89438-goog
On Wed, Sep 15, 2021 at 11:05:25PM -0700, Ian Rogers wrote:
> Test every metric and metricgroup with 'true' as a workload.
>
> Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
thanks,
jirka
> ---
> tools/perf/tests/shell/stat_all_metricgroups.sh | 12 ++++++++++++
> tools/perf/tests/shell/stat_all_metrics.sh | 16 ++++++++++++++++
> 2 files changed, 28 insertions(+)
> create mode 100755 tools/perf/tests/shell/stat_all_metricgroups.sh
> create mode 100755 tools/perf/tests/shell/stat_all_metrics.sh
>
> diff --git a/tools/perf/tests/shell/stat_all_metricgroups.sh b/tools/perf/tests/shell/stat_all_metricgroups.sh
> new file mode 100755
> index 000000000000..de24d374ce24
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat_all_metricgroups.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +# perf all metricgroups test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +for m in $(perf list --raw-dump metricgroups); do
> + echo "Testing $m"
> + perf stat -M "$m" true
> +done
> +
> +exit 0
> diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
> new file mode 100755
> index 000000000000..81b19ba27e68
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat_all_metrics.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +# perf all metrics test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +for m in `perf list --raw-dump metrics`; do
> + echo "Testing $m"
> + result=$(perf stat -M "$m" true)
> + if [[ "$result" =~ "$m" ]]; then
> + echo "Metric not printed: $m"
> + exit 1
> + fi
> +done
> +
> +exit 0
> --
> 2.33.0.309.g3052b89438-goog
>
On 16/09/2021 07:05, Ian Rogers wrote:
> Test every metric and metricgroup with 'true' as a workload.
>
> Signed-off-by: Ian Rogers<[email protected]>
Reviewed-by: John Garry <[email protected]>
Note that I also had a local test for pmu events:
for e in `$PERF list --raw-dump pmu`; do
echo "Testing $e"
result=$($PERF stat -v -e "$e" perf bench internals synthesize)
if [[ "$result" =~ "$e" ]]; then
echo "Event not printed: $e"
exit 1
fi
done
Is there any value in upstreaming this? I could not see same already
there. Or else make your new script generic, so that it accepts an
argument whether to test events or metrics or metricgroups
On Wed, Sep 15, 2021 at 11:05:25PM -0700, Ian Rogers wrote:
> Test every metric and metricgroup with 'true' as a workload.
Good idea! (However...)
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/tests/shell/stat_all_metricgroups.sh | 12 ++++++++++++
> tools/perf/tests/shell/stat_all_metrics.sh | 16 ++++++++++++++++
> 2 files changed, 28 insertions(+)
> create mode 100755 tools/perf/tests/shell/stat_all_metricgroups.sh
> create mode 100755 tools/perf/tests/shell/stat_all_metrics.sh
>
> diff --git a/tools/perf/tests/shell/stat_all_metricgroups.sh b/tools/perf/tests/shell/stat_all_metricgroups.sh
> new file mode 100755
> index 000000000000..de24d374ce24
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat_all_metricgroups.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +# perf all metricgroups test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +for m in $(perf list --raw-dump metricgroups); do
> + echo "Testing $m"
> + perf stat -M "$m" true
> +done
> +
> +exit 0
This always succeeds. Is that what you want?
Maybe check the return code from "perf", at least?
> diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
> new file mode 100755
> index 000000000000..81b19ba27e68
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat_all_metrics.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +# perf all metrics test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +for m in `perf list --raw-dump metrics`; do
> + echo "Testing $m"
> + result=$(perf stat -M "$m" true)
I don't think this is doing what you want it to do, as it just captures the
output of "true", which is always empty.
> + if [[ "$result" =~ "$m" ]]; then
So this always fails to match, and you'll never fail here, either. :-)
> + echo "Metric not printed: $m"
> + exit 1
> + fi
> +done
> +
> +exit 0
You may want to redirect the output of the "perf" command to a temporary file,
then grep within that. And, you'll need to remove the file before running the
perf command, because if it fails, it will leave any existing file untouched.
Thanks! :-)
PC
On Thu, Sep 16, 2021 at 5:04 AM Paul A. Clarke <[email protected]> wrote:
>
> On Wed, Sep 15, 2021 at 11:05:25PM -0700, Ian Rogers wrote:
> > Test every metric and metricgroup with 'true' as a workload.
>
> Good idea! (However...)
>
> > Signed-off-by: Ian Rogers <[email protected]>
> > ---
> > tools/perf/tests/shell/stat_all_metricgroups.sh | 12 ++++++++++++
> > tools/perf/tests/shell/stat_all_metrics.sh | 16 ++++++++++++++++
> > 2 files changed, 28 insertions(+)
> > create mode 100755 tools/perf/tests/shell/stat_all_metricgroups.sh
> > create mode 100755 tools/perf/tests/shell/stat_all_metrics.sh
> >
> > diff --git a/tools/perf/tests/shell/stat_all_metricgroups.sh b/tools/perf/tests/shell/stat_all_metricgroups.sh
> > new file mode 100755
> > index 000000000000..de24d374ce24
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/stat_all_metricgroups.sh
> > @@ -0,0 +1,12 @@
> > +#!/bin/sh
> > +# perf all metricgroups test
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +set -e
> > +
> > +for m in $(perf list --raw-dump metricgroups); do
> > + echo "Testing $m"
> > + perf stat -M "$m" true
> > +done
> > +
> > +exit 0
>
> This always succeeds. Is that what you want?
> Maybe check the return code from "perf", at least?
The "set -e" above means that if the perf command fails then the test exits.
> > diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
> > new file mode 100755
> > index 000000000000..81b19ba27e68
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/stat_all_metrics.sh
> > @@ -0,0 +1,16 @@
> > +#!/bin/sh
> > +# perf all metrics test
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +set -e
> > +
> > +for m in `perf list --raw-dump metrics`; do
> > + echo "Testing $m"
> > + result=$(perf stat -M "$m" true)
>
> I don't think this is doing what you want it to do, as it just captures the
> output of "true", which is always empty.
>
> > + if [[ "$result" =~ "$m" ]]; then
>
> So this always fails to match, and you'll never fail here, either. :-)
Doh! Thanks for catching this!
> > + echo "Metric not printed: $m"
> > + exit 1
> > + fi
> > +done
> > +
> > +exit 0
>
> You may want to redirect the output of the "perf" command to a temporary file,
> then grep within that. And, you'll need to remove the file before running the
> perf command, because if it fails, it will leave any existing file untouched.
I fixed it by redirecting stderr.
Thanks!
Ian
> Thanks! :-)
>
> PC
On Thu, Sep 16, 2021 at 12:37 AM John Garry <[email protected]> wrote:
>
> On 16/09/2021 07:05, Ian Rogers wrote:
> > Test every metric and metricgroup with 'true' as a workload.
> >
> > Signed-off-by: Ian Rogers<[email protected]>
>
> Reviewed-by: John Garry <[email protected]>
>
> Note that I also had a local test for pmu events:
> for e in `$PERF list --raw-dump pmu`; do
> echo "Testing $e"
> result=$($PERF stat -v -e "$e" perf bench internals synthesize)
> if [[ "$result" =~ "$e" ]]; then
> echo "Event not printed: $e"
> exit 1
> fi
> done
>
> Is there any value in upstreaming this? I could not see same already
> there. Or else make your new script generic, so that it accepts an
> argument whether to test events or metrics or metricgroups
It is not easy to make a generic script with the current shell test
infrastructure. I made a variant of this test:
https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
For skylake it ran for 1m15s and so it may be too slow. Perhaps we
need to add to the test infrastructure with some kind of speed flag.
Thanks,
Ian
On 17/09/2021 20:16, Ian Rogers wrote:
> On Thu, Sep 16, 2021 at 12:37 AM John Garry<[email protected]> wrote:
>> On 16/09/2021 07:05, Ian Rogers wrote:
>>> Test every metric and metricgroup with 'true' as a workload.
>>>
>>> Signed-off-by: Ian Rogers<[email protected]>
>> Reviewed-by: John Garry<[email protected]>
>>
>> Note that I also had a local test for pmu events:
>> for e in `$PERF list --raw-dump pmu`; do
>> echo "Testing $e"
>> result=$($PERF stat -v -e "$e" perf bench internals synthesize)
>> if [[ "$result" =~ "$e" ]]; then
>> echo "Event not printed: $e"
>> exit 1
>> fi
>> done
>>
>> Is there any value in upstreaming this? I could not see same already
>> there. Or else make your new script generic, so that it accepts an
>> argument whether to test events or metrics or metricgroups
> It is not easy to make a generic script with the current shell test
> infrastructure. I made a variant of this test:
> https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
> For skylake it ran for 1m15s and so it may be too slow. Perhaps we
> need to add to the test infrastructure with some kind of speed flag.
Hi Ian,
I suggested this before I realized that it would be called from "perf test".
You think that 1m15s could be considered too slow, but I think that it
could be much slower to now run "perf test" on some other systems. Like
my arm64 system - see series
https://lore.kernel.org/linux-perf-users/[email protected]/T/#t
- where I mention that we have >700 HW PMU events (before applying that
series to take advantage of the event merging). And each of those events
would be tested individually - slow...
So firstly maybe a speed or test level flag could be added before we try
this. Sorry for any inconvenience caused.
Thanks,
John
On Mon, Sep 20, 2021 at 3:00 AM John Garry <[email protected]> wrote:
>
> On 17/09/2021 20:16, Ian Rogers wrote:
> > On Thu, Sep 16, 2021 at 12:37 AM John Garry<[email protected]> wrote:
> >> On 16/09/2021 07:05, Ian Rogers wrote:
> >>> Test every metric and metricgroup with 'true' as a workload.
> >>>
> >>> Signed-off-by: Ian Rogers<[email protected]>
> >> Reviewed-by: John Garry<[email protected]>
> >>
> >> Note that I also had a local test for pmu events:
> >> for e in `$PERF list --raw-dump pmu`; do
> >> echo "Testing $e"
> >> result=$($PERF stat -v -e "$e" perf bench internals synthesize)
> >> if [[ "$result" =~ "$e" ]]; then
> >> echo "Event not printed: $e"
> >> exit 1
> >> fi
> >> done
> >>
> >> Is there any value in upstreaming this? I could not see same already
> >> there. Or else make your new script generic, so that it accepts an
> >> argument whether to test events or metrics or metricgroups
> > It is not easy to make a generic script with the current shell test
> > infrastructure. I made a variant of this test:
> > https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
> > For skylake it ran for 1m15s and so it may be too slow. Perhaps we
> > need to add to the test infrastructure with some kind of speed flag.
>
> Hi Ian,
>
> I suggested this before I realized that it would be called from "perf test".
>
> You think that 1m15s could be considered too slow, but I think that it
> could be much slower to now run "perf test" on some other systems. Like
> my arm64 system - see series
> https://lore.kernel.org/linux-perf-users/[email protected]/T/#t
> - where I mention that we have >700 HW PMU events (before applying that
> series to take advantage of the event merging). And each of those events
> would be tested individually - slow...
>
> So firstly maybe a speed or test level flag could be added before we try
> this. Sorry for any inconvenience caused.
Hi John,
I think a flag would be best. I'll look to add a notion of test sizes,
as that mirrors what works well at Google. We can then tag a test as
small, medium, large and default to just say running small and medium
tests.
Thanks,
Ian
> Thanks,
> John