* [PATCH] perf test: Skip watchpoint tests if no watchpoints available
@ 2022-11-21 10:27 Naveen N. Rao
2022-11-22 19:18 ` Christophe Leroy
2022-11-23 8:05 ` kajoljain
0 siblings, 2 replies; 5+ messages in thread
From: Naveen N. Rao @ 2022-11-21 10:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Disha Goel, Ravi Bangoria, linux-perf-users, linux-kernel,
linuxppc-dev
On IBM Power9, perf watchpoint tests fail since no hardware breakpoints
are available. Detect this by checking the error returned by
perf_event_open() and skip the tests in that case.
Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
tools/perf/tests/wp.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
index 56455da30341b4..cc8719609b19ea 100644
--- a/tools/perf/tests/wp.c
+++ b/tools/perf/tests/wp.c
@@ -59,8 +59,10 @@ static int __event(int wp_type, void *wp_addr, unsigned long wp_len)
get__perf_event_attr(&attr, wp_type, wp_addr, wp_len);
fd = sys_perf_event_open(&attr, 0, -1, -1,
perf_event_open_cloexec_flag());
- if (fd < 0)
+ if (fd < 0) {
+ fd = -errno;
pr_debug("failed opening event %x\n", attr.bp_type);
+ }
return fd;
}
@@ -77,7 +79,7 @@ static int test__wp_ro(struct test_suite *test __maybe_unused,
fd = __event(HW_BREAKPOINT_R, (void *)&data1, sizeof(data1));
if (fd < 0)
- return -1;
+ return fd == -ENODEV ? TEST_SKIP : -1;
tmp = data1;
WP_TEST_ASSERT_VAL(fd, "RO watchpoint", 1);
@@ -101,7 +103,7 @@ static int test__wp_wo(struct test_suite *test __maybe_unused,
fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
if (fd < 0)
- return -1;
+ return fd == -ENODEV ? TEST_SKIP : -1;
tmp = data1;
WP_TEST_ASSERT_VAL(fd, "WO watchpoint", 0);
@@ -126,7 +128,7 @@ static int test__wp_rw(struct test_suite *test __maybe_unused,
fd = __event(HW_BREAKPOINT_R | HW_BREAKPOINT_W, (void *)&data1,
sizeof(data1));
if (fd < 0)
- return -1;
+ return fd == -ENODEV ? TEST_SKIP : -1;
tmp = data1;
WP_TEST_ASSERT_VAL(fd, "RW watchpoint", 1);
@@ -150,7 +152,7 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
if (fd < 0)
- return -1;
+ return fd == -ENODEV ? TEST_SKIP : -1;
data1 = tmp;
WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1);
base-commit: 63a3bf5e8d9e79ce456c8f73d4395a5a51d841b1
--
2.38.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] perf test: Skip watchpoint tests if no watchpoints available
2022-11-21 10:27 [PATCH] perf test: Skip watchpoint tests if no watchpoints available Naveen N. Rao
@ 2022-11-22 19:18 ` Christophe Leroy
2022-11-22 20:57 ` Ian Rogers
2022-11-23 8:05 ` kajoljain
1 sibling, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2022-11-22 19:18 UTC (permalink / raw)
To: Naveen N. Rao, Arnaldo Carvalho de Melo
Cc: Ravi Bangoria, linux-perf-users@vger.kernel.org, Disha Goel,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Le 21/11/2022 à 11:27, Naveen N. Rao a écrit :
> On IBM Power9, perf watchpoint tests fail since no hardware breakpoints
> are available. Detect this by checking the error returned by
> perf_event_open() and skip the tests in that case.
>
> Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> tools/perf/tests/wp.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
> index 56455da30341b4..cc8719609b19ea 100644
> --- a/tools/perf/tests/wp.c
> +++ b/tools/perf/tests/wp.c
> @@ -59,8 +59,10 @@ static int __event(int wp_type, void *wp_addr, unsigned long wp_len)
> get__perf_event_attr(&attr, wp_type, wp_addr, wp_len);
> fd = sys_perf_event_open(&attr, 0, -1, -1,
> perf_event_open_cloexec_flag());
> - if (fd < 0)
> + if (fd < 0) {
> + fd = -errno;
> pr_debug("failed opening event %x\n", attr.bp_type);
> + }
Do you really need that ?
Can't you directly check errno in the caller ?
>
> return fd;
> }
> @@ -77,7 +79,7 @@ static int test__wp_ro(struct test_suite *test __maybe_unused,
>
> fd = __event(HW_BREAKPOINT_R, (void *)&data1, sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> tmp = data1;
> WP_TEST_ASSERT_VAL(fd, "RO watchpoint", 1);
> @@ -101,7 +103,7 @@ static int test__wp_wo(struct test_suite *test __maybe_unused,
>
> fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> tmp = data1;
> WP_TEST_ASSERT_VAL(fd, "WO watchpoint", 0);
> @@ -126,7 +128,7 @@ static int test__wp_rw(struct test_suite *test __maybe_unused,
> fd = __event(HW_BREAKPOINT_R | HW_BREAKPOINT_W, (void *)&data1,
> sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> tmp = data1;
> WP_TEST_ASSERT_VAL(fd, "RW watchpoint", 1);
> @@ -150,7 +152,7 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
>
> fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> data1 = tmp;
> WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1);
>
> base-commit: 63a3bf5e8d9e79ce456c8f73d4395a5a51d841b1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf test: Skip watchpoint tests if no watchpoints available
2022-11-22 19:18 ` Christophe Leroy
@ 2022-11-22 20:57 ` Ian Rogers
2022-11-23 13:33 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2022-11-22 20:57 UTC (permalink / raw)
To: Christophe Leroy
Cc: Naveen N. Rao, Arnaldo Carvalho de Melo, Ravi Bangoria,
linux-perf-users@vger.kernel.org, Disha Goel,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
On Tue, Nov 22, 2022 at 11:19 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 21/11/2022 à 11:27, Naveen N. Rao a écrit :
> > On IBM Power9, perf watchpoint tests fail since no hardware breakpoints
> > are available. Detect this by checking the error returned by
> > perf_event_open() and skip the tests in that case.
> >
> > Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> > tools/perf/tests/wp.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
> > index 56455da30341b4..cc8719609b19ea 100644
> > --- a/tools/perf/tests/wp.c
> > +++ b/tools/perf/tests/wp.c
> > @@ -59,8 +59,10 @@ static int __event(int wp_type, void *wp_addr, unsigned long wp_len)
> > get__perf_event_attr(&attr, wp_type, wp_addr, wp_len);
> > fd = sys_perf_event_open(&attr, 0, -1, -1,
> > perf_event_open_cloexec_flag());
> > - if (fd < 0)
> > + if (fd < 0) {
> > + fd = -errno;
> > pr_debug("failed opening event %x\n", attr.bp_type);
> > + }
>
> Do you really need that ?
>
> Can't you directly check errno in the caller ?
errno is very easily clobbered and not clearly set on success - ie,
it'd be better not to do that.
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> >
> > return fd;
> > }
> > @@ -77,7 +79,7 @@ static int test__wp_ro(struct test_suite *test __maybe_unused,
> >
> > fd = __event(HW_BREAKPOINT_R, (void *)&data1, sizeof(data1));
> > if (fd < 0)
> > - return -1;
> > + return fd == -ENODEV ? TEST_SKIP : -1;
> >
> > tmp = data1;
> > WP_TEST_ASSERT_VAL(fd, "RO watchpoint", 1);
> > @@ -101,7 +103,7 @@ static int test__wp_wo(struct test_suite *test __maybe_unused,
> >
> > fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
> > if (fd < 0)
> > - return -1;
> > + return fd == -ENODEV ? TEST_SKIP : -1;
> >
> > tmp = data1;
> > WP_TEST_ASSERT_VAL(fd, "WO watchpoint", 0);
> > @@ -126,7 +128,7 @@ static int test__wp_rw(struct test_suite *test __maybe_unused,
> > fd = __event(HW_BREAKPOINT_R | HW_BREAKPOINT_W, (void *)&data1,
> > sizeof(data1));
> > if (fd < 0)
> > - return -1;
> > + return fd == -ENODEV ? TEST_SKIP : -1;
> >
> > tmp = data1;
> > WP_TEST_ASSERT_VAL(fd, "RW watchpoint", 1);
> > @@ -150,7 +152,7 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
> >
> > fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
> > if (fd < 0)
> > - return -1;
> > + return fd == -ENODEV ? TEST_SKIP : -1;
> >
> > data1 = tmp;
> > WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1);
> >
> > base-commit: 63a3bf5e8d9e79ce456c8f73d4395a5a51d841b1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf test: Skip watchpoint tests if no watchpoints available
2022-11-22 20:57 ` Ian Rogers
@ 2022-11-23 13:33 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-11-23 13:33 UTC (permalink / raw)
To: Ian Rogers
Cc: Christophe Leroy, Naveen N. Rao, Ravi Bangoria,
linux-perf-users@vger.kernel.org, Disha Goel,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Em Tue, Nov 22, 2022 at 12:57:05PM -0800, Ian Rogers escreveu:
> On Tue, Nov 22, 2022 at 11:19 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
> >
> >
> >
> > Le 21/11/2022 à 11:27, Naveen N. Rao a écrit :
> > > On IBM Power9, perf watchpoint tests fail since no hardware breakpoints
> > > are available. Detect this by checking the error returned by
> > > perf_event_open() and skip the tests in that case.
> > >
> > > Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
> > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > > ---
> > > tools/perf/tests/wp.c | 12 +++++++-----
> > > 1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
> > > index 56455da30341b4..cc8719609b19ea 100644
> > > --- a/tools/perf/tests/wp.c
> > > +++ b/tools/perf/tests/wp.c
> > > @@ -59,8 +59,10 @@ static int __event(int wp_type, void *wp_addr, unsigned long wp_len)
> > > get__perf_event_attr(&attr, wp_type, wp_addr, wp_len);
> > > fd = sys_perf_event_open(&attr, 0, -1, -1,
> > > perf_event_open_cloexec_flag());
> > > - if (fd < 0)
> > > + if (fd < 0) {
> > > + fd = -errno;
> > > pr_debug("failed opening event %x\n", attr.bp_type);
> > > + }
> >
> > Do you really need that ?
> >
> > Can't you directly check errno in the caller ?
>
> errno is very easily clobbered and not clearly set on success - ie,
> it'd be better not to do that.
>
> Acked-by: Ian Rogers <irogers@google.com>
Thanks, applied.
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf test: Skip watchpoint tests if no watchpoints available
2022-11-21 10:27 [PATCH] perf test: Skip watchpoint tests if no watchpoints available Naveen N. Rao
2022-11-22 19:18 ` Christophe Leroy
@ 2022-11-23 8:05 ` kajoljain
1 sibling, 0 replies; 5+ messages in thread
From: kajoljain @ 2022-11-23 8:05 UTC (permalink / raw)
To: Naveen N. Rao, Arnaldo Carvalho de Melo
Cc: Disha Goel, Ravi Bangoria, linux-perf-users, linux-kernel,
linuxppc-dev
On 11/21/22 15:57, Naveen N. Rao wrote:
> On IBM Power9, perf watchpoint tests fail since no hardware breakpoints
> are available. Detect this by checking the error returned by
> perf_event_open() and skip the tests in that case.
>
> Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
Patch looks fine to me. Also tested it in power9 box
Test result without this patch changes :
23: Watchpoint :
23.1: Read Only Watchpoint : FAILED!
23.2: Write Only Watchpoint : FAILED!
23.3: Read / Write Watchpoint : FAILED!
23.4: Modify Watchpoint : FAILED!
Test result with patch changes:
23: Watchpoint :
23.1: Read Only Watchpoint : Skip (missing hardware support)
23.2: Write Only Watchpoint : Skip (missing hardware support)
23.3: Read / Write Watchpoint : Skip (missing hardware support)
23.4: Modify Watchpoint : Skip (missing hardware support)
Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
Tested-by: Kajol Jain<kjain@linux.ibm.com>
Thanks,
Kajol Jain
> tools/perf/tests/wp.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c
> index 56455da30341b4..cc8719609b19ea 100644
> --- a/tools/perf/tests/wp.c
> +++ b/tools/perf/tests/wp.c
> @@ -59,8 +59,10 @@ static int __event(int wp_type, void *wp_addr, unsigned long wp_len)
> get__perf_event_attr(&attr, wp_type, wp_addr, wp_len);
> fd = sys_perf_event_open(&attr, 0, -1, -1,
> perf_event_open_cloexec_flag());
> - if (fd < 0)
> + if (fd < 0) {
> + fd = -errno;
> pr_debug("failed opening event %x\n", attr.bp_type);
> + }
>
> return fd;
> }
> @@ -77,7 +79,7 @@ static int test__wp_ro(struct test_suite *test __maybe_unused,
>
> fd = __event(HW_BREAKPOINT_R, (void *)&data1, sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> tmp = data1;
> WP_TEST_ASSERT_VAL(fd, "RO watchpoint", 1);
> @@ -101,7 +103,7 @@ static int test__wp_wo(struct test_suite *test __maybe_unused,
>
> fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> tmp = data1;
> WP_TEST_ASSERT_VAL(fd, "WO watchpoint", 0);
> @@ -126,7 +128,7 @@ static int test__wp_rw(struct test_suite *test __maybe_unused,
> fd = __event(HW_BREAKPOINT_R | HW_BREAKPOINT_W, (void *)&data1,
> sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> tmp = data1;
> WP_TEST_ASSERT_VAL(fd, "RW watchpoint", 1);
> @@ -150,7 +152,7 @@ static int test__wp_modify(struct test_suite *test __maybe_unused, int subtest _
>
> fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1));
> if (fd < 0)
> - return -1;
> + return fd == -ENODEV ? TEST_SKIP : -1;
>
> data1 = tmp;
> WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1);
>
> base-commit: 63a3bf5e8d9e79ce456c8f73d4395a5a51d841b1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-23 13:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-21 10:27 [PATCH] perf test: Skip watchpoint tests if no watchpoints available Naveen N. Rao
2022-11-22 19:18 ` Christophe Leroy
2022-11-22 20:57 ` Ian Rogers
2022-11-23 13:33 ` Arnaldo Carvalho de Melo
2022-11-23 8:05 ` kajoljain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).