[PATCH 1/2] perf test attr: fix python error on empty result

[PATCH 1/2] perf test attr: fix python error on empty result

[Thomas Richter]

Commit d78ada4a767 ("perf tests attr: Do not store failed events")
does not create an event file in the /tmp directory when the
perf_open_event() system call failed.
This can lead to a situation where not /tmp/event-xx-yy-zz result
file exists at all (for example on a s390x virtual machine environment)
where no CPUMF hardware is available.

The following command then fails with a python call back chain
instead of printing failure:
[root@s8360046 perf]# /usr/bin/python2 ./tests/attr.py -d ./tests/attr/ \
    -p ./perf -v -ttest-stat-basic
running './tests/attr//test-stat-basic'
Traceback (most recent call last):
  File "./tests/attr.py", line 379, in <module>
    main()
  File "./tests/attr.py", line 370, in main
    run_tests(options)
  File "./tests/attr.py", line 311, in run_tests
    Test(f, options).run()
  File "./tests/attr.py", line 300, in run
    self.compare(self.expect, self.result)
  File "./tests/attr.py", line 248, in compare
    exp_event.diff(res_event)
UnboundLocalError: local variable 'res_event' referenced before assignment
[root@s8360046 perf]#

This patch catches this pitfall and prints an error message
instead:
[root@s8360047 perf]# /usr/bin/python2 ./tests/attr.py -d ./tests/attr/ \
     -p ./perf  -vvv -ttest-stat-basic
running './tests/attr//test-stat-basic'
  loading expected events
    Event event:base-stat
      fd = 1
      group_fd = -1
      flags = 0|8
      [....]
      sample_regs_user = 0
      sample_stack_user = 0
  'PERF_TEST_ATTR=/tmp/tmpJbMQMP ./perf stat -o /tmp/tmpJbMQMP/perf.data -e cycles kill >/dev/null 2>&1' ret '1', expected '1'
  loading result events
  compare
    matching [event:base-stat]
    match: [event:base-stat] matches []
    res_event is empty
FAILED './tests/attr//test-stat-basic' - match failure
[root@s8360047 perf]#

Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
---
 tools/perf/tests/attr.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/attr.py b/tools/perf/tests/attr.py
index 6bb50e8..a13cd78 100644
--- a/tools/perf/tests/attr.py
+++ b/tools/perf/tests/attr.py
@@ -237,6 +237,7 @@ class Test(object):
         # events in result. Fail if there's not any.
         for exp_name, exp_event in expect.items():
             exp_list = []
+            res_event = {}
             log.debug("    matching [%s]" % exp_name)
             for res_name, res_event in result.items():
                 log.debug("      to [%s]" % res_name)
@@ -253,7 +254,10 @@ class Test(object):
                 if exp_event.optional():
                     log.debug("    %s does not match, but is optional" % exp_name)
                 else:
-                    exp_event.diff(res_event)
+                    if not res_event:
+                        log.debug("    res_event is empty");
+                    else:
+                        exp_event.diff(res_event)
                     raise Fail(self, 'match failure');
 
             match[exp_name] = exp_list
-- 
2.9.3

[PATCH 2/2] perf test attr: fix ignored test case result

[Thomas Richter]

Command perf test -v 16 (Setup struct perf_event_attr test)
always reports success even if the test case fails.
It works correctly if you also specify -F (for don't fork).

   root@s35lp76 perf]# ./perf test -v 16
   15: Setup struct perf_event_attr               :
   --- start ---
   running './tests/attr/test-record-no-delay'
   [ perf record: Woken up 1 times to write data ]
   [ perf record: Captured and wrote 0.002 MB /tmp/tmp4E1h7R/perf.data
     (1 samples) ]
   expected task=0, got 1
   expected precise_ip=0, got 3
   expected wakeup_events=1, got 0
   FAILED './tests/attr/test-record-no-delay' - match failure
   test child finished with 0
   ---- end ----
   Setup struct perf_event_attr: Ok

