2020-10-16 13:26:15

by Tommi Rantala

Subject: [PATCH] perf test: Implement skip_reason callback for watchpoint tests

Currently reason for skipping the read only watchpoint test is only seen
when running in verbose mode:

$ perf test watchpoint
23: Watchpoint :
23.1: Read Only Watchpoint : Skip
23.2: Write Only Watchpoint : Ok
23.3: Read / Write Watchpoint : Ok
23.4: Modify Watchpoint : Ok

$ perf test -v watchpoint
23: Watchpoint :
23.1: Read Only Watchpoint :
--- start ---
test child forked, pid 60204
Hardware does not support read only watchpoints.
test child finished with -2

Implement skip_reason callback for the watchpoint tests, so that it's
easy to see reason why the test is skipped:

$ perf test watchpoint
23: Watchpoint :
23.1: Read Only Watchpoint : Skip (missing hardware support)
23.2: Write Only Watchpoint : Ok
23.3: Read / Write Watchpoint : Ok
23.4: Modify Watchpoint : Ok

Signed-off-by: Tommi Rantala <[email protected]>
---
tools/perf/tests/builtin-test.c | 1 +
tools/perf/tests/tests.h | 1 +
tools/perf/tests/wp.c | 21 +++++++++++++++------
3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index d328caaba45d..3bfad4ee31ae 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -142,6 +142,7 @@ static struct test generic_tests[] = {
.skip_if_fail = false,
.get_nr = test__wp_subtest_get_nr,
.get_desc = test__wp_subtest_get_desc,
+ .skip_reason = test__wp_subtest_skip_reason,
},
},
{
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 4447a516c689..0630301087a6 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -66,6 +66,7 @@ int test__bp_signal_overflow(struct test *test, int subtest);
int test__bp_accounting(struct test *test, int subtest);
int test__wp(struct test *test, int subtest);
const char *test__wp_subtest_get_desc(int subtest);
+const char *test__wp_subtest_skip_reason(int subtest);
int test__wp_subtest_get_nr(void);
int test__task_exit(struct test *test, int subtest);
int test__mem(struct test *test, int subtest);
diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
index d262d6639829..9387fa76faa5 100644
--- a/tools/perf/tests/wp.c
+++ b/tools/perf/tests/wp.c
@@ -174,10 +174,12 @@ static bool wp_ro_supported(void)
#endif
}

-static void wp_ro_skip_msg(void)
+static const char *wp_ro_skip_msg(void)
{
#if defined (__x86_64__) || defined (__i386__)
- pr_debug("Hardware does not support read only watchpoints.\n");
+ return "missing hardware support";
+#else
+ return NULL;
#endif
}

@@ -185,7 +187,7 @@ static struct {
const char *desc;
int (*target_func)(void);
bool (*is_supported)(void);
- void (*skip_msg)(void);
+ const char *(*skip_msg)(void);
} wp_testcase_table[] = {
{
.desc = "Read Only Watchpoint",
@@ -219,16 +221,23 @@ const char *test__wp_subtest_get_desc(int i)
return wp_testcase_table[i].desc;
}

+const char *test__wp_subtest_skip_reason(int i)
+{
+ if (i < 0 || i >= (int)ARRAY_SIZE(wp_testcase_table))
+ return NULL;
+ if (!wp_testcase_table[i].skip_msg)
+ return NULL;
+ return wp_testcase_table[i].skip_msg();
+}
+
int test__wp(struct test *test __maybe_unused, int i)
{
if (i < 0 || i >= (int)ARRAY_SIZE(wp_testcase_table))
return TEST_FAIL;

if (wp_testcase_table[i].is_supported &&
- !wp_testcase_table[i].is_supported()) {
- wp_testcase_table[i].skip_msg();
+ !wp_testcase_table[i].is_supported())
return TEST_SKIP;
- }

return !wp_testcase_table[i].target_func() ? TEST_OK : TEST_FAIL;
}
--
2.26.2

2020-10-20 12:20:49

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH] perf test: Implement skip_reason callback for watchpoint tests

