Use new function trace_find_event_by_name to lookup events before looking through /sys files. This helps 'perf sched replay' to map event names to IDs correctly when processing perf.data recorded on another machine.
Signed-off-by: Dmitry Antipov dmitry.antipov@linaro.org --- tools/perf/util/evlist.c | 18 ++++++++++++++++-- tools/perf/util/trace-event-parse.c | 4 ++++ tools/perf/util/trace-event.h | 1 + 3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 4ac5f5a..7ebb9c5 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -17,6 +17,7 @@ #include <unistd.h>
#include "parse-events.h" +#include "trace-event.h"
#include <sys/mman.h>
@@ -231,12 +232,25 @@ int perf_evlist__set_tracepoints_handlers(struct perf_evlist *evlist, const struct perf_evsel_str_handler *assocs, size_t nr_assocs) { + struct event_format *event; struct perf_evsel *evsel; + char *p, *sys, *name; int err; - size_t i; + size_t i, off;
for (i = 0; i < nr_assocs; i++) { - err = trace_event__id(assocs[i].name); + err = -ENOENT; + p = strchr(assocs[i].name, ':'); + if (!p) + goto out; + off = p - assocs[i].name; + sys = malloc(off + 1); + memcpy(sys, assocs[i].name, off); + sys[off] = '\0'; + name = p + 1; + event = trace_find_event_by_name(sys, name); + err = event ? event->id : trace_event__id(assocs[i].name); + free(sys); if (err < 0) goto out;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c index df2fddb..44cbb40 100644 --- a/tools/perf/util/trace-event-parse.c +++ b/tools/perf/util/trace-event-parse.c @@ -176,6 +176,10 @@ struct event_format *trace_find_event(int type) return pevent_find_event(pevent, type); }
+struct event_format *trace_find_event_by_name(const char *sys, const char *name) +{ + return pevent_find_event_by_name(pevent, sys, name); +}
void print_trace_event(int cpu, void *data, int size) { diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h index 639852a..66f83a0 100644 --- a/tools/perf/util/trace-event.h +++ b/tools/perf/util/trace-event.h @@ -40,6 +40,7 @@ int parse_event_file(char *buf, unsigned long size, char *sys);
struct pevent_record *trace_peek_data(int cpu); struct event_format *trace_find_event(int type); +struct event_format *trace_find_event_by_name(const char *sys, const char *name);
unsigned long long raw_field_value(struct event_format *event, const char *name, void *data);
Hi,
On Sat, 9 Jun 2012 13:05:58 +0400, Dmitry Antipov wrote:
Use new function trace_find_event_by_name to lookup events before looking through /sys files. This helps 'perf sched replay' to map event names to IDs correctly when processing perf.data recorded on another machine.
Basically the same approach with the previous reply, please put this into trace_event__id(). And minor nits below..
Signed-off-by: Dmitry Antipov dmitry.antipov@linaro.org
tools/perf/util/evlist.c | 18 ++++++++++++++++-- tools/perf/util/trace-event-parse.c | 4 ++++ tools/perf/util/trace-event.h | 1 + 3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 4ac5f5a..7ebb9c5 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -17,6 +17,7 @@ #include <unistd.h> #include "parse-events.h" +#include "trace-event.h" #include <sys/mman.h> @@ -231,12 +232,25 @@ int perf_evlist__set_tracepoints_handlers(struct perf_evlist *evlist, const struct perf_evsel_str_handler *assocs, size_t nr_assocs) {
- struct event_format *event; struct perf_evsel *evsel;
- char *p, *sys, *name; int err;
- size_t i;
- size_t i, off;
for (i = 0; i < nr_assocs; i++) {
err = trace_event__id(assocs[i].name);
err = -ENOENT;p = strchr(assocs[i].name, ':');if (!p)goto out;
Is this really needed? It looks original trace_event__id() require this. But because we'll use pevent_find_event_by_name, the 'sys' part can be omitted from now on?
off = p - assocs[i].name;sys = malloc(off + 1);
The malloc() can fail, please check the return value.
Thanks, Namhyung
memcpy(sys, assocs[i].name, off);sys[off] = '\0';name = p + 1;event = trace_find_event_by_name(sys, name);err = event ? event->id : trace_event__id(assocs[i].name);free(sys);diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c index df2fddb..44cbb40 100644 --- a/tools/perf/util/trace-event-parse.c +++ b/tools/perf/util/trace-event-parse.c @@ -176,6 +176,10 @@ struct event_format *trace_find_event(int type) return pevent_find_event(pevent, type); } +struct event_format *trace_find_event_by_name(const char *sys, const char *name) +{
- return pevent_find_event_by_name(pevent, sys, name);
+} void print_trace_event(int cpu, void *data, int size) { diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h index 639852a..66f83a0 100644 --- a/tools/perf/util/trace-event.h +++ b/tools/perf/util/trace-event.h @@ -40,6 +40,7 @@ int parse_event_file(char *buf, unsigned long size, char *sys); struct pevent_record *trace_peek_data(int cpu); struct event_format *trace_find_event(int type); +struct event_format *trace_find_event_by_name(const char *sys, const char *name); unsigned long long raw_field_value(struct event_format *event, const char *name, void *data);
Em Mon, Jun 11, 2012 at 02:46:02PM +0900, Namhyung Kim escreveu:
On Sat, 9 Jun 2012 13:05:58 +0400, Dmitry Antipov wrote:
Use new function trace_find_event_by_name to lookup events before looking through /sys files. This helps 'perf sched replay' to map event names to IDs correctly when processing perf.data recorded on another machine.
Basically the same approach with the previous reply, please put this into trace_event__id(). And minor nits below..
Well, trace_event__id() is private to evlist and evlist so far is a local thing, i.e. it doesn't know anything about perf.data files.
So I think we should have a per perf.data (perf_session) method that knows that it shouldn't look _at all_ to /sys, but just at what came in the perf.data file.
As well when we want something that is on the running machine, even if we're dealing somehow with a perf.data file, we shouldn't use what is in it.
- Arnaldo
Hi,
On Mon, 11 Jun 2012 11:08:52 -0300, Arnaldo Carvalho de Melo wrote:
Em Mon, Jun 11, 2012 at 02:46:02PM +0900, Namhyung Kim escreveu:
On Sat, 9 Jun 2012 13:05:58 +0400, Dmitry Antipov wrote:
Use new function trace_find_event_by_name to lookup events before looking through /sys files. This helps 'perf sched replay' to map event names to IDs correctly when processing perf.data recorded on another machine.
Basically the same approach with the previous reply, please put this into trace_event__id(). And minor nits below..
Well, trace_event__id() is private to evlist and evlist so far is a local thing, i.e. it doesn't know anything about perf.data files.
Really? I see that perf_session__open make up an evlist for the session and a tracepoint event in the evlist should look up the perf.data first. As this patch addressed, perf sched replay dealt with the session->evlist already. Am I missing something?
So I think we should have a per perf.data (perf_session) method that knows that it shouldn't look _at all_ to /sys, but just at what came in the perf.data file.
Fair enough. The method should be a simple wrapper to libtraceevent APIs like this patch.
As well when we want something that is on the running machine, even if we're dealing somehow with a perf.data file, we shouldn't use what is in it.
That's the current behavior of the trace_event__id(). Do you want to make it public?
Thanks, Namhyung
Em Tue, Jun 12, 2012 at 03:01:26PM +0900, Namhyung Kim escreveu:
Hi,
On Mon, 11 Jun 2012 11:08:52 -0300, Arnaldo Carvalho de Melo wrote:
Em Mon, Jun 11, 2012 at 02:46:02PM +0900, Namhyung Kim escreveu:
On Sat, 9 Jun 2012 13:05:58 +0400, Dmitry Antipov wrote:
Use new function trace_find_event_by_name to lookup events before looking through /sys files. This helps 'perf sched replay' to map event names to IDs correctly when processing perf.data recorded on another machine.
Basically the same approach with the previous reply, please put this into trace_event__id(). And minor nits below..
Well, trace_event__id() is private to evlist and evlist so far is a local thing, i.e. it doesn't know anything about perf.data files.
Really? I see that perf_session__open make up an evlist for the session and a tracepoint event in the evlist should look up the perf.data first. As this patch addressed, perf sched replay dealt with the session->evlist already. Am I missing something?
I sent a patch fixing it, basically after creating the evlist in perf_session__open it will traverse it, looking up the pevents list created while processing the trace feature section in the header, setting up evsel->name properly.
So I think we should have a per perf.data (perf_session) method that knows that it shouldn't look _at all_ to /sys, but just at what came in the perf.data file.
Fair enough. The method should be a simple wrapper to libtraceevent APIs like this patch.
Right, that is what it does.
As well when we want something that is on the running machine, even if we're dealing somehow with a perf.data file, we shouldn't use what is in it.
That's the current behavior of the trace_event__id(). Do you want to make it public?
No need for it, the only users should be inside evsel.c.
- Arnaldo
Em Tue, Jun 12, 2012 at 03:01:26PM +0900, Namhyung Kim escreveu:
On Mon, 11 Jun 2012 11:08:52 -0300, Arnaldo Carvalho de Melo wrote:
So I think we should have a per perf.data (perf_session) method that knows that it shouldn't look _at all_ to /sys, but just at what came in the perf.data file.
Fair enough. The method should be a simple wrapper to libtraceevent APIs like this patch.
The pevent thing is per perf.data file, so I made it stop being static and become a perf_session member, so tools processing perf.data files use perf_session and _there_ we read the event into session->pevent and then have to change everywhere to stop using that single global pevent variable and use the per session one.
Dmitry, can you test the attached patch to check if it solves the problems you reported?
Note that it _doesn't_ fall backs to trace__event_id, as we're not interested at all in what is present in the /sys/kernel/debug/tracing/events in the workstation doing the analysis, just in what is in the perf.data file.
- Arnaldo
On 06/26/2012 03:20 AM, Arnaldo Carvalho de Melo wrote:
Fair enough. The method should be a simple wrapper to libtraceevent APIs like this patch.
The pevent thing is per perf.data file, so I made it stop being static and become a perf_session member, so tools processing perf.data files use perf_session and _there_ we read the event into session->pevent and then have to change everywhere to stop using that single global pevent variable and use the per session one.
Dmitry, can you test the attached patch to check if it solves the problems you reported?
This looks good and works for my x86 <-> ARM tests.
Note that it _doesn't_ fall backs to trace__event_id, as we're not interested at all in what is present in the /sys/kernel/debug/tracing/events in the workstation doing the analysis, just in what is in the perf.data file.
Agree.
Dmitry
Em Wed, Jun 27, 2012 at 01:18:17PM +0400, Dmitry Antipov escreveu:
On 06/26/2012 03:20 AM, Arnaldo Carvalho de Melo wrote:
Fair enough. The method should be a simple wrapper to libtraceevent APIs like this patch.
The pevent thing is per perf.data file, so I made it stop being static and become a perf_session member, so tools processing perf.data files use perf_session and _there_ we read the event into session->pevent and then have to change everywhere to stop using that single global pevent variable and use the per session one.
Dmitry, can you test the attached patch to check if it solves the problems you reported?
This looks good and works for my x86 <-> ARM tests.
Thanks, adding your Tested-by: Dmitry, ok?
Note that it _doesn't_ fall backs to trace__event_id, as we're not interested at all in what is present in the /sys/kernel/debug/tracing/events in the workstation doing the analysis, just in what is in the perf.data file.
Agree.
Dmitry
Em Sat, Jun 09, 2012 at 01:05:58PM +0400, Dmitry Antipov escreveu:
Use new function trace_find_event_by_name to lookup events before looking through /sys files. This helps 'perf sched replay' to map event names to IDs correctly when processing perf.data recorded on another machine.
Signed-off-by: Dmitry Antipov dmitry.antipov@linaro.org
tools/perf/util/evlist.c | 18 ++++++++++++++++-- tools/perf/util/trace-event-parse.c | 4 ++++ tools/perf/util/trace-event.h | 1 + 3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 4ac5f5a..7ebb9c5 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -17,6 +17,7 @@ #include <unistd.h> #include "parse-events.h" +#include "trace-event.h" #include <sys/mman.h> @@ -231,12 +232,25 @@ int perf_evlist__set_tracepoints_handlers(struct perf_evlist *evlist, const struct perf_evsel_str_handler *assocs, size_t nr_assocs) {
- struct event_format *event; struct perf_evsel *evsel;
- char *p, *sys, *name; int err;
- size_t i;
- size_t i, off;
for (i = 0; i < nr_assocs; i++) {
err = trace_event__id(assocs[i].name);
err = -ENOENT;p = strchr(assocs[i].name, ':');if (!p)goto out;off = p - assocs[i].name;sys = malloc(off + 1);
malloc can fail, so testing it is needed here. Also I think using just strdup(assocs[i].name), then looking at the separators and then inline setting them to '\0', etc would be simpler, no?
memcpy(sys, assocs[i].name, off);sys[off] = '\0';name = p + 1;event = trace_find_event_by_name(sys, name);err = event ? event->id : trace_event__id(assocs[i].name);
I'll try and cook an alternate patch fixing this problem.
free(sys);diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c index df2fddb..44cbb40 100644 --- a/tools/perf/util/trace-event-parse.c +++ b/tools/perf/util/trace-event-parse.c @@ -176,6 +176,10 @@ struct event_format *trace_find_event(int type) return pevent_find_event(pevent, type); } +struct event_format *trace_find_event_by_name(const char *sys, const char *name) +{
- return pevent_find_event_by_name(pevent, sys, name);
+} void print_trace_event(int cpu, void *data, int size) { diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h index 639852a..66f83a0 100644 --- a/tools/perf/util/trace-event.h +++ b/tools/perf/util/trace-event.h @@ -40,6 +40,7 @@ int parse_event_file(char *buf, unsigned long size, char *sys); struct pevent_record *trace_peek_data(int cpu); struct event_format *trace_find_event(int type); +struct event_format *trace_find_event_by_name(const char *sys, const char *name); unsigned long long raw_field_value(struct event_format *event, const char *name, void *data); -- 1.7.7.6
-
Arnaldo Carvalho de Melo -
Dmitry Antipov -
Namhyung Kim -
Namhyung Kim