2023-05-24 21:13:38

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH] perf test: Fix perf stat JSON output test

The recent --per-cache option test caused a problem. According to
the option name, I think it should check args.per_cache instead of
args.per_cache_instance.

$ sudo ./perf test -v 99
99: perf stat JSON output linter :
--- start ---
test child forked, pid 3086101
Checking json output: no args [Success]
Checking json output: system wide [Success]
Checking json output: interval [Success]
Checking json output: event [Success]
Checking json output: per thread [Success]
Checking json output: per node [Success]
Checking json output: system wide no aggregation [Success]
Checking json output: per core [Success]
Checking json output: per cache_instance Test failed for input:
...
Traceback (most recent call last):
File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
test child finished with -1
---- end ----
perf stat JSON output linter: FAILED!

Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")
Cc: K Prateek Nayak <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/tests/shell/lib/perf_json_output_lint.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
index 4acaaed5560d..b81582a89d36 100644
--- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
+++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
@@ -85,7 +85,7 @@ Lines = args.file.readlines()
expected_items = 7
elif args.interval or args.per_thread or args.system_wide_no_aggr:
expected_items = 8
- elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
+ elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache:
expected_items = 9
else:
# If no option is specified, don't check the number of items.
--
2.41.0.rc0.172.g3f132b7071-goog



2023-05-25 03:11:13

by K Prateek Nayak

[permalink] [raw]
Subject: Re: [PATCH] perf test: Fix perf stat JSON output test

Hello Namhyung,

On 5/25/2023 2:36 AM, Namhyung Kim wrote:
> The recent --per-cache option test caused a problem. According to
> the option name, I think it should check args.per_cache instead of
> args.per_cache_instance.
>
> $ sudo ./perf test -v 99
> 99: perf stat JSON output linter :
> --- start ---
> test child forked, pid 3086101
> Checking json output: no args [Success]
> Checking json output: system wide [Success]
> Checking json output: interval [Success]
> Checking json output: event [Success]
> Checking json output: per thread [Success]
> Checking json output: per node [Success]
> Checking json output: system wide no aggregation [Success]
> Checking json output: per core [Success]
> Checking json output: per cache_instance Test failed for input:
> ...
> Traceback (most recent call last):
> File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
> elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
> test child finished with -1
> ---- end ----
> perf stat JSON output linter: FAILED!
>
> Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")

Another oversight on my part. Thank you for fixing this :)

$ sudo perf test -v 99
99: perf stat JSON output linter :
--- start ---
test child forked, pid 25046
Checking json output: no args [Success]
Checking json output: system wide [Success]
Checking json output: interval [Success]
Checking json output: event [Success]
Checking json output: per thread [Success]
Checking json output: per node [Success]
Checking json output: system wide no aggregation [Success]
Checking json output: per core [Success]
Checking json output: per cache_instance [Success]
Checking json output: per die [Success]
Checking json output: per socket [Success]
test child finished with 0
---- end ----
perf stat JSON output linter: Ok

Tested-by: K Prateek Nayak <[email protected]>

> Cc: K Prateek Nayak <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/tests/shell/lib/perf_json_output_lint.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> index 4acaaed5560d..b81582a89d36 100644
> --- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
> +++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> @@ -85,7 +85,7 @@ Lines = args.file.readlines()
> expected_items = 7
> elif args.interval or args.per_thread or args.system_wide_no_aggr:
> expected_items = 8
> - elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> + elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache:
> expected_items = 9
> else:
> # If no option is specified, don't check the number of items.

--
Thanks and Regards,
Prateek

2023-05-26 22:53:13

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH] perf test: Fix perf stat JSON output test

On Wed, May 24, 2023 at 8:01 PM K Prateek Nayak <[email protected]> wrote:
>
> Hello Namhyung,
>
> On 5/25/2023 2:36 AM, Namhyung Kim wrote:
> > The recent --per-cache option test caused a problem. According to
> > the option name, I think it should check args.per_cache instead of
> > args.per_cache_instance.
> >
> > $ sudo ./perf test -v 99
> > 99: perf stat JSON output linter :
> > --- start ---
> > test child forked, pid 3086101
> > Checking json output: no args [Success]
> > Checking json output: system wide [Success]
> > Checking json output: interval [Success]
> > Checking json output: event [Success]
> > Checking json output: per thread [Success]
> > Checking json output: per node [Success]
> > Checking json output: system wide no aggregation [Success]
> > Checking json output: per core [Success]
> > Checking json output: per cache_instance Test failed for input:
> > ...
> > Traceback (most recent call last):
> > File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
> > elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> > AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
> > test child finished with -1
> > ---- end ----
> > perf stat JSON output linter: FAILED!
> >
> > Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")
>
> Another oversight on my part. Thank you for fixing this :)
>
> $ sudo perf test -v 99
> 99: perf stat JSON output linter :
> --- start ---
> test child forked, pid 25046
> Checking json output: no args [Success]
> Checking json output: system wide [Success]
> Checking json output: interval [Success]
> Checking json output: event [Success]
> Checking json output: per thread [Success]
> Checking json output: per node [Success]
> Checking json output: system wide no aggregation [Success]
> Checking json output: per core [Success]
> Checking json output: per cache_instance [Success]
> Checking json output: per die [Success]
> Checking json output: per socket [Success]
> test child finished with 0
> ---- end ----
> perf stat JSON output linter: Ok
>
> Tested-by: K Prateek Nayak <[email protected]>

