[PATCH] perf: fix wrong hw_breakpoint documentation

[PATCH] perf: fix wrong hw_breakpoint documentation

[Jovi Zhang]

>From f5f9c3a064482cf3d0fb7ed788c66630bddbfc79 Mon Sep 17 00:00:00 2001
From: Jovi Zhang <bookjovi@gmail.com>
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 <bookjovi@gmail.com>
---
 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

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Namhyung Kim]

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 <bookjovi@gmail.com>
> 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]+
 
 %%
 

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Jovi Zhang]

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

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Arnaldo Carvalho de Melo]

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 <bookjovi@gmail.com>
> > 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]+
>  
>  %%
>  

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Jiri Olsa]

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 <bookjovi@gmail.com>
> > > 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

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Peter Zijlstra]

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 ;-)

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Jiri Olsa]

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 ;-)

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Namhyung Kim]

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

Re: [PATCH] perf: fix wrong hw_breakpoint documentation

[Jiri Olsa]

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

[PATCH 0/2] perf, tool: Hw breakpoint events parsing fixes

[Jiri Olsa]

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(-)

[PATCH 1/2] perf, tool: Fix hw breakpoint's type modifier parsing

[Jiri Olsa]

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 <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 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

Re: [PATCH 1/2] perf, tool: Fix hw breakpoint's type modifier parsing

[Namhyung Kim]

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 <bookzovi@gmail.com>

Thanks,
Namhyung


> Original-patch-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
>  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}
>  
>  %%

Re: [PATCH 1/2] perf, tool: Fix hw breakpoint's type modifier parsing

[Jovi Zhang]

On Fri, Jun 29, 2012 at 8:40 AM, Namhyung Kim <namhyung@kernel.org> 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 <bookzovi@gmail.com>
>
> Thanks,
> Namhyung
>

Thanks Namhyung, Jovi Zhang <bookjovi@gmail.com> :)

Re: [PATCH 1/2] perf, tool: Fix hw breakpoint's type modifier parsing

[Namhyung Kim]

On Fri, 29 Jun 2012 09:01:05 +0800, Jovi Zhang wrote:
> On Fri, Jun 29, 2012 at 8:40 AM, Namhyung Kim <namhyung@kernel.org> 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 <bookzovi@gmail.com>
>>
>> Thanks,
>> Namhyung
>>
>
> Thanks Namhyung, Jovi Zhang <bookjovi@gmail.com> :)

Oops, sorry about that. ;-)

Thanks,
Namhyung

[PATCHv2 1/2] perf, tool: Fix hw breakpoint's type modifier parsing

[Jiri Olsa]

On Fri, Jun 29, 2012 at 09:01:05AM +0800, Jovi Zhang wrote:
> On Fri, Jun 29, 2012 at 8:40 AM, Namhyung Kim <namhyung@kernel.org> 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 <bookzovi@gmail.com>
> >
> > Thanks,
> > Namhyung
> >
> 
> Thanks Namhyung, Jovi Zhang <bookjovi@gmail.com> :)

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 <namhyung@kernel.org>
Reported-by: Jovi Zhang <bookjovi@gmail.com>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 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

[tip:perf/core] perf tools: Fix hw breakpoint' s type modifier parsing

[tip-bot for Jiri Olsa]

Commit-ID:  7582732f57e547929d4c65ad8e38f3446e0538d8
Gitweb:     http://git.kernel.org/tip/7582732f57e547929d4c65ad8e38f3446e0538d8
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Fri, 29 Jun 2012 09:22:54 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 29 Jun 2012 13:28:13 -0300

perf tools: Fix hw breakpoint's type modifier parsing

Fixing the hw breakpoint's type modifier parsing to allow all possible
combinations of 'rwx' characters.

Adding automated tests to the parsing test suite.

Reported-by: Jovi Zhang <bookjovi@gmail.com>
Original-patch-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Jovi Zhang <bookjovi@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20120629072254.GA940@krava.brq.redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 a0f61a2..de81fe1 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)
 {
 
@@ -585,6 +614,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}
 
 %%
 

[PATCH 2/2] perf, tool: Handle hw breakpoints event names via perf_evsel__name

[Jiri Olsa]

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 <jolsa@redhat.com>
---
 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

[tip:perf/core] perf evsel: Handle hw breakpoints event names in perf_evsel__name()

[tip-bot for Jiri Olsa]

Commit-ID:  287e74aa3db097469bdca401f33f00ef20dc710d
Gitweb:     http://git.kernel.org/tip/287e74aa3db097469bdca401f33f00ef20dc710d
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 28 Jun 2012 23:18:49 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 29 Jun 2012 13:28:13 -0300

perf evsel: Handle hw breakpoints event names in perf_evsel__name()

Adding hw breakpoint events hook in the perf_evsel__name function, to
display event names properly all over the perf tools.

Updated hw breakpoints events tests.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Jovi Zhang <bookjovi@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1340918329-3012-3-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 3d1f696..e817713 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -15,6 +15,7 @@
 #include "cpumap.h"
 #include "thread_map.h"
 #include "target.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))
@@ -152,6 +153,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 = scnprintf(bf, size, "mem:0x%" PRIx64 ":", addr);
+
+	if (type & HW_BREAKPOINT_R)
+		r += scnprintf(bf + r, size - r, "r");
+
+	if (type & HW_BREAKPOINT_W)
+		r += scnprintf(bf + r, size - r, "w");
+
+	if (type & HW_BREAKPOINT_X)
+		r += scnprintf(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",		},
@@ -285,6 +311,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 de81fe1..dd0c306 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,