2012-05-11 00:18:01

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] perf, utils: Print the original modifiers in event_name

From: Andi Kleen <[email protected]>

We need to save the original string for perf stat output,
otherwise :t and :c are invisible.

This used to work, but broke with the parser refactoring.

Not sure if this is the best fix.

Signed-off-by: Andi Kleen <[email protected]>
---
tools/perf/util/parse-events.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 14fa682..cc561c2 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -810,6 +810,7 @@ int parse_events(struct perf_evlist *evlist, const char *str, int unset __used)
LIST_HEAD(list_tmp);
YY_BUFFER_STATE buffer;
int ret, idx = evlist->nr_entries;
+ struct perf_evsel *evsel;

buffer = parse_events__scan_string(str);

@@ -818,6 +819,10 @@ int parse_events(struct perf_evlist *evlist, const char *str, int unset __used)
parse_events__flush_buffer(buffer);
parse_events__delete_buffer(buffer);

+ /* XXX right thing for multiple events? Do all callers free? */
+ list_for_each_entry(evsel, &list, node)
+ evsel->name = strdup(str);
+
if (!ret) {
int entries = idx - evlist->nr_entries;
perf_evlist__splice_list_tail(evlist, &list, entries);
--
1.7.7.6


2012-05-11 10:30:22

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH] perf, tool: Add modifiers to be part of the event name

On Thu, May 10, 2012 at 05:17:48PM -0700, Andi Kleen wrote:
> From: Andi Kleen <[email protected]>
>
> We need to save the original string for perf stat output,
> otherwise :t and :c are invisible.

right, the modifier got lost

We need to fix the name once modifier is detected and final
event name is assigned (e.g. there could be "*?" in the tracepoint
event name, causing the name to be unwinded..).

Please check attached patch.

I'll add some testcases later, since there's patchset
waiting in Arnaldo's queue that would need to be rebased.

thanks,
jirka


---
We lost modifier as part of the event name after event parser
refactoring. Adding it back so it could be properly displayed
in perf stat output.

If there's a modifier specified for the event we fix the event
name to include it.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/parse-events.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 5b3a0ef..6125f5e 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -684,11 +684,24 @@ void parse_events_update_lists(struct list_head *list_event,
INIT_LIST_HEAD(list_event);
}

+static int update_event_name(struct perf_evsel *evsel, char *mod)
+{
+ char name[MAX_NAME_LEN];
+
+ /* Each event should have a name defined at this point. */
+ BUG_ON(!evsel->name);
+ snprintf(name, MAX_NAME_LEN, "%s:%s", evsel->name, mod);
+ free(evsel->name);
+ evsel->name = strdup(name);
+ return evsel->name ? 0 : -ENOMEM;
+}
+
int parse_events_modifier(struct list_head *list, char *str)
{
struct perf_evsel *evsel;
int exclude = 0, exclude_GH = 0;
int eu = 0, ek = 0, eh = 0, eH = 0, eG = 0, precise = 0;
+ char *mod = str;

if (str == NULL)
return 0;
@@ -742,6 +755,9 @@ int parse_events_modifier(struct list_head *list, char *str)
evsel->attr.precise_ip = precise;
evsel->attr.exclude_host = eH;
evsel->attr.exclude_guest = eG;
+
+ if (update_event_name(evsel, mod))
+ return -ENOMEM;
}

return 0;
--
1.7.7.6

2012-05-14 23:55:23

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] perf, tool: Add modifiers to be part of the event name

> Please check attached patch.

Does not apply to Linus tree. I assume you want to fix it there --
after all it's a regression -- so do need a patch for that.

-Andi

2012-05-15 11:54:53

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf, tool: Add modifiers to be part of the event name

On Tue, May 15, 2012 at 01:55:17AM +0200, Andi Kleen wrote:
> > Please check attached patch.
>
> Does not apply to Linus tree. I assume you want to fix it there --
> after all it's a regression -- so do need a patch for that.

hm, I got it cleanly applied/built in:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git

what do I miss?
jirka

2012-05-25 22:35:30

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] perf, tool: Add modifiers to be part of the event name

On Fri, May 11, 2012 at 12:25:22PM +0200, Jiri Olsa wrote:
> On Thu, May 10, 2012 at 05:17:48PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <[email protected]>
> >
> > We need to save the original string for perf stat output,
> > otherwise :t and :c are invisible.
>
> right, the modifier got lost
>
> We need to fix the name once modifier is detected and final
> event name is assigned (e.g. there could be "*?" in the tracepoint
> event name, causing the name to be unwinded..).

What happened to this patch? Seems to be still broken in 3.4

-Andi

2012-05-26 03:16:09

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf, tool: Add modifiers to be part of the event name

Em Sat, May 26, 2012 at 12:35:28AM +0200, Andi Kleen escreveu:
> On Fri, May 11, 2012 at 12:25:22PM +0200, Jiri Olsa wrote:
> > On Thu, May 10, 2012 at 05:17:48PM -0700, Andi Kleen wrote:
> > > From: Andi Kleen <[email protected]>
> > >
> > > We need to save the original string for perf stat output,
> > > otherwise :t and :c are invisible.
> >
> > right, the modifier got lost
> >
> > We need to fix the name once modifier is detected and final
> > event name is assigned (e.g. there could be "*?" in the tracepoint
> > event name, causing the name to be unwinded..).
>
> What happened to this patch? Seems to be still broken in 3.4

The latest patchset I submitted should fix this for raw and hardware
events.

Saving the events specified by the user is not sufficient, as those
strings are not saved in perf.data files, we must reconstruct the
strings from what is stored in the perf_event_attr fields.

- Arnaldo