All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf test 39 (Session topology) dumps core on s390
@ 2018-05-24 13:56 Thomas Richter
  2018-05-25 15:11 ` Arnaldo Carvalho de Melo
  2018-05-28  3:49 ` Ravi Bangoria
  0 siblings, 2 replies; 4+ messages in thread
From: Thomas Richter @ 2018-05-24 13:56 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users, acme
  Cc: brueckner, schwidefsky, heiko.carstens, Thomas Richter

perf test 39 fails with core dump on s390. The root cause
is a NULL pointer reference in function check_cpu_topology()
line 76 (or line 82 without -v).
The session->header.env.cpu variable is NULL because on s390
function process_cpu_topology() returns with error:
    socket_id number is too big.
    You may need to upgrade the perf tool.
and releases the env.cpu variable via zfree() and sets it to NULL.

Here is the gdb output:
(gdb) n
76                      pr_debug("CPU %d, core %d, socket %d\n", i,
(gdb) n

Program received signal SIGSEGV, Segmentation fault.
0x00000000010f4d9e in check_cpu_topology (path=0x3ffffffd6c8
	"/tmp/perf-test-J6CHMa", map=0x14a1740) at tests/topology.c:76
76  pr_debug("CPU %d, core %d, socket %d\n", i,
(gdb)

Make sure the env.cpu variable is not used when its NULL.
Test for NULL pointer and return TEST_SKIP if so.

Output before:
[root@p23lp27 perf]# ./perf test -F 39
39: Session topology  :Segmentation fault (core dumped)
[root@p23lp27 perf]#

Output after:
[root@p23lp27 perf]# ./perf test -vF 39
39: Session topology                                      :
--- start ---
templ file: /tmp/perf-test-Ajx59D
socket_id number is too big.You may need to upgrade the perf tool.
---- end ----
Session topology: Skip
[root@p23lp27 perf]#

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
---
 tools/perf/tests/topology.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
index 17cb1bb3448c..5f366d650d2b 100644
--- a/tools/perf/tests/topology.c
+++ b/tools/perf/tests/topology.c
@@ -70,6 +70,9 @@ static int check_cpu_topology(char *path, struct cpu_map *map)
 	session = perf_session__new(&data, false, NULL);
 	TEST_ASSERT_VAL("can't get session", session);
 
+	if (!session->header.env.cpu)
+		return TEST_SKIP;
+
 	for (i = 0; i < session->header.env.nr_cpus_avail; i++) {
 		if (!cpu_map__has(map, i))
 			continue;
@@ -95,7 +98,7 @@ int test__session_topology(struct test *test __maybe_unused, int subtest __maybe
 {
 	char path[PATH_MAX];
 	struct cpu_map *map;
-	int ret = -1;
+	int ret;
 
 	TEST_ASSERT_VAL("can't get templ file", !get_temp(path));
 
@@ -110,12 +113,9 @@ int test__session_topology(struct test *test __maybe_unused, int subtest __maybe
 		goto free_path;
 	}
 
-	if (check_cpu_topology(path, map))
-		goto free_map;
-	ret = 0;
-
-free_map:
+	ret = check_cpu_topology(path, map);
 	cpu_map__put(map);
+
 free_path:
 	unlink(path);
 	return ret;
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf test 39 (Session topology) dumps core on s390
  2018-05-24 13:56 [PATCH] perf test 39 (Session topology) dumps core on s390 Thomas Richter
@ 2018-05-25 15:11 ` Arnaldo Carvalho de Melo
  2018-05-28  3:49 ` Ravi Bangoria
  1 sibling, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-05-25 15:11 UTC (permalink / raw)
  To: Thomas Richter
  Cc: linux-kernel, linux-perf-users, brueckner, schwidefsky, heiko.carstens

Em Thu, May 24, 2018 at 03:56:00PM +0200, Thomas Richter escreveu:
> perf test 39 fails with core dump on s390. The root cause
> is a NULL pointer reference in function check_cpu_topology()
> line 76 (or line 82 without -v).
> The session->header.env.cpu variable is NULL because on s390
> function process_cpu_topology() returns with error:
>     socket_id number is too big.
>     You may need to upgrade the perf tool.
> and releases the env.cpu variable via zfree() and sets it to NULL.
> 
> Here is the gdb output:
> (gdb) n
> 76                      pr_debug("CPU %d, core %d, socket %d\n", i,
> (gdb) n
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000010f4d9e in check_cpu_topology (path=0x3ffffffd6c8
> 	"/tmp/perf-test-J6CHMa", map=0x14a1740) at tests/topology.c:76
> 76  pr_debug("CPU %d, core %d, socket %d\n", i,
> (gdb)
> 
> Make sure the env.cpu variable is not used when its NULL.
> Test for NULL pointer and return TEST_SKIP if so.

Ok, but can you please add a comment just before the added lines:

+     if (!session->header.env.cpu)
+             return TEST_SKIP;
+

Informing that this is so due to some bug, i.e. why is that
process_cpu_topology() returns with that "socked_id is too big" message?

This way people interested in auditing the tests to figure out why some
tests are being skipped can validate that comment and possibly come up
with some fix or better warning message after the "Skip" after that
'perf test' entry?

- Arnaldo
 
> Output before:
> [root@p23lp27 perf]# ./perf test -F 39
> 39: Session topology  :Segmentation fault (core dumped)
> [root@p23lp27 perf]#
> 
> Output after:
> [root@p23lp27 perf]# ./perf test -vF 39
> 39: Session topology                                      :
> --- start ---
> templ file: /tmp/perf-test-Ajx59D
> socket_id number is too big.You may need to upgrade the perf tool.
> ---- end ----
> Session topology: Skip
> [root@p23lp27 perf]#
> 
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
> ---
>  tools/perf/tests/topology.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
> index 17cb1bb3448c..5f366d650d2b 100644
> --- a/tools/perf/tests/topology.c
> +++ b/tools/perf/tests/topology.c
> @@ -70,6 +70,9 @@ static int check_cpu_topology(char *path, struct cpu_map *map)
>  	session = perf_session__new(&data, false, NULL);
>  	TEST_ASSERT_VAL("can't get session", session);
>  
> +	if (!session->header.env.cpu)
> +		return TEST_SKIP;
> +
>  	for (i = 0; i < session->header.env.nr_cpus_avail; i++) {
>  		if (!cpu_map__has(map, i))
>  			continue;
> @@ -95,7 +98,7 @@ int test__session_topology(struct test *test __maybe_unused, int subtest __maybe
>  {
>  	char path[PATH_MAX];
>  	struct cpu_map *map;
> -	int ret = -1;
> +	int ret;
>  
>  	TEST_ASSERT_VAL("can't get templ file", !get_temp(path));
>  
> @@ -110,12 +113,9 @@ int test__session_topology(struct test *test __maybe_unused, int subtest __maybe
>  		goto free_path;
>  	}
>  
> -	if (check_cpu_topology(path, map))
> -		goto free_map;
> -	ret = 0;
> -
> -free_map:
> +	ret = check_cpu_topology(path, map);
>  	cpu_map__put(map);
> +
>  free_path:
>  	unlink(path);
>  	return ret;
> -- 
> 2.14.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf test 39 (Session topology) dumps core on s390
  2018-05-24 13:56 [PATCH] perf test 39 (Session topology) dumps core on s390 Thomas Richter
  2018-05-25 15:11 ` Arnaldo Carvalho de Melo
@ 2018-05-28  3:49 ` Ravi Bangoria
  2018-05-28  6:50   ` Thomas-Mich Richter
  1 sibling, 1 reply; 4+ messages in thread
From: Ravi Bangoria @ 2018-05-28  3:49 UTC (permalink / raw)
  To: Thomas Richter
  Cc: linux-kernel, linux-perf-users, acme, brueckner, schwidefsky,
	heiko.carstens, Ravi Bangoria

Hi Thomas,

On 05/24/2018 07:26 PM, Thomas Richter wrote:
> @@ -95,7 +98,7 @@ int test__session_topology(struct test *test __maybe_unused, int subtest __maybe
>  {
>  	char path[PATH_MAX];
>  	struct cpu_map *map;
> -	int ret = -1;
> +	int ret;

This is failing for me:

tests/topology.c: In function ‘test__session_topology’:
tests/topology.c:101:6: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  int ret;
      ^
cc1: all warnings being treated as errors
  CC       util/intlist.o
mv: cannot stat 'tests/.topology.o.tmp': No such file or directory
/home/ravi/Workspace/linux/tools/build/Makefile.build:96: recipe for target 'tests/topology.o' failed
make[4]: *** [tests/topology.o] Error 1
make[4]: *** Waiting for unfinished jobs....


Thanks,
Ravi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf test 39 (Session topology) dumps core on s390
  2018-05-28  3:49 ` Ravi Bangoria
@ 2018-05-28  6:50   ` Thomas-Mich Richter
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas-Mich Richter @ 2018-05-28  6:50 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: linux-kernel, linux-perf-users, acme, brueckner, schwidefsky,
	heiko.carstens

On 05/28/2018 05:49 AM, Ravi Bangoria wrote:
> Hi Thomas,
> 
> On 05/24/2018 07:26 PM, Thomas Richter wrote:
>> @@ -95,7 +98,7 @@ int test__session_topology(struct test *test __maybe_unused, int subtest __maybe
>>  {
>>  	char path[PATH_MAX];
>>  	struct cpu_map *map;
>> -	int ret = -1;
>> +	int ret;
> 
> This is failing for me:
> 
> tests/topology.c: In function ‘test__session_topology’:
> tests/topology.c:101:6: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   int ret;
>       ^
> cc1: all warnings being treated as errors
>   CC       util/intlist.o
> mv: cannot stat 'tests/.topology.o.tmp': No such file or directory
> /home/ravi/Workspace/linux/tools/build/Makefile.build:96: recipe for target 'tests/topology.o' failed
> make[4]: *** [tests/topology.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> 
> 
> Thanks,
> Ravi
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Thanks for the pointer, will provide V2.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-28  6:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-24 13:56 [PATCH] perf test 39 (Session topology) dumps core on s390 Thomas Richter
2018-05-25 15:11 ` Arnaldo Carvalho de Melo
2018-05-28  3:49 ` Ravi Bangoria
2018-05-28  6:50   ` Thomas-Mich Richter

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.