>From f5f9c3a064482cf3d0fb7ed788c66630bddbfc79 Mon Sep 17 00:00:00 2001
From: Jovi Zhang <[email protected]>
Date: Wed, 27 Jun 2012 16:09:21 +0800
Subject: [PATCH] perf: fix wrong hw_breakpoint documentation
read-write access hw_breakpoint event is passed as 'mem:addr',
'mem:0x1000:rw' is parsed as invalid argument currently.
Signed-off-by: Jovi Zhang <[email protected]>
---
tools/perf/Documentation/perf-record.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-record.txt
b/tools/perf/Documentation/perf-record.txt
index b38a1f9..f9321b3 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -38,7 +38,7 @@ OPTIONS
Access is the memory access type (read, write, execute) it can
be passed as follows: '\mem:addr[:[r][w][x]]'.
If you want to profile read-write accesses in 0x1000, just set
- 'mem:0x1000:rw'.
+ 'mem:0x1000'.
--filter=<filter>::
Event filter.
--
1.7.9.7
Hi, Jovi
On Wed, 27 Jun 2012 16:39:19 +0800, Jovi Zhang wrote:
> From f5f9c3a064482cf3d0fb7ed788c66630bddbfc79 Mon Sep 17 00:00:00 2001
> From: Jovi Zhang <[email protected]>
> Date: Wed, 27 Jun 2012 16:09:21 +0800
> Subject: [PATCH] perf: fix wrong hw_breakpoint documentation
>
> read-write access hw_breakpoint event is passed as 'mem:addr',
> 'mem:0x1000:rw' is parsed as invalid argument currently.
>
It should be a bug in event parser. I guess the patch below will fix it:
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 488362e14133..aafca33a8a09 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -76,7 +76,7 @@ num_hex 0x[a-fA-F0-9]+
num_raw_hex [a-fA-F0-9]+
name [a-zA-Z_*?][a-zA-Z0-9_*?]*
modifier_event [ukhpGH]{1,8}
-modifier_bp [rwx]
+modifier_bp [rwx]+
%%
Hi Kim,
> It should be a bug in event parser. I guess the patch below will fix it:
>
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 488362e14133..aafca33a8a09 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -76,7 +76,7 @@ num_hex 0x[a-fA-F0-9]+
> num_raw_hex [a-fA-F0-9]+
> name [a-zA-Z_*?][a-zA-Z0-9_*?]*
> modifier_event [ukhpGH]{1,8}
> -modifier_bp [rwx]
> +modifier_bp [rwx]+
>
that should also be fine, but it will export rx/wx interface to user,
and rx/wx mostly not supported on many architecture.
Perhaps this issue is very tiny :)
[root@jovi perf]# ./perf stat -e mem:0x080652c8:rx -e mem:0x0 --
/usr/bin/ls > /dev/null
Performance counter stats for '/usr/bin/ls':
<not supported> mem:0x80652c8:rx
0 mem:(nil):rw
0.006728643 seconds time elapsed
Em Wed, Jun 27, 2012 at 05:59:34PM +0900, Namhyung Kim escreveu:
> Hi, Jovi
>
> On Wed, 27 Jun 2012 16:39:19 +0800, Jovi Zhang wrote:
> > From f5f9c3a064482cf3d0fb7ed788c66630bddbfc79 Mon Sep 17 00:00:00 2001
> > From: Jovi Zhang <[email protected]>
> > Date: Wed, 27 Jun 2012 16:09:21 +0800
> > Subject: [PATCH] perf: fix wrong hw_breakpoint documentation
> >
> > read-write access hw_breakpoint event is passed as 'mem:addr',
> > 'mem:0x1000:rw' is parsed as invalid argument currently.
> >
>
> It should be a bug in event parser. I guess the patch below will fix it:
Jiri,
Ack? If so, Namhyung, can you send a patch with his Ack?
- Arnaldo
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 488362e14133..aafca33a8a09 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -76,7 +76,7 @@ num_hex 0x[a-fA-F0-9]+
> num_raw_hex [a-fA-F0-9]+
> name [a-zA-Z_*?][a-zA-Z0-9_*?]*
> modifier_event [ukhpGH]{1,8}
> -modifier_bp [rwx]
> +modifier_bp [rwx]+
>
> %%
>
On Wed, Jun 27, 2012 at 01:15:57PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 27, 2012 at 05:59:34PM +0900, Namhyung Kim escreveu:
> > Hi, Jovi
> >
> > On Wed, 27 Jun 2012 16:39:19 +0800, Jovi Zhang wrote:
> > > From f5f9c3a064482cf3d0fb7ed788c66630bddbfc79 Mon Sep 17 00:00:00 2001
> > > From: Jovi Zhang <[email protected]>
> > > Date: Wed, 27 Jun 2012 16:09:21 +0800
> > > Subject: [PATCH] perf: fix wrong hw_breakpoint documentation
> > >
> > > read-write access hw_breakpoint event is passed as 'mem:addr',
> > > 'mem:0x1000:rw' is parsed as invalid argument currently.
> > >
> >
> > It should be a bug in event parser. I guess the patch below will fix it:
>
> Jiri,
>
> Ack? If so, Namhyung, can you send a patch with his Ack?
>
> - Arnaldo
>
> >
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index 488362e14133..aafca33a8a09 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -76,7 +76,7 @@ num_hex 0x[a-fA-F0-9]+
> > num_raw_hex [a-fA-F0-9]+
> > name [a-zA-Z_*?][a-zA-Z0-9_*?]*
> > modifier_event [ukhpGH]{1,8}
> > -modifier_bp [rwx]
> > +modifier_bp [rwx]+
> >
> > %%
> >
we could have it more precise with
-modifier_bp [rwx]
+modifier_bp [rwx]{1,3}
and fail parse_breakpoint_type function for nonsense types
aaand automated tests updated ;)
jirka
On Wed, 2012-06-27 at 21:07 +0200, Jiri Olsa wrote:
> -modifier_bp [rwx]
> +modifier_bp [rwx]{1,3}
>
> and fail parse_breakpoint_type function for nonsense types
If you're going to modify parse_breakpoint_type() to fail on nonsense
types, you might as well do the + thing, right?
Doesn't really matter that much though ;-)
On Wed, Jun 27, 2012 at 09:58:22PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-06-27 at 21:07 +0200, Jiri Olsa wrote:
> > -modifier_bp [rwx]
> > +modifier_bp [rwx]{1,3}
> >
> > and fail parse_breakpoint_type function for nonsense types
>
> If you're going to modify parse_breakpoint_type() to fail on nonsense
> types, you might as well do the + thing, right?
>
> Doesn't really matter that much though ;-)
yo ;-)
On Wed, 27 Jun 2012 22:19:28 +0200, Jiri Olsa wrote:
> On Wed, Jun 27, 2012 at 09:58:22PM +0200, Peter Zijlstra wrote:
>> On Wed, 2012-06-27 at 21:07 +0200, Jiri Olsa wrote:
>> > -modifier_bp [rwx]
>> > +modifier_bp [rwx]{1,3}
>> >
>> > and fail parse_breakpoint_type function for nonsense types
>>
>> If you're going to modify parse_breakpoint_type() to fail on nonsense
>> types, you might as well do the + thing, right?
>>
>> Doesn't really matter that much though ;-)
>
> yo ;-)
So, are you ok to resend the original patch with your ack? Or will you
send the above soon?
Thanks,
Namhyung
On Thu, Jun 28, 2012 at 10:30:51AM +0900, Namhyung Kim wrote:
> On Wed, 27 Jun 2012 22:19:28 +0200, Jiri Olsa wrote:
> > On Wed, Jun 27, 2012 at 09:58:22PM +0200, Peter Zijlstra wrote:
> >> On Wed, 2012-06-27 at 21:07 +0200, Jiri Olsa wrote:
> >> > -modifier_bp [rwx]
> >> > +modifier_bp [rwx]{1,3}
> >> >
> >> > and fail parse_breakpoint_type function for nonsense types
> >>
> >> If you're going to modify parse_breakpoint_type() to fail on nonsense
> >> types, you might as well do the + thing, right?
> >>
> >> Doesn't really matter that much though ;-)
> >
> > yo ;-)
>
> So, are you ok to resend the original patch with your ack? Or will you
> send the above soon?
I can send the change by the end of the week
jirka
Fixing the hw breakpoint's type modifier parsing to allow all
possible combinations of 'rwx' characters.
Adding automated tests to the parsing test suite.
Original-patch-by: Namhyung Kim <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/parse-events-test.c | 37 +++++++++++++++++++++++++++++++++++
tools/perf/util/parse-events.c | 16 ++++++++++++--
tools/perf/util/parse-events.l | 2 +-
3 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
index 229af6d..50e6a94 100644
--- a/tools/perf/util/parse-events-test.c
+++ b/tools/perf/util/parse-events-test.c
@@ -181,6 +181,22 @@ static int test__checkevent_breakpoint_w(struct perf_evlist *evlist)
return 0;
}
+static int test__checkevent_breakpoint_rw(struct perf_evlist *evlist)
+{
+ struct perf_evsel *evsel = list_entry(evlist->entries.next,
+ struct perf_evsel, node);
+
+ TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->nr_entries);
+ TEST_ASSERT_VAL("wrong type",
+ PERF_TYPE_BREAKPOINT == evsel->attr.type);
+ TEST_ASSERT_VAL("wrong config", 0 == evsel->attr.config);
+ TEST_ASSERT_VAL("wrong bp_type",
+ (HW_BREAKPOINT_R|HW_BREAKPOINT_W) == evsel->attr.bp_type);
+ TEST_ASSERT_VAL("wrong bp_len",
+ HW_BREAKPOINT_LEN_4 == evsel->attr.bp_len);
+ return 0;
+}
+
static int test__checkevent_tracepoint_modifier(struct perf_evlist *evlist)
{
struct perf_evsel *evsel = list_entry(evlist->entries.next,
@@ -352,6 +368,19 @@ static int test__checkevent_breakpoint_w_modifier(struct perf_evlist *evlist)
return test__checkevent_breakpoint_w(evlist);
}
+static int test__checkevent_breakpoint_rw_modifier(struct perf_evlist *evlist)
+{
+ struct perf_evsel *evsel = list_entry(evlist->entries.next,
+ struct perf_evsel, node);
+
+ TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude_user);
+ TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
+ TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
+ TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+
+ return test__checkevent_breakpoint_rw(evlist);
+}
+
static int test__checkevent_pmu(struct perf_evlist *evlist)
{
@@ -584,6 +613,14 @@ static struct test__event_st test__events[] = {
.name = "instructions:H",
.check = test__checkevent_exclude_guest_modifier,
},
+ [26] = {
+ .name = "mem:0:rw",
+ .check = test__checkevent_breakpoint_rw,
+ },
+ [27] = {
+ .name = "mem:0:rw:kp",
+ .check = test__checkevent_breakpoint_rw_modifier,
+ },
};
#define TEST__EVENTS_CNT (sizeof(test__events) / sizeof(struct test__event_st))
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 0cc27da..7ae76af 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -383,21 +383,31 @@ parse_breakpoint_type(const char *type, struct perf_event_attr *attr)
if (!type || !type[i])
break;
+#define CHECK_SET_TYPE(bit) \
+do { \
+ if (attr->bp_type & bit) \
+ return -EINVAL; \
+ else \
+ attr->bp_type |= bit; \
+} while (0)
+
switch (type[i]) {
case 'r':
- attr->bp_type |= HW_BREAKPOINT_R;
+ CHECK_SET_TYPE(HW_BREAKPOINT_R);
break;
case 'w':
- attr->bp_type |= HW_BREAKPOINT_W;
+ CHECK_SET_TYPE(HW_BREAKPOINT_W);
break;
case 'x':
- attr->bp_type |= HW_BREAKPOINT_X;
+ CHECK_SET_TYPE(HW_BREAKPOINT_X);
break;
default:
return -EINVAL;
}
}
+#undef CHECK_SET_TYPE
+
if (!attr->bp_type) /* Default */
attr->bp_type = HW_BREAKPOINT_R | HW_BREAKPOINT_W;
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 488362e..a066894 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -76,7 +76,7 @@ num_hex 0x[a-fA-F0-9]+
num_raw_hex [a-fA-F0-9]+
name [a-zA-Z_*?][a-zA-Z0-9_*?]*
modifier_event [ukhpGH]{1,8}
-modifier_bp [rwx]
+modifier_bp [rwx]{1,3}
%%
--
1.7.7.6
hi,
patch 1 is the change we discussed, patch 2 is to fix hw breakpoint
event names.
Attached patches:
1/2 perf, tool: Fix hw breakpoint's type modifier parsing
2/2 perf, tool: Handle hw breakpoints event names via perf_evsel__name
jirka
---
tools/perf/util/evsel.c | 30 ++++++++++++++++++++++
tools/perf/util/parse-events-test.c | 47 +++++++++++++++++++++++++++++++++++
tools/perf/util/parse-events.c | 20 ++++++++++----
tools/perf/util/parse-events.l | 2 +-
4 files changed, 92 insertions(+), 7 deletions(-)
Adding hw breakpoint events hook to the perf_evsel__name function,
to display event names properly all over the perf.
Updating hw breakpoints events tests.
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/evsel.c | 30 ++++++++++++++++++++++++++++++
tools/perf/util/parse-events-test.c | 10 ++++++++++
tools/perf/util/parse-events.c | 4 +---
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 876f639..9a46487 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -16,6 +16,7 @@
#include "thread_map.h"
#include "target.h"
#include "../../include/linux/perf_event.h"
+#include "../../../include/linux/hw_breakpoint.h"
#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
#define GROUP_FD(group_fd, cpu) (*(int *)xyarray__entry(group_fd, cpu, 0))
@@ -153,6 +154,31 @@ static int perf_evsel__sw_name(struct perf_evsel *evsel, char *bf, size_t size)
return r + perf_evsel__add_modifiers(evsel, bf + r, size - r);
}
+static int __perf_evsel__bp_name(char *bf, size_t size, u64 addr, u64 type)
+{
+ int r;
+
+ r = snprintf(bf, size, "mem:0x%" PRIx64 ":", addr);
+
+ if (type & HW_BREAKPOINT_R)
+ r += snprintf(bf + r, size - r, "r");
+
+ if (type & HW_BREAKPOINT_W)
+ r += snprintf(bf + r, size - r, "w");
+
+ if (type & HW_BREAKPOINT_X)
+ r += snprintf(bf + r, size - r, "x");
+
+ return r;
+}
+
+static int perf_evsel__bp_name(struct perf_evsel *evsel, char *bf, size_t size)
+{
+ struct perf_event_attr *attr = &evsel->attr;
+ int r = __perf_evsel__bp_name(bf, size, attr->bp_addr, attr->bp_type);
+ return r + perf_evsel__add_modifiers(evsel, bf + r, size - r);
+}
+
const char *perf_evsel__hw_cache[PERF_COUNT_HW_CACHE_MAX]
[PERF_EVSEL__MAX_ALIASES] = {
{ "L1-dcache", "l1-d", "l1d", "L1-data", },
@@ -286,6 +312,10 @@ const char *perf_evsel__name(struct perf_evsel *evsel)
scnprintf(bf, sizeof(bf), "%s", "unknown tracepoint");
break;
+ case PERF_TYPE_BREAKPOINT:
+ perf_evsel__bp_name(evsel, bf, sizeof(bf));
+ break;
+
default:
scnprintf(bf, sizeof(bf), "%s", "unknown attr type");
break;
diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
index 50e6a94..5cefbec 100644
--- a/tools/perf/util/parse-events-test.c
+++ b/tools/perf/util/parse-events-test.c
@@ -325,6 +325,8 @@ static int test__checkevent_breakpoint_modifier(struct perf_evlist *evlist)
TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
+ TEST_ASSERT_VAL("wrong name",
+ !strcmp(perf_evsel__name(evsel), "mem:0x0:rw:u"));
return test__checkevent_breakpoint(evlist);
}
@@ -338,6 +340,8 @@ static int test__checkevent_breakpoint_x_modifier(struct perf_evlist *evlist)
TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
+ TEST_ASSERT_VAL("wrong name",
+ !strcmp(perf_evsel__name(evsel), "mem:0x0:x:k"));
return test__checkevent_breakpoint_x(evlist);
}
@@ -351,6 +355,8 @@ static int test__checkevent_breakpoint_r_modifier(struct perf_evlist *evlist)
TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", !evsel->attr.exclude_hv);
TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+ TEST_ASSERT_VAL("wrong name",
+ !strcmp(perf_evsel__name(evsel), "mem:0x0:r:hp"));
return test__checkevent_breakpoint_r(evlist);
}
@@ -364,6 +370,8 @@ static int test__checkevent_breakpoint_w_modifier(struct perf_evlist *evlist)
TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+ TEST_ASSERT_VAL("wrong name",
+ !strcmp(perf_evsel__name(evsel), "mem:0x0:w:up"));
return test__checkevent_breakpoint_w(evlist);
}
@@ -377,6 +385,8 @@ static int test__checkevent_breakpoint_rw_modifier(struct perf_evlist *evlist)
TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+ TEST_ASSERT_VAL("wrong name",
+ !strcmp(perf_evsel__name(evsel), "mem:0x0:rw:kp"));
return test__checkevent_breakpoint_rw(evlist);
}
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7ae76af..1dc44dc 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -418,7 +418,6 @@ int parse_events_add_breakpoint(struct list_head **list, int *idx,
void *ptr, char *type)
{
struct perf_event_attr attr;
- char name[MAX_NAME_LEN];
memset(&attr, 0, sizeof(attr));
attr.bp_addr = (unsigned long) ptr;
@@ -437,8 +436,7 @@ int parse_events_add_breakpoint(struct list_head **list, int *idx,
attr.type = PERF_TYPE_BREAKPOINT;
- snprintf(name, MAX_NAME_LEN, "mem:%p:%s", ptr, type ? type : "rw");
- return add_event(list, idx, &attr, name);
+ return add_event(list, idx, &attr, NULL);
}
static int config_term(struct perf_event_attr *attr,
--
1.7.7.6
On Thu, 28 Jun 2012 23:18:48 +0200, Jiri Olsa wrote:
> Fixing the hw breakpoint's type modifier parsing to allow all
> possible combinations of 'rwx' characters.
>
> Adding automated tests to the parsing test suite.
>
You might need this too:
Reported-by: Zovi Zhang <[email protected]>
Thanks,
Namhyung
> Original-patch-by: Namhyung Kim <[email protected]>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> tools/perf/util/parse-events-test.c | 37 +++++++++++++++++++++++++++++++++++
> tools/perf/util/parse-events.c | 16 ++++++++++++--
> tools/perf/util/parse-events.l | 2 +-
> 3 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
> index 229af6d..50e6a94 100644
> --- a/tools/perf/util/parse-events-test.c
> +++ b/tools/perf/util/parse-events-test.c
> @@ -181,6 +181,22 @@ static int test__checkevent_breakpoint_w(struct perf_evlist *evlist)
> return 0;
> }
>
> +static int test__checkevent_breakpoint_rw(struct perf_evlist *evlist)
> +{
> + struct perf_evsel *evsel = list_entry(evlist->entries.next,
> + struct perf_evsel, node);
> +
> + TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->nr_entries);
> + TEST_ASSERT_VAL("wrong type",
> + PERF_TYPE_BREAKPOINT == evsel->attr.type);
> + TEST_ASSERT_VAL("wrong config", 0 == evsel->attr.config);
> + TEST_ASSERT_VAL("wrong bp_type",
> + (HW_BREAKPOINT_R|HW_BREAKPOINT_W) == evsel->attr.bp_type);
> + TEST_ASSERT_VAL("wrong bp_len",
> + HW_BREAKPOINT_LEN_4 == evsel->attr.bp_len);
> + return 0;
> +}
> +
> static int test__checkevent_tracepoint_modifier(struct perf_evlist *evlist)
> {
> struct perf_evsel *evsel = list_entry(evlist->entries.next,
> @@ -352,6 +368,19 @@ static int test__checkevent_breakpoint_w_modifier(struct perf_evlist *evlist)
> return test__checkevent_breakpoint_w(evlist);
> }
>
> +static int test__checkevent_breakpoint_rw_modifier(struct perf_evlist *evlist)
> +{
> + struct perf_evsel *evsel = list_entry(evlist->entries.next,
> + struct perf_evsel, node);
> +
> + TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude_user);
> + TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
> + TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
> + TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
> +
> + return test__checkevent_breakpoint_rw(evlist);
> +}
> +
> static int test__checkevent_pmu(struct perf_evlist *evlist)
> {
>
> @@ -584,6 +613,14 @@ static struct test__event_st test__events[] = {
> .name = "instructions:H",
> .check = test__checkevent_exclude_guest_modifier,
> },
> + [26] = {
> + .name = "mem:0:rw",
> + .check = test__checkevent_breakpoint_rw,
> + },
> + [27] = {
> + .name = "mem:0:rw:kp",
> + .check = test__checkevent_breakpoint_rw_modifier,
> + },
> };
>
> #define TEST__EVENTS_CNT (sizeof(test__events) / sizeof(struct test__event_st))
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 0cc27da..7ae76af 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -383,21 +383,31 @@ parse_breakpoint_type(const char *type, struct perf_event_attr *attr)
> if (!type || !type[i])
> break;
>
> +#define CHECK_SET_TYPE(bit) \
> +do { \
> + if (attr->bp_type & bit) \
> + return -EINVAL; \
> + else \
> + attr->bp_type |= bit; \
> +} while (0)
> +
> switch (type[i]) {
> case 'r':
> - attr->bp_type |= HW_BREAKPOINT_R;
> + CHECK_SET_TYPE(HW_BREAKPOINT_R);
> break;
> case 'w':
> - attr->bp_type |= HW_BREAKPOINT_W;
> + CHECK_SET_TYPE(HW_BREAKPOINT_W);
> break;
> case 'x':
> - attr->bp_type |= HW_BREAKPOINT_X;
> + CHECK_SET_TYPE(HW_BREAKPOINT_X);
> break;
> default:
> return -EINVAL;
> }
> }
>
> +#undef CHECK_SET_TYPE
> +
> if (!attr->bp_type) /* Default */
> attr->bp_type = HW_BREAKPOINT_R | HW_BREAKPOINT_W;
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 488362e..a066894 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -76,7 +76,7 @@ num_hex 0x[a-fA-F0-9]+
> num_raw_hex [a-fA-F0-9]+
> name [a-zA-Z_*?][a-zA-Z0-9_*?]*
> modifier_event [ukhpGH]{1,8}
> -modifier_bp [rwx]
> +modifier_bp [rwx]{1,3}
>
> %%
On Fri, Jun 29, 2012 at 8:40 AM, Namhyung Kim <[email protected]> wrote:
> On Thu, 28 Jun 2012 23:18:48 +0200, Jiri Olsa wrote:
>> Fixing the hw breakpoint's type modifier parsing to allow all
>> possible combinations of 'rwx' characters.
>>
>> Adding automated tests to the parsing test suite.
>>
>
> You might need this too:
>
> Reported-by: Zovi Zhang <[email protected]>
>
> Thanks,
> Namhyung
>
Thanks Namhyung, Jovi Zhang <[email protected]> :)
On Fri, 29 Jun 2012 09:01:05 +0800, Jovi Zhang wrote:
> On Fri, Jun 29, 2012 at 8:40 AM, Namhyung Kim <[email protected]> wrote:
>> On Thu, 28 Jun 2012 23:18:48 +0200, Jiri Olsa wrote:
>>> Fixing the hw breakpoint's type modifier parsing to allow all
>>> possible combinations of 'rwx' characters.
>>>
>>> Adding automated tests to the parsing test suite.
>>>
>>
>> You might need this too:
>>
>> Reported-by: Zovi Zhang <[email protected]>
>>
>> Thanks,
>> Namhyung
>>
>
> Thanks Namhyung, Jovi Zhang <[email protected]> :)
Oops, sorry about that. ;-)
Thanks,
Namhyung
On Fri, Jun 29, 2012 at 09:01:05AM +0800, Jovi Zhang wrote:
> On Fri, Jun 29, 2012 at 8:40 AM, Namhyung Kim <[email protected]> wrote:
> > On Thu, 28 Jun 2012 23:18:48 +0200, Jiri Olsa wrote:
> >> Fixing the hw breakpoint's type modifier parsing to allow all
> >> possible combinations of 'rwx' characters.
> >>
> >> Adding automated tests to the parsing test suite.
> >>
> >
> > You might need this too:
> >
> > ?Reported-by: Zovi Zhang <[email protected]>
> >
> > Thanks,
> > Namhyung
> >
>
> Thanks Namhyung, Jovi Zhang <[email protected]> :)
oops, sry about that..
Arnaldo, please take this one
thanks,
jirka
---
Fixing the hw breakpoint's type modifier parsing to allow all
possible combinations of 'rwx' characters.
Adding automated tests to the parsing test suite.
Original-patch-by: Namhyung Kim <[email protected]>
Reported-by: Jovi Zhang <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/parse-events-test.c | 37 +++++++++++++++++++++++++++++++++++
tools/perf/util/parse-events.c | 16 ++++++++++++--
tools/perf/util/parse-events.l | 2 +-
3 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
index 229af6d..50e6a94 100644
--- a/tools/perf/util/parse-events-test.c
+++ b/tools/perf/util/parse-events-test.c
@@ -181,6 +181,22 @@ static int test__checkevent_breakpoint_w(struct perf_evlist *evlist)
return 0;
}
+static int test__checkevent_breakpoint_rw(struct perf_evlist *evlist)
+{
+ struct perf_evsel *evsel = list_entry(evlist->entries.next,
+ struct perf_evsel, node);
+
+ TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->nr_entries);
+ TEST_ASSERT_VAL("wrong type",
+ PERF_TYPE_BREAKPOINT == evsel->attr.type);
+ TEST_ASSERT_VAL("wrong config", 0 == evsel->attr.config);
+ TEST_ASSERT_VAL("wrong bp_type",
+ (HW_BREAKPOINT_R|HW_BREAKPOINT_W) == evsel->attr.bp_type);
+ TEST_ASSERT_VAL("wrong bp_len",
+ HW_BREAKPOINT_LEN_4 == evsel->attr.bp_len);
+ return 0;
+}
+
static int test__checkevent_tracepoint_modifier(struct perf_evlist *evlist)
{
struct perf_evsel *evsel = list_entry(evlist->entries.next,
@@ -352,6 +368,19 @@ static int test__checkevent_breakpoint_w_modifier(struct perf_evlist *evlist)
return test__checkevent_breakpoint_w(evlist);
}
+static int test__checkevent_breakpoint_rw_modifier(struct perf_evlist *evlist)
+{
+ struct perf_evsel *evsel = list_entry(evlist->entries.next,
+ struct perf_evsel, node);
+
+ TEST_ASSERT_VAL("wrong exclude_user", evsel->attr.exclude_user);
+ TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->attr.exclude_kernel);
+ TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
+ TEST_ASSERT_VAL("wrong precise_ip", evsel->attr.precise_ip);
+
+ return test__checkevent_breakpoint_rw(evlist);
+}
+
static int test__checkevent_pmu(struct perf_evlist *evlist)
{
@@ -584,6 +613,14 @@ static struct test__event_st test__events[] = {
.name = "instructions:H",
.check = test__checkevent_exclude_guest_modifier,
},
+ [26] = {
+ .name = "mem:0:rw",
+ .check = test__checkevent_breakpoint_rw,
+ },
+ [27] = {
+ .name = "mem:0:rw:kp",
+ .check = test__checkevent_breakpoint_rw_modifier,
+ },
};
#define TEST__EVENTS_CNT (sizeof(test__events) / sizeof(struct test__event_st))
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 0cc27da..7ae76af 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -383,21 +383,31 @@ parse_breakpoint_type(const char *type, struct perf_event_attr *attr)
if (!type || !type[i])
break;
+#define CHECK_SET_TYPE(bit) \
+do { \
+ if (attr->bp_type & bit) \
+ return -EINVAL; \
+ else \
+ attr->bp_type |= bit; \
+} while (0)
+
switch (type[i]) {
case 'r':
- attr->bp_type |= HW_BREAKPOINT_R;
+ CHECK_SET_TYPE(HW_BREAKPOINT_R);
break;
case 'w':
- attr->bp_type |= HW_BREAKPOINT_W;
+ CHECK_SET_TYPE(HW_BREAKPOINT_W);
break;
case 'x':
- attr->bp_type |= HW_BREAKPOINT_X;
+ CHECK_SET_TYPE(HW_BREAKPOINT_X);
break;
default:
return -EINVAL;
}
}
+#undef CHECK_SET_TYPE
+
if (!attr->bp_type) /* Default */
attr->bp_type = HW_BREAKPOINT_R | HW_BREAKPOINT_W;
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 488362e..a066894 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -76,7 +76,7 @@ num_hex 0x[a-fA-F0-9]+
num_raw_hex [a-fA-F0-9]+
name [a-zA-Z_*?][a-zA-Z0-9_*?]*
modifier_event [ukhpGH]{1,8}
-modifier_bp [rwx]
+modifier_bp [rwx]{1,3}
%%
--
1.7.7.6