Thanks Namhyung and Prateek, Arnaldo could we get this in
perf-tools-next so that the failing test goes away?

Acked-by: Ian Rogers <[email protected]>

> > Cc: K Prateek Nayak <[email protected]>
> > Signed-off-by: Namhyung Kim <[email protected]>
> > ---
> > tools/perf/tests/shell/lib/perf_json_output_lint.py | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > index 4acaaed5560d..b81582a89d36 100644
> > --- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > +++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
> > @@ -85,7 +85,7 @@ Lines = args.file.readlines()
> > expected_items = 7
> > elif args.interval or args.per_thread or args.system_wide_no_aggr:
> > expected_items = 8
> > - elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> > + elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache:
> > expected_items = 9
> > else:
> > # If no option is specified, don't check the number of items.
>
> --
> Thanks and Regards,
> Prateek

2023-05-27 02:11:58

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf test: Fix perf stat JSON output test

Em Fri, May 26, 2023 at 03:41:29PM -0700, Ian Rogers escreveu:
> On Wed, May 24, 2023 at 8:01 PM K Prateek Nayak <[email protected]> wrote:
> >
> > Hello Namhyung,
> >
> > On 5/25/2023 2:36 AM, Namhyung Kim wrote:
> > > The recent --per-cache option test caused a problem. According to
> > > the option name, I think it should check args.per_cache instead of
> > > args.per_cache_instance.
> > >
> > > $ sudo ./perf test -v 99
> > > 99: perf stat JSON output linter :
> > > --- start ---
> > > test child forked, pid 3086101
> > > Checking json output: no args [Success]
> > > Checking json output: system wide [Success]
> > > Checking json output: interval [Success]
> > > Checking json output: event [Success]
> > > Checking json output: per thread [Success]
> > > Checking json output: per node [Success]
> > > Checking json output: system wide no aggregation [Success]
> > > Checking json output: per core [Success]
> > > Checking json output: per cache_instance Test failed for input:
> > > ...
> > > Traceback (most recent call last):
> > > File "linux/tools/perf/tests/shell/lib/perf_json_output_lint.py", line 88, in <module>
> > > elif args.per_core or args.per_socket or args.per_node or args.per_die or args.per_cache_instance:
> > > AttributeError: 'Namespace' object has no attribute 'per_cache_instance'
> > > test child finished with -1
> > > ---- end ----
> > > perf stat JSON output linter: FAILED!
> > >
> > > Fixes: bfce728db317 ("pert tests: Add tests for new "perf stat --per-cache" aggregation option")
> >
> > Another oversight on my part. Thank you for fixing this :)
> >
> > $ sudo perf test -v 99
> > 99: perf stat JSON output linter :
> > --- start ---
> > test child forked, pid 25046
> > Checking json output: no args [Success]
> > Checking json output: system wide [Success]
> > Checking json output: interval [Success]
> > Checking json output: event [Success]
> > Checking json output: per thread [Success]
> > Checking json output: per node [Success]
> > Checking json output: system wide no aggregation [Success]
> > Checking json output: per core [Success]
> > Checking json output: per cache_instance [Success]
> > Checking json output: per die [Success]
> > Checking json output: per socket [Success]
> > test child finished with 0
> > ---- end ----
> > perf stat JSON output linter: Ok
> >
> > Tested-by: K Prateek Nayak <[email protected]>
>
> Thanks Namhyung and Prateek, Arnaldo could we get this in
> perf-tools-next so that the failing test goes away?
>
> Acked-by: Ian Rogers <[email protected]>

Applied and pushed to perf-tools-next, please continue from there.

⬢[acme@toolbox perf-tools-next]$ git log --oneline -10
540c910c65a94fb4 (HEAD -> perf-tools-next) perf test: Fix perf stat JSON output test
5cebb33fd929dc67 perf tests: Organize cpu_map tests into a single suite
237d41d4a2d7d45e perf cpumap: Add intersect function
6ac2230b55d392e6 perf vendor events intel: Add metricgroup descriptions for all models
66c6e0c100277175 perf jevents: Add support for metricgroup descriptions
bfce728db3179042 pert tests: Add tests for new "perf stat --per-cache" aggregation option
aab667ca8837e45f perf stat: Add "--per-cache" aggregation option and document it
4b87406a3b590888 perf stat record: Save cache level information
995ed074b829f293 perf stat: Setup the foundation to allow aggregation based on cache topology
2b72cec9eef19d73 perf: Extract building cache level for a CPU into separate function
⬢[acme@toolbox perf-tools-next]$

- Arnaldo