perf test: Disable breakpoint signal tests for powerpc

PATCH In review Jiri Olsa 4 participants 9 messages May 16, 2017 – Jun 7, 2017 perf events

lore

Share & tools

lore

Jiri Olsa

author reviewer

2017-05-16 17:49 UTC #0 lore raw

PATCH 1/1 perf test: Disable breakpoint signal tests for powerpc

Following tests are constantly failing on powerpc:

  # perf test break
  18: Breakpoint overflow signal handler         : FAILED!
  19: Breakpoint overflow sampling               : FAILED!

The powerpc so far does not have support to even create instruction breakpoint using the perf event interface, so those tests fail early in the config phase.

I added 'ison' callback to test struct to be able to disable specific test. It seems better than putting ifdefs directly to the test array.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/tests/bp_signal.c    | 14 ++++++++++++++
 tools/perf/tests/builtin-test.c |  7 +++++++
 tools/perf/tests/tests.h        |  3 +++
 3 files changed, 24 insertions(+)

tools/perf/tests/bp_signal.c +14 -0

index e7664fe3bd33..0467c3b6d849 100644
@@ -288,3 +288,17 @@ int test__bp_signal(int subtest __maybe_unused)
 	return count1 == 1 && overflows == 3 && count2 == 3 && overflows_2 == 3 && count3 == 2 ?
 		TEST_OK : TEST_FAIL;
 }
+
+bool test__bp_signal_is_on(void)
+{
+/*
+ * The powerpc so far does not have support to even create
+ * instruction breakpoint using the perf event interface.
+ * Once it's there we can release this.
+ */
+#ifdef __powerpc__
+	return false;
+#else
+	return true;
+#endif
+}

tools/perf/tests/builtin-test.c +7 -0