The reason for the wrong error reporting is the return value of the
system() library call. It is called in run_dir() file tests/attr.c
and returns the exit status, in above case 0xff00.
This value is given as parameter to the exit() function which
can only handle values 0-0xff.
The child process terminates with exit value of 0 and the parent
does not detect any error.

This patch corrects the error reporting and prints the
correct test result.

Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
---
 tools/perf/tests/attr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/attr.c b/tools/perf/tests/attr.c
index c9aafed..25ede44 100644
--- a/tools/perf/tests/attr.c
+++ b/tools/perf/tests/attr.c
@@ -166,7 +166,7 @@ static int run_dir(const char *d, const char *perf)
 	snprintf(cmd, 3*PATH_MAX, PYTHON " %s/attr.py -d %s/attr/ -p %s %.*s",
 		 d, d, perf, vcnt, v);
 
-	return system(cmd);
+	return system(cmd) ? TEST_FAIL : TEST_OK;
 }
 
 int test__attr(struct test *test __maybe_unused, int subtest __maybe_unused)
-- 
2.9.3

Re: [PATCH 1/2] perf test attr: fix python error on empty result

[Jiri Olsa]

On Wed, Sep 13, 2017 at 10:12:08AM +0200, Thomas Richter wrote:
> Commit d78ada4a767 ("perf tests attr: Do not store failed events")
> does not create an event file in the /tmp directory when the
> perf_open_event() system call failed.
> This can lead to a situation where not /tmp/event-xx-yy-zz result
> file exists at all (for example on a s390x virtual machine environment)
> where no CPUMF hardware is available.

those 2 were already sent earlier if I'm not mistaken,
I think I've already acked them, if not:

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

Re: [PATCH 1/2] perf test attr: fix python error on empty result

[Thomas-Mich Richter]

On 09/15/2017 08:45 PM, Jiri Olsa wrote:
> On Wed, Sep 13, 2017 at 10:12:08AM +0200, Thomas Richter wrote:
>> Commit d78ada4a767 ("perf tests attr: Do not store failed events")
>> does not create an event file in the /tmp directory when the
>> perf_open_event() system call failed.
>> This can lead to a situation where not /tmp/event-xx-yy-zz result
>> file exists at all (for example on a s390x virtual machine environment)
>> where no CPUMF hardware is available.
> 
> those 2 were already sent earlier if I'm not mistaken,
> I think I've already acked them, if not:

I developed the first patch just before my vacation and submitted
it for the first time after my vacation.

The second patch was submitted before but is not part of 4.14.0-rc1
so I guess it got lost.

> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> 
> thanks,
> jirka
>

Thanks
-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH 2/2] perf test attr: fix ignored test case result

[Arnaldo Carvalho de Melo]

