Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1655120 > unrolled thread

Re: [PATCH] perf test: Disable breakpoint signal tests for powerpc

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] perf test: Disable breakpoint signal tests for powerpc Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-01 15:10 +0200
    Re: [PATCH] perf test: Disable breakpoint signal tests for powerpc Jiri Olsa <jolsa@redhat.com> - 2017-06-01 15:20 +0200
      [PATCHv2] perf test: Disable breakpoint signal tests for powerpc Jiri Olsa <jolsa@redhat.com> - 2017-06-01 23:00 +0200
        Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-02 16:10 +0200
          Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-02 16:10 +0200
        Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-02 16:40 +0200
          Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc Jiri Olsa <jolsa@redhat.com> - 2017-06-04 21:00 +0200
        [tip:perf/urgent] perf test: Disable breakpoint signal tests for  powerpc tip-bot for Jiri Olsa <tipbot@zytor.com> - 2017-06-07 18:10 +0200

#1655120 — Re: [PATCH] perf test: Disable breakpoint signal tests for powerpc

Em Tue, May 16, 2017 at 07:49:42PM +0200, Jiri Olsa escreveu:
> 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/tip-erwshbq6f7b7hdtu65z5a25y@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  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(+)
> 
> diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
> index e7664fe3bd33..0467c3b6d849 100644
> --- a/tools/perf/tests/bp_signal.c
> +++ b/tools/perf/tests/bp_signal.c
> @@ -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

> +{
> +/*
> + * 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
> +}
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 9e08d297f1a9..ddbbff7e4148 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -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)) {
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index 631859629403..1e72f14845c6 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -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;
> -- 
> 2.9.4

[toc] | [next] | [standalone]

#1655134

On Thu, Jun 01, 2017 at 10:04:58AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 16, 2017 at 07:49:42PM +0200, Jiri Olsa escreveu:
> > 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/tip-erwshbq6f7b7hdtu65z5a25y@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  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(+)
> > 
> > diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
> > index e7664fe3bd33..0467c3b6d849 100644
> > --- a/tools/perf/tests/bp_signal.c
> > +++ b/tools/perf/tests/bp_signal.c
> > @@ -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()"?

ok

jirka

[toc] | [prev] | [next] | [standalone]

#1655759 — [PATCHv2] perf test: Disable breakpoint signal tests for powerpc

On Thu, Jun 01, 2017 at 03:10:07PM +0200, Jiri Olsa wrote:
> On Thu, Jun 01, 2017 at 10:04:58AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, May 16, 2017 at 07:49:42PM +0200, Jiri Olsa escreveu:
> > > 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/tip-erwshbq6f7b7hdtu65z5a25y@git.kernel.org
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  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(+)
> > > 
> > > diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
> > > index e7664fe3bd33..0467c3b6d849 100644
> > > --- a/tools/perf/tests/bp_signal.c
> > > +++ b/tools/perf/tests/bp_signal.c
> > > @@ -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/tip-erwshbq6f7b7hdtu65z5a25y@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(+)

diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
index e7664fe3bd33..8ba2c4618fe9 100644
--- a/tools/perf/tests/bp_signal.c
+++ b/tools/perf/tests/bp_signal.c
@@ -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
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 9e08d297f1a9..3ccfd58a8c3c 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -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)) {
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 631859629403..577363809c9b 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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;
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1656257 — Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc

Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:
>  
> +		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

> +			continue;
> +		}
> +
>  		pr_info("%2d: %-*s:", i, width, t->desc);
>  
>  		if (intlist__find(skiplist, i)) {
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index 631859629403..577363809c9b 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -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;
> -- 
> 2.9.4

[toc] | [prev] | [next] | [standalone]

#1656258 — Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc

Em Fri, Jun 02, 2017 at 11:04:33AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:
> >  
> > +		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 ;-)

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
 
> - Arnaldo
> 
> > +			continue;
> > +		}
> > +
> >  		pr_info("%2d: %-*s:", i, width, t->desc);
> >  
> >  		if (intlist__find(skiplist, i)) {
> > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> > index 631859629403..577363809c9b 100644
> > --- a/tools/perf/tests/tests.h
> > +++ b/tools/perf/tests/tests.h
> > @@ -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;
> > -- 
> > 2.9.4

[toc] | [prev] | [next] | [standalone]

#1656304 — Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc

Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:
> > > > +bool test__bp_signal_is_on(void)
> > > 
> > > Can you please rename this "_is_on()" thing to "_supported()"?
> 
> v2 attached,
> jirka

Thanks, applied.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1657135 — Re: [PATCHv2] perf test: Disable breakpoint signal tests for powerpc

On Fri, Jun 02, 2017 at 11:31:12AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 01, 2017 at 10:54:50PM +0200, Jiri Olsa escreveu:
> > > > > +bool test__bp_signal_is_on(void)
> > > > 
> > > > Can you please rename this "_is_on()" thing to "_supported()"?
> > 
> > v2 attached,
> > jirka
> 
> Thanks, applied.

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1659966 — [tip:perf/urgent] perf test: Disable breakpoint signal tests for powerpc

Commit-ID:  598762cf91532e6e86dd21199b5e7f803df73f49
Gitweb:     http://git.kernel.org/tip/598762cf91532e6e86dd21199b5e7f803df73f49
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 1 Jun 2017 22:54:50 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170601205450.GA398@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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(+)

diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
index e7664fe..8ba2c46 100644
--- a/tools/perf/tests/bp_signal.c
+++ b/tools/perf/tests/bp_signal.c
@@ -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
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 9e08d29..3ccfd58 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -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)) {
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 6318596..5773638 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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;

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web