index 9e08d297f1a9..ddbbff7e4148 100644
@@ -97,10 +97,12 @@ static struct test generic_tests[] = {
 	{
 		.desc = "Breakpoint overflow signal handler",
 		.func = test__bp_signal,
+		.ison = test__bp_signal_is_on,
 	},
 	{
 		.desc = "Breakpoint overflow sampling",
 		.func = test__bp_signal_overflow,
+		.ison = test__bp_signal_is_on,
 	},
 	{
 		.desc = "Number of exit events of a simple workload",

@@ -401,6 +403,11 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 		if (!perf_test__matches(t, curr, argc, argv))
 			continue;
 
+		if (t->ison && !t->ison()) {
+			pr_debug("%2d: %-*s: Disabled\n", i, width, t->desc);
+			continue;
+		}
+
 		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {

tools/perf/tests/tests.h +3 -0

index 631859629403..1e72f14845c6 100644
@@ -34,6 +34,7 @@ struct test {
 		int (*get_nr)(void);
 		const char *(*get_desc)(int subtest);
 	} subtest;
+	bool (*ison)(void);
 };
 
 /* Tests */

@@ -99,6 +100,8 @@ const char *test__clang_subtest_get_desc(int subtest);
 int test__clang_subtest_get_nr(void);
 int test__unit_number__scnprint(int subtest);
 
+bool test__bp_signal_is_on(void);
+
 #if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 struct thread;

Signature · 1 line

2.9.4

Arnaldo Carvalho de Melo

maintainer

2017-06-01 13:05 UTC #1 lore raw

Em Tue, May 16, 2017 at 07:49:42PM +0200, Jiri Olsa escreveu:

Quoted 27 lines

Following tests are constantly failing on powerpc:
  # perf test break
  18: Breakpoint overflow signal handler         : FAILED!
  19: Breakpoint overflow sampling               : FAILED!
The powerpc so far does not have support to even create instruction breakpoint using the perf event interface, so those tests fail early in the config phase.
I added 'ison' callback to test struct to be able to disable specific test. It seems better than putting ifdefs directly to the test array.
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/tests/bp_signal.c    | 14 ++++++++++++++
 tools/perf/tests/builtin-test.c |  7 +++++++
 tools/perf/tests/tests.h        |  3 +++
 3 files changed, 24 insertions(+)
tools/perf/tests/bp_signal.c +0 -0
index e7664fe3bd33..0467c3b6d849 100644

tools/perf/tests/bp_signal.c int test__bp_signal(int subtest __maybe_unused) · line 292

Quoted 5 lines

tools/perf/tests/bp_signal.c +2 -0

@@ -288,3 +288,17 @@ int test__bp_signal(int subtest __maybe_unused)
 	return count1 == 1 && overflows == 3 && count2 == 3 && overflows_2 == 3 && count3 == 2 ?
 		TEST_OK : TEST_FAIL;
 }
+
+bool test__bp_signal_is_on(void)

Can you please rename this "_is_on()" thing to "_supported()"?

- Arnaldo

Quoted 64 lines

tools/perf/tests/bp_signal.c +12 -0

+{
+/*
+ * The powerpc so far does not have support to even create
+ * instruction breakpoint using the perf event interface.
+ * Once it's there we can release this.
+ */
+#ifdef __powerpc__
+	return false;
+#else
+	return true;
+#endif
+}

tools/perf/tests/builtin-test.c +7 -0

index 9e08d297f1a9..ddbbff7e4148 100644
@@ -97,10 +97,12 @@ static struct test generic_tests[] = {
 	{
 		.desc = "Breakpoint overflow signal handler",
 		.func = test__bp_signal,
+		.ison = test__bp_signal_is_on,
 	},
 	{
 		.desc = "Breakpoint overflow sampling",
 		.func = test__bp_signal_overflow,
+		.ison = test__bp_signal_is_on,
 	},
 	{
 		.desc = "Number of exit events of a simple workload",

@@ -401,6 +403,11 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 		if (!perf_test__matches(t, curr, argc, argv))
 			continue;
 
+		if (t->ison && !t->ison()) {
+			pr_debug("%2d: %-*s: Disabled\n", i, width, t->desc);
+			continue;
+		}
+
 		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {

tools/perf/tests/tests.h +3 -0

index 631859629403..1e72f14845c6 100644
@@ -34,6 +34,7 @@ struct test {
 		int (*get_nr)(void);
 		const char *(*get_desc)(int subtest);
 	} subtest;
+	bool (*ison)(void);
 };
 
 /* Tests */

@@ -99,6 +100,8 @@ const char *test__clang_subtest_get_desc(int subtest);
 int test__clang_subtest_get_nr(void);
 int test__unit_number__scnprint(int subtest);
 
+bool test__bp_signal_is_on(void);
+
 #if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 struct thread;

Signature · 1 line

2.9.4

Jiri Olsa

2017-06-01 13:10 UTC #2 lore raw

Quoted 23 lines · Arnaldo Carvalho de Melo

Em Tue, May 16, 2017 at 07:49:42PM +0200, Jiri Olsa escreveu:
Quoted 22 lines
Following tests are constantly failing on powerpc:
  # perf test break
  18: Breakpoint overflow signal handler         : FAILED!
  19: Breakpoint overflow sampling               : FAILED!
The powerpc so far does not have support to even create instruction breakpoint using the perf event interface, so those tests fail early in the config phase.
I added 'ison' callback to test struct to be able to disable specific test. It seems better than putting ifdefs directly to the test array.
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/tests/bp_signal.c    | 14 ++++++++++++++
 tools/perf/tests/builtin-test.c |  7 +++++++
 tools/perf/tests/tests.h        |  3 +++
 3 files changed, 24 insertions(+)

Quoted 12 lines

tools/perf/tests/bp_signal.c int test__bp_signal(int subtest __maybe_unused) · line 292
Quoted 10 lines
tools/perf/tests/bp_signal.c +2 -0
index e7664fe3bd33..0467c3b6d849 100644
@@ -288,3 +288,17 @@ int test__bp_signal(int subtest __maybe_unused)
 	return count1 == 1 && overflows == 3 && count2 == 3 && overflows_2 == 3 && count3 == 2 ?
 		TEST_OK : TEST_FAIL;
 }
+
+bool test__bp_signal_is_on(void)
Can you please rename this "_is_on()" thing to "_supported()"?

jirka

Jiri Olsa

2017-06-01 20:54 UTC #3 lore raw

PATCH v2 1/1 perf test: Disable breakpoint signal tests for powerpc

Quoted 24 lines · Jiri Olsa

Quoted 23 lines · Arnaldo Carvalho de Melo
Em Tue, May 16, 2017 at 07:49:42PM +0200, Jiri Olsa escreveu:
Quoted 22 lines
Following tests are constantly failing on powerpc:
  # perf test break
  18: Breakpoint overflow signal handler         : FAILED!
  19: Breakpoint overflow sampling               : FAILED!
The powerpc so far does not have support to even create instruction breakpoint using the perf event interface, so those tests fail early in the config phase.
I added 'ison' callback to test struct to be able to disable specific test. It seems better than putting ifdefs directly to the test array.
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/tests/bp_signal.c    | 14 ++++++++++++++
 tools/perf/tests/builtin-test.c |  7 +++++++
 tools/perf/tests/tests.h        |  3 +++
 3 files changed, 24 insertions(+)

Quoted 12 lines

Quoted 4 lines
Quoted 4 lines
tools/perf/tests/bp_signal.c +0 -0
index e7664fe3bd33..0467c3b6d849 100644
Quoted 8 lines
tools/perf/tests/bp_signal.c int test__bp_signal(int subtest __maybe_unused) · line 292
Quoted 6 lines
tools/perf/tests/bp_signal.c +2 -0
@@ -288,3 +288,17 @@ int test__bp_signal(int subtest __maybe_unused)
 	return count1 == 1 && overflows == 3 && count2 == 3 && overflows_2 == 3 && count3 == 2 ?
 		TEST_OK : TEST_FAIL;
 }
+
+bool test__bp_signal_is_on(void)
Can you please rename this "_is_on()" thing to "_supported()"?

v2 attached,
jirka

---
Following tests are constantly failing on powerpc:

  # perf test break
  18: Breakpoint overflow signal handler         : FAILED!
  19: Breakpoint overflow sampling               : FAILED!

The powerpc so far does not have support to even create instruction breakpoint using the perf event interface, so those tests fail early in the config phase.

I added 'is_supported' callback to test struct to be able to disable specific test. It seems better than putting ifdefs directly to the test array.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/tests/bp_signal.c    | 14 ++++++++++++++
 tools/perf/tests/builtin-test.c |  7 +++++++
 tools/perf/tests/tests.h        |  3 +++
 3 files changed, 24 insertions(+)

tools/perf/tests/bp_signal.c +14 -0

index e7664fe3bd33..8ba2c4618fe9 100644
@@ -288,3 +288,17 @@ int test__bp_signal(int subtest __maybe_unused)
 	return count1 == 1 && overflows == 3 && count2 == 3 && overflows_2 == 3 && count3 == 2 ?
 		TEST_OK : TEST_FAIL;
 }