Em Tue, Oct 20, 2020 at 03:07:15PM +0900, Namhyung Kim escreveu:
> Hello,
> On Fri, Oct 16, 2020 at 10:17 PM Tommi Rantala
> <[email protected]> wrote:
> >
> > Currently reason for skipping the read only watchpoint test is only seen
> > when running in verbose mode:
> >
> > $ perf test watchpoint
> > 23: Watchpoint :
> > 23.1: Read Only Watchpoint : Skip
> > 23.2: Write Only Watchpoint : Ok
> > 23.3: Read / Write Watchpoint : Ok
> > 23.4: Modify Watchpoint : Ok
> >
> > $ perf test -v watchpoint
> > 23: Watchpoint :
> > 23.1: Read Only Watchpoint :
> > --- start ---
> > test child forked, pid 60204
> > Hardware does not support read only watchpoints.
> > test child finished with -2
> >
> > Implement skip_reason callback for the watchpoint tests, so that it's
> > easy to see reason why the test is skipped:
> >
> > $ perf test watchpoint
> > 23: Watchpoint :
> > 23.1: Read Only Watchpoint : Skip (missing hardware support)
> > 23.2: Write Only Watchpoint : Ok
> > 23.3: Read / Write Watchpoint : Ok
> > 23.4: Modify Watchpoint : Ok
> >
> > Signed-off-by: Tommi Rantala <[email protected]>
>
> Acked-by: Namhyung Kim <[email protected]>

Thanks, applied.

- Arnaldo

>
> Thanks
> Namhyung
>
>
> > ---
> > tools/perf/tests/builtin-test.c | 1 +
> > tools/perf/tests/tests.h | 1 +
> > tools/perf/tests/wp.c | 21 +++++++++++++++------
> > 3 files changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index d328caaba45d..3bfad4ee31ae 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -142,6 +142,7 @@ static struct test generic_tests[] = {
> > .skip_if_fail = false,
> > .get_nr = test__wp_subtest_get_nr,
> > .get_desc = test__wp_subtest_get_desc,
> > + .skip_reason = test__wp_subtest_skip_reason,
> > },
> > },
> > {
> > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> > index 4447a516c689..0630301087a6 100644
> > --- a/tools/perf/tests/tests.h
> > +++ b/tools/perf/tests/tests.h
> > @@ -66,6 +66,7 @@ int test__bp_signal_overflow(struct test *test, int subtest);
> > int test__bp_accounting(struct test *test, int subtest);
> > int test__wp(struct test *test, int subtest);
> > const char *test__wp_subtest_get_desc(int subtest);
> > +const char *test__wp_subtest_skip_reason(int subtest);
> > int test__wp_subtest_get_nr(void);
> > int test__task_exit(struct test *test, int subtest);
> > int test__mem(struct test *test, int subtest);
> > diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
> > index d262d6639829..9387fa76faa5 100644
> > --- a/tools/perf/tests/wp.c
> > +++ b/tools/perf/tests/wp.c
> > @@ -174,10 +174,12 @@ static bool wp_ro_supported(void)
> > #endif
> > }
> >
> > -static void wp_ro_skip_msg(void)
> > +static const char *wp_ro_skip_msg(void)
> > {
> > #if defined (__x86_64__) || defined (__i386__)
> > - pr_debug("Hardware does not support read only watchpoints.\n");
> > + return "missing hardware support";
> > +#else
> > + return NULL;
> > #endif
> > }
> >
> > @@ -185,7 +187,7 @@ static struct {
> > const char *desc;
> > int (*target_func)(void);
> > bool (*is_supported)(void);
> > - void (*skip_msg)(void);
> > + const char *(*skip_msg)(void);
> > } wp_testcase_table[] = {
> > {
> > .desc = "Read Only Watchpoint",
> > @@ -219,16 +221,23 @@ const char *test__wp_subtest_get_desc(int i)
> > return wp_testcase_table[i].desc;
> > }
> >
> > +const char *test__wp_subtest_skip_reason(int i)
> > +{
> > + if (i < 0 || i >= (int)ARRAY_SIZE(wp_testcase_table))
> > + return NULL;
> > + if (!wp_testcase_table[i].skip_msg)
> > + return NULL;
> > + return wp_testcase_table[i].skip_msg();
> > +}
> > +
> > int test__wp(struct test *test __maybe_unused, int i)
> > {
> > if (i < 0 || i >= (int)ARRAY_SIZE(wp_testcase_table))
> > return TEST_FAIL;
> >
> > if (wp_testcase_table[i].is_supported &&
> > - !wp_testcase_table[i].is_supported()) {
> > - wp_testcase_table[i].skip_msg();
> > + !wp_testcase_table[i].is_supported())
> > return TEST_SKIP;
> > - }
> >
> > return !wp_testcase_table[i].target_func() ? TEST_OK : TEST_FAIL;
> > }
> > --
> > 2.26.2
> >

--