Em Wed, Sep 13, 2017 at 10:12:09AM +0200, Thomas Richter escreveu:
> Command perf test -v 16 (Setup struct perf_event_attr test)
> always reports success even if the test case fails.
> It works correctly if you also specify -F (for don't fork).

So, now I'm getting:

[root@jouet ~]# uname -a
Linux jouet 4.13.0+ #3 SMP Mon Sep 25 11:51:22 -03 2017 x86_64 x86_64 x86_64 GNU/Linux
[root@jouet ~]# perf test -vv 16
16: Setup struct perf_event_attr                          :
--- start ---
test child forked, pid 10932
running '/home/acme/libexec/perf-core/tests/attr/test-stat-group'
  'PERF_TEST_ATTR=/tmp/tmpI_lp9P /home/acme/bin/perf stat -o /tmp/tmpI_lp9P/perf.data --group -e cycles,instructions kill >/dev/null 2>&1' ret '1', expected '1'
expected read_format=3, got 15
FAILED '/home/acme/libexec/perf-core/tests/attr/test-stat-group' - match failure
test child finished with -1
---- end ----
Setup struct perf_event_attr: FAILED!
[root@jouet ~]#

Jiri?

- Arnaldo
 
>    root@s35lp76 perf]# ./perf test -v 16
>    15: Setup struct perf_event_attr               :
>    --- start ---
>    running './tests/attr/test-record-no-delay'
>    [ perf record: Woken up 1 times to write data ]
>    [ perf record: Captured and wrote 0.002 MB /tmp/tmp4E1h7R/perf.data
>      (1 samples) ]
>    expected task=0, got 1
>    expected precise_ip=0, got 3
>    expected wakeup_events=1, got 0
>    FAILED './tests/attr/test-record-no-delay' - match failure
>    test child finished with 0
>    ---- end ----
>    Setup struct perf_event_attr: Ok
> 
> The reason for the wrong error reporting is the return value of the
> system() library call. It is called in run_dir() file tests/attr.c
> and returns the exit status, in above case 0xff00.
> This value is given as parameter to the exit() function which
> can only handle values 0-0xff.
> The child process terminates with exit value of 0 and the parent
> does not detect any error.
> 
> This patch corrects the error reporting and prints the
> correct test result.
> 
> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
> ---
>  tools/perf/tests/attr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/attr.c b/tools/perf/tests/attr.c
> index c9aafed..25ede44 100644
> --- a/tools/perf/tests/attr.c
> +++ b/tools/perf/tests/attr.c
> @@ -166,7 +166,7 @@ static int run_dir(const char *d, const char *perf)
>  	snprintf(cmd, 3*PATH_MAX, PYTHON " %s/attr.py -d %s/attr/ -p %s %.*s",
>  		 d, d, perf, vcnt, v);
>  
> -	return system(cmd);
> +	return system(cmd) ? TEST_FAIL : TEST_OK;
>  }
>  
>  int test__attr(struct test *test __maybe_unused, int subtest __maybe_unused)
> -- 
> 2.9.3

Re: [PATCH 2/2] perf test attr: fix ignored test case result

[Jiri Olsa]