+
+bool test__bp_signal_is_supported(void)
+{
+/*
+ * The powerpc so far does not have support to even create
+ * instruction breakpoint using the perf event interface.
+ * Once it's there we can release this.
+ */
+#ifdef __powerpc__
+	return false;
+#else
+	return true;
+#endif
+}

tools/perf/tests/builtin-test.c +7 -0

index 9e08d297f1a9..3ccfd58a8c3c 100644
@@ -97,10 +97,12 @@ static struct test generic_tests[] = {
 	{
 		.desc = "Breakpoint overflow signal handler",
 		.func = test__bp_signal,
+		.is_supported = test__bp_signal_is_supported,
 	},
 	{
 		.desc = "Breakpoint overflow sampling",
 		.func = test__bp_signal_overflow,
+		.is_supported = test__bp_signal_is_supported,
 	},
 	{
 		.desc = "Number of exit events of a simple workload",

@@ -401,6 +403,11 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 		if (!perf_test__matches(t, curr, argc, argv))
 			continue;
 
+		if (t->is_supported && !t->is_supported()) {
+			pr_debug("%2d: %-*s: Disabled\n", i, width, t->desc);
+			continue;
+		}
+
 		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {

tools/perf/tests/tests.h +3 -0

index 631859629403..577363809c9b 100644
@@ -34,6 +34,7 @@ struct test {
 		int (*get_nr)(void);
 		const char *(*get_desc)(int subtest);
 	} subtest;
+	bool (*is_supported)(void);
 };
 
 /* Tests */

@@ -99,6 +100,8 @@ const char *test__clang_subtest_get_desc(int subtest);
 int test__clang_subtest_get_nr(void);
 int test__unit_number__scnprint(int subtest);
 
+bool test__bp_signal_is_supported(void);
+
 #if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 struct thread;

Signature · 1 line

2.9.4

Arnaldo Carvalho de Melo

maintainer

2017-06-02 14:04 UTC #4 lore raw

Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:

tools/perf/tests/builtin-test.c static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist) · line 407

Quoted 3 lines

tools/perf/tests/builtin-test.c +2 -0

+		if (t->is_supported && !t->is_supported()) {
+			pr_debug("%2d: %-*s: Disabled\n", i, width, t->desc);

This isn't clear, I'll change this to leave the pr_debug up to the ->is_supported() code (that will receive the struct test pointer too), that can then say something more clear like what you put in that comment.

This way, ppc hackers running 'perf test' will see that longstanding TODO entry ;-)

- Arnaldo

Quoted 29 lines

tools/perf/tests/builtin-test.c +3 -0

+			continue;
+		}
+
 		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {

tools/perf/tests/tests.h +3 -0

index 631859629403..577363809c9b 100644
@@ -34,6 +34,7 @@ struct test {
 		int (*get_nr)(void);
 		const char *(*get_desc)(int subtest);
 	} subtest;
+	bool (*is_supported)(void);
 };
 
 /* Tests */