- Arnaldo

2020-10-20 19:20:52

by Namhyung Kim

Subject: Re: [PATCH] perf test: Implement skip_reason callback for watchpoint tests

Hello,

On Fri, Oct 16, 2020 at 10:17 PM Tommi Rantala
<[email protected]> wrote:
>
> Currently reason for skipping the read only watchpoint test is only seen
> when running in verbose mode:
>
> $ perf test watchpoint
> 23: Watchpoint :
> 23.1: Read Only Watchpoint : Skip
> 23.2: Write Only Watchpoint : Ok
> 23.3: Read / Write Watchpoint : Ok
> 23.4: Modify Watchpoint : Ok
>
> $ perf test -v watchpoint
> 23: Watchpoint :
> 23.1: Read Only Watchpoint :
> --- start ---
> test child forked, pid 60204
> Hardware does not support read only watchpoints.
> test child finished with -2
>
> Implement skip_reason callback for the watchpoint tests, so that it's
> easy to see reason why the test is skipped:
>
> $ perf test watchpoint
> 23: Watchpoint :
> 23.1: Read Only Watchpoint : Skip (missing hardware support)
> 23.2: Write Only Watchpoint : Ok
> 23.3: Read / Write Watchpoint : Ok
> 23.4: Modify Watchpoint : Ok
>
> Signed-off-by: Tommi Rantala <[email protected]>

Acked-by: Namhyung Kim <[email protected]>

Thanks
Namhyung


> ---
> tools/perf/tests/builtin-test.c | 1 +
> tools/perf/tests/tests.h | 1 +
> tools/perf/tests/wp.c | 21 +++++++++++++++------
> 3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index d328caaba45d..3bfad4ee31ae 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -142,6 +142,7 @@ static struct test generic_tests[] = {
> .skip_if_fail = false,
> .get_nr = test__wp_subtest_get_nr,
> .get_desc = test__wp_subtest_get_desc,
> + .skip_reason = test__wp_subtest_skip_reason,
> },
> },
> {
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index 4447a516c689..0630301087a6 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -66,6 +66,7 @@ int test__bp_signal_overflow(struct test *test, int subtest);
> int test__bp_accounting(struct test *test, int subtest);
> int test__wp(struct test *test, int subtest);
> const char *test__wp_subtest_get_desc(int subtest);
> +const char *test__wp_subtest_skip_reason(int subtest);
> int test__wp_subtest_get_nr(void);
> int test__task_exit(struct test *test, int subtest);
> int test__mem(struct test *test, int subtest);
> diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
> index d262d6639829..9387fa76faa5 100644
> --- a/tools/perf/tests/wp.c
> +++ b/tools/perf/tests/wp.c
> @@ -174,10 +174,12 @@ static bool wp_ro_supported(void)
> #endif
> }
>
> -static void wp_ro_skip_msg(void)
> +static const char *wp_ro_skip_msg(void)
> {
> #if defined (__x86_64__) || defined (__i386__)
> - pr_debug("Hardware does not support read only watchpoints.\n");
> + return "missing hardware support";
> +#else
> + return NULL;
> #endif
> }
>
> @@ -185,7 +187,7 @@ static struct {
> const char *desc;
> int (*target_func)(void);
> bool (*is_supported)(void);
> - void (*skip_msg)(void);
> + const char *(*skip_msg)(void);
> } wp_testcase_table[] = {
> {
> .desc = "Read Only Watchpoint",
> @@ -219,16 +221,23 @@ const char *test__wp_subtest_get_desc(int i)
> return wp_testcase_table[i].desc;
> }
>
> +const char *test__wp_subtest_skip_reason(int i)
> +{
> + if (i < 0 || i >= (int)ARRAY_SIZE(wp_testcase_table))
> + return NULL;
> + if (!wp_testcase_table[i].skip_msg)
> + return NULL;
> + return wp_testcase_table[i].skip_msg();
> +}
> +
> int test__wp(struct test *test __maybe_unused, int i)
> {
> if (i < 0 || i >= (int)ARRAY_SIZE(wp_testcase_table))
> return TEST_FAIL;
>
> if (wp_testcase_table[i].is_supported &&
> - !wp_testcase_table[i].is_supported()) {
> - wp_testcase_table[i].skip_msg();
> + !wp_testcase_table[i].is_supported())
> return TEST_SKIP;
> - }
>
> return !wp_testcase_table[i].target_func() ? TEST_OK : TEST_FAIL;
> }
> --
> 2.26.2
>