On Thu, Sep 28, 2017 at 12:26:51PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 13, 2017 at 10:12:09AM +0200, Thomas Richter escreveu:
> > Command perf test -v 16 (Setup struct perf_event_attr test)
> > always reports success even if the test case fails.
> > It works correctly if you also specify -F (for don't fork).
> 
> So, now I'm getting:
> 
> [root@jouet ~]# uname -a
> Linux jouet 4.13.0+ #3 SMP Mon Sep 25 11:51:22 -03 2017 x86_64 x86_64 x86_64 GNU/Linux
> [root@jouet ~]# perf test -vv 16
> 16: Setup struct perf_event_attr                          :
> --- start ---
> test child forked, pid 10932
> running '/home/acme/libexec/perf-core/tests/attr/test-stat-group'
>   'PERF_TEST_ATTR=/tmp/tmpI_lp9P /home/acme/bin/perf stat -o /tmp/tmpI_lp9P/perf.data --group -e cycles,instructions kill >/dev/null 2>&1' ret '1', expected '1'
> expected read_format=3, got 15
> FAILED '/home/acme/libexec/perf-core/tests/attr/test-stat-group' - match failure
> test child finished with -1
> ---- end ----
> Setup struct perf_event_attr: FAILED!
> [root@jouet ~]#
> 
> Jiri?


there was the change to use group read when we can,
so we need following change I think

I'll check more and post full patch

jirka


---
diff --git a/tools/perf/tests/attr/test-stat-group b/tools/perf/tests/attr/test-stat-group
index fdc1596a8862..e15d6946e9b3 100644
--- a/tools/perf/tests/attr/test-stat-group
+++ b/tools/perf/tests/attr/test-stat-group
@@ -6,6 +6,7 @@ ret     = 1
 [event-1:base-stat]
 fd=1
 group_fd=-1
+read_format=3|15
 
 [event-2:base-stat]
 fd=2
@@ -13,3 +14,4 @@ group_fd=1
 config=1
 disabled=0
 enable_on_exec=0
+read_format=3|15
diff --git a/tools/perf/tests/attr/test-stat-group1 b/tools/perf/tests/attr/test-stat-group1
index 2a1f86e4a904..1746751123dc 100644
--- a/tools/perf/tests/attr/test-stat-group1
+++ b/tools/perf/tests/attr/test-stat-group1
@@ -6,6 +6,7 @@ ret     = 1
 [event-1:base-stat]
 fd=1
 group_fd=-1
+read_format=3|15
 
 [event-2:base-stat]
 fd=2
@@ -13,3 +14,4 @@ group_fd=1
 config=1
 disabled=0
 enable_on_exec=0
+read_format=3|15

[PATCH] perf tests attr: Fix group stat tests

[Jiri Olsa]

On Thu, Sep 28, 2017 at 05:58:59PM +0200, Jiri Olsa wrote:
> On Thu, Sep 28, 2017 at 12:26:51PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Sep 13, 2017 at 10:12:09AM +0200, Thomas Richter escreveu:
> > > Command perf test -v 16 (Setup struct perf_event_attr test)
> > > always reports success even if the test case fails.
> > > It works correctly if you also specify -F (for don't fork).
> > 
> > So, now I'm getting:
> > 
> > [root@jouet ~]# uname -a
> > Linux jouet 4.13.0+ #3 SMP Mon Sep 25 11:51:22 -03 2017 x86_64 x86_64 x86_64 GNU/Linux
> > [root@jouet ~]# perf test -vv 16
> > 16: Setup struct perf_event_attr                          :
> > --- start ---
> > test child forked, pid 10932
> > running '/home/acme/libexec/perf-core/tests/attr/test-stat-group'
> >   'PERF_TEST_ATTR=/tmp/tmpI_lp9P /home/acme/bin/perf stat -o /tmp/tmpI_lp9P/perf.data --group -e cycles,instructions kill >/dev/null 2>&1' ret '1', expected '1'
> > expected read_format=3, got 15
> > FAILED '/home/acme/libexec/perf-core/tests/attr/test-stat-group' - match failure
> > test child finished with -1
> > ---- end ----
> > Setup struct perf_event_attr: FAILED!
> > [root@jouet ~]#
> > 
> > Jiri?
> 
> 
> there was the change to use group read when we can,
> so we need following change I think
> 
> I'll check more and post full patch

---
We started to use group read whenever it's possible:
  82bf311e15d2 perf stat: Use group read for event groups

That breaks some of attr tests, this change adds the
new possible read_format value.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/attr/test-stat-group  | 2 ++
 tools/perf/tests/attr/test-stat-group1 | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/tools/perf/tests/attr/test-stat-group b/tools/perf/tests/attr/test-stat-group
index fdc1596a8862..e15d6946e9b3 100644
--- a/tools/perf/tests/attr/test-stat-group
+++ b/tools/perf/tests/attr/test-stat-group
@@ -6,6 +6,7 @@ ret     = 1
 [event-1:base-stat]
 fd=1
 group_fd=-1
+read_format=3|15
 
 [event-2:base-stat]
 fd=2
@@ -13,3 +14,4 @@ group_fd=1
 config=1
 disabled=0
 enable_on_exec=0
+read_format=3|15
diff --git a/tools/perf/tests/attr/test-stat-group1 b/tools/perf/tests/attr/test-stat-group1
index 2a1f86e4a904..1746751123dc 100644
--- a/tools/perf/tests/attr/test-stat-group1
+++ b/tools/perf/tests/attr/test-stat-group1
@@ -6,6 +6,7 @@ ret     = 1
 [event-1:base-stat]
 fd=1
 group_fd=-1
+read_format=3|15
 
 [event-2:base-stat]
 fd=2
@@ -13,3 +14,4 @@ group_fd=1
 config=1
 disabled=0
 enable_on_exec=0
+read_format=3|15
-- 
2.13.5