@@ -99,6 +100,8 @@ const char *test__clang_subtest_get_desc(int subtest);
 int test__clang_subtest_get_nr(void);
 int test__unit_number__scnprint(int subtest);
 
+bool test__bp_signal_is_supported(void);
+
 #if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 struct thread;

Signature · 1 line

2.9.4

Arnaldo Carvalho de Melo

maintainer

2017-06-02 14:05 UTC #5 lore raw

Em Fri, Jun 02, 2017 at 11:04:33AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 10 lines

Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:
tools/perf/tests/builtin-test.c static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist) · line 407
Quoted 3 lines
tools/perf/tests/builtin-test.c +2 -0
+		if (t->is_supported && !t->is_supported()) {
+			pr_debug("%2d: %-*s: Disabled\n", i, width, t->desc);
This isn't clear, I'll change this to leave the pr_debug up to the ->is_supported() code (that will receive the struct test pointer too), that can then say something more clear like what you put in that comment.

Quoted 2 lines

This way, ppc hackers running 'perf test' will see that longstanding TODO entry ;-)

On a second thought, this can be done as a patch on top of yours, done in perf/core, while your patch can go in perf/urgent.

- Arnaldo

Quoted 31 lines

- Arnaldo

Quoted 29 lines

tools/perf/tests/builtin-test.c +3 -0

+			continue;
+		}
+
 		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {

tools/perf/tests/tests.h +3 -0

index 631859629403..577363809c9b 100644
@@ -34,6 +34,7 @@ struct test {
 		int (*get_nr)(void);
 		const char *(*get_desc)(int subtest);
 	} subtest;
+	bool (*is_supported)(void);
 };
 
 /* Tests */

@@ -99,6 +100,8 @@ const char *test__clang_subtest_get_desc(int subtest);
 int test__clang_subtest_get_nr(void);
 int test__unit_number__scnprint(int subtest);
 
+bool test__bp_signal_is_supported(void);
+
 #if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 struct thread;

Signature · 1 line

2.9.4

Arnaldo Carvalho de Melo

maintainer Applied

2017-06-02 14:31 UTC #6 lore raw

In reply to Jiri Olsa

Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:

