* [PATCH v2] perf test: Workload test of metric and metricgroups @ 2021-09-16 6:05 Ian Rogers 2021-09-16 7:30 ` Jiri Olsa ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Ian Rogers @ 2021-09-16 6:05 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel Cc: eranian, Ian Rogers Test every metric and metricgroup with 'true' as a workload. Signed-off-by: Ian Rogers <irogers@google.com> --- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf test: Workload test of metric and metricgroups 2021-09-16 6:05 [PATCH v2] perf test: Workload test of metric and metricgroups Ian Rogers @ 2021-09-16 7:30 ` Jiri Olsa 2021-09-16 7:41 ` John Garry 2021-09-16 12:04 ` Paul A. Clarke 2 siblings, 0 replies; 8+ messages in thread From: Jiri Olsa @ 2021-09-16 7:30 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel, eranian 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 <irogers@google.com> Acked-by: Jiri Olsa <jolsa@redhat.com> 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf test: Workload test of metric and metricgroups 2021-09-16 6:05 [PATCH v2] perf test: Workload test of metric and metricgroups Ian Rogers 2021-09-16 7:30 ` Jiri Olsa @ 2021-09-16 7:41 ` John Garry 2021-09-17 19:16 ` Ian Rogers 2021-09-16 12:04 ` Paul A. Clarke 2 siblings, 1 reply; 8+ messages in thread From: John Garry @ 2021-09-16 7:41 UTC (permalink / raw) To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel Cc: eranian On 16/09/2021 07:05, Ian Rogers wrote: > Test every metric and metricgroup with 'true' as a workload. > > Signed-off-by: Ian Rogers<irogers@google.com> Reviewed-by: John Garry <john.garry@huawei.com> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf test: Workload test of metric and metricgroups 2021-09-16 7:41 ` John Garry @ 2021-09-17 19:16 ` Ian Rogers 2021-09-20 10:04 ` John Garry 0 siblings, 1 reply; 8+ messages in thread From: Ian Rogers @ 2021-09-17 19:16 UTC (permalink / raw) To: John Garry Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel, eranian, Paul Clarke On Thu, Sep 16, 2021 at 12:37 AM John Garry <john.garry@huawei.com> 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<irogers@google.com> > > Reviewed-by: John Garry <john.garry@huawei.com> > > 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/20210917184240.2181186-2-irogers@google.com/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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf test: Workload test of metric and metricgroups 2021-09-17 19:16 ` Ian Rogers @ 2021-09-20 10:04 ` John Garry 2021-09-21 5:05 ` Ian Rogers 0 siblings, 1 reply; 8+ messages in thread From: John Garry @ 2021-09-20 10:04 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel, eranian, Paul Clarke On 17/09/2021 20:16, Ian Rogers wrote: > On Thu, Sep 16, 2021 at 12:37 AM John Garry<john.garry@huawei.com> 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<irogers@google.com> >> Reviewed-by: John Garry<john.garry@huawei.com> >> >> 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/20210917184240.2181186-2-irogers@google.com/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/1631795665-240946-1-git-send-email-john.garry@huawei.com/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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf test: Workload test of metric and metricgroups 2021-09-20 10:04 ` John Garry @ 2021-09-21 5:05 ` Ian Rogers 0 siblings, 0 replies; 8+ messages in thread From: Ian Rogers @ 2021-09-21 5:05 UTC (permalink / raw) To: John Garry Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel, eranian, Paul Clarke On Mon, Sep 20, 2021 at 3:00 AM John Garry <john.garry@huawei.com> wrote: > > On 17/09/2021 20:16, Ian Rogers wrote: > > On Thu, Sep 16, 2021 at 12:37 AM John Garry<john.garry@huawei.com> 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<irogers@google.com> > >> Reviewed-by: John Garry<john.garry@huawei.com> > >> > >> 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/20210917184240.2181186-2-irogers@google.com/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/1631795665-240946-1-git-send-email-john.garry@huawei.com/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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf test: Workload test of metric and metricgroups 2021-09-16 6:05 [PATCH v2] perf test: Workload test of metric and metricgroups Ian Rogers 2021-09-16 7:30 ` Jiri Olsa 2021-09-16 7:41 ` John Garry @ 2021-09-16 12:04 ` Paul A. Clarke 2021-09-17 19:17 ` Ian Rogers 2 siblings, 1 reply; 8+ messages in thread From: Paul A. Clarke @ 2021-09-16 12:04 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel, eranian 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 <irogers@google.com> > --- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf test: Workload test of metric and metricgroups 2021-09-16 12:04 ` Paul A. Clarke @ 2021-09-17 19:17 ` Ian Rogers 0 siblings, 0 replies; 8+ messages in thread From: Ian Rogers @ 2021-09-17 19:17 UTC (permalink / raw) To: Paul A. Clarke, John Garry Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Jin Yao, linux-perf-users, linux-kernel, eranian On Thu, Sep 16, 2021 at 5:04 AM Paul A. Clarke <pc@us.ibm.com> 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 <irogers@google.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-21 5:06 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-16 6:05 [PATCH v2] perf test: Workload test of metric and metricgroups Ian Rogers 2021-09-16 7:30 ` Jiri Olsa 2021-09-16 7:41 ` John Garry 2021-09-17 19:16 ` Ian Rogers 2021-09-20 10:04 ` John Garry 2021-09-21 5:05 ` Ian Rogers 2021-09-16 12:04 ` Paul A. Clarke 2021-09-17 19:17 ` Ian Rogers
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.