Quoted 6 lines

Quoted 3 lines
Quoted 3 lines
Quoted 1 line
+bool test__bp_signal_is_on(void)
Can you please rename this "_is_on()" thing to "_supported()"?
v2 attached,
jirka

Thanks, applied.

- Arnaldo

Jiri Olsa

2017-06-04 18:55 UTC #7 lore raw

Quoted 9 lines · Arnaldo Carvalho de Melo

Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
+bool test__bp_signal_is_on(void)
Can you please rename this "_is_on()" thing to "_supported()"?
v2 attached,
jirka
Thanks, applied.

thanks,
jirka

tip-bot for Jiri Olsa

merge bot

2017-06-07 16:02 UTC #8 lore raw

In reply to Jiri Olsa

perf test: Disable breakpoint signal tests for powerpc

Commit-ID:  598762cf91532e6e86dd21199b5e7f803df73f49
Gitweb:     http://git.kernel.org/tip/598762cf91532e6e86dd21199b5e7f803df73f49
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 1 Jun 2017 22:54:50 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 5 Jun 2017 14:18:01 -0300

perf test: Disable breakpoint signal tests for powerpc

The following tests are failing on powerpc:

  # perf test break
  18: Breakpoint overflow signal handler  : FAILED!
  19: Breakpoint overflow sampling        : FAILED!

The powerpc kenel so far does not have support to even create instruction breakpoints using the perf event interface, so those tests fail early in the config phase.

I added a '->is_supported()' callback to test struct to be able to disable specific tests. It seems better than putting ifdefs directly to the test array.

Signed-off-by Jiri Olsa <[email protected]>

Cc David Ahern <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/20170601205450.GA398@krava open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/tests/bp_signal.c    | 14 ++++++++++++++
 tools/perf/tests/builtin-test.c |  7 +++++++
 tools/perf/tests/tests.h        |  3 +++
 3 files changed, 24 insertions(+)

tools/perf/tests/bp_signal.c +14 -0

index e7664fe..8ba2c46 100644
@@ -288,3 +288,17 @@ int test__bp_signal(int subtest __maybe_unused)
 	return count1 == 1 && overflows == 3 && count2 == 3 && overflows_2 == 3 && count3 == 2 ?
 		TEST_OK : TEST_FAIL;
 }
+
+bool test__bp_signal_is_supported(void)
+{
+/*
+ * The powerpc so far does not have support to even create
+ * instruction breakpoint using the perf event interface.
+ * Once it's there we can release this.
+ */
+#ifdef __powerpc__
+	return false;
+#else
+	return true;
+#endif
+}

tools/perf/tests/builtin-test.c +7 -0

index 9e08d29..3ccfd58 100644
@@ -97,10 +97,12 @@ static struct test generic_tests[] = {
 	{
 		.desc = "Breakpoint overflow signal handler",
 		.func = test__bp_signal,
+		.is_supported = test__bp_signal_is_supported,
 	},
 	{
 		.desc = "Breakpoint overflow sampling",
 		.func = test__bp_signal_overflow,
+		.is_supported = test__bp_signal_is_supported,
 	},
 	{
 		.desc = "Number of exit events of a simple workload",

@@ -401,6 +403,11 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 		if (!perf_test__matches(t, curr, argc, argv))
 			continue;
 
+		if (t->is_supported && !t->is_supported()) {
+			pr_debug("%2d: %-*s: Disabled\n", i, width, t->desc);
+			continue;
+		}
+
 		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {

tools/perf/tests/tests.h +3 -0

index 6318596..5773638 100644
@@ -34,6 +34,7 @@ struct test {
 		int (*get_nr)(void);
 		const char *(*get_desc)(int subtest);
 	} subtest;
+	bool (*is_supported)(void);
 };
 
 /* Tests */

@@ -99,6 +100,8 @@ const char *test__clang_subtest_get_desc(int subtest);
 int test__clang_subtest_get_nr(void);
 int test__unit_number__scnprint(int subtest);
 
+bool test__bp_signal_is_supported(void);
+
 #if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 struct thread;