Use trace_find_event to find event name before looking through /sys files. This helps 'perf report' to show real event names instead of 'unknown:unknown' when processing perf.data recorded on another machine.
Signed-off-by: Dmitry Antipov dmitry.antipov@linaro.org --- tools/perf/builtin-report.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 8c767c6..a6fd309 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -24,6 +24,7 @@ #include "util/evlist.h" #include "util/evsel.h" #include "util/header.h" +#include "util/trace-event.h" #include "util/session.h" #include "util/tool.h"
@@ -314,7 +315,8 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
list_for_each_entry(pos, &evlist->entries, node) { struct hists *hists = &pos->hists; - const char *evname = event_name(pos); + struct event_format *event = trace_find_event(pos->attr.config); + const char *evname = event ? event->name : event_name(pos);
hists__fprintf_nr_sample_events(hists, evname, stdout); hists__fprintf(hists, NULL, false, true, 0, 0, stdout);
Hi,
On Fri, 8 Jun 2012 16:23:27 +0400, Dmitry Antipov wrote:
Use trace_find_event to find event name before looking through /sys files. This helps 'perf report' to show real event names instead of 'unknown:unknown' when processing perf.data recorded on another machine.
Right, it should be a default action for a tracepoint event IMHO. (But this patch doesn't check it's a tracepoint) There are a lot of places call event_name() to be converted like this, so I suggest changing event_name itself (or recent perf_evsel__name() ?) instead of just a call-site. It might require checking whether the pevent is initialized and if not, falls back to the sysfs walking.
Thanks, Namhyung
Signed-off-by: Dmitry Antipov dmitry.antipov@linaro.org
tools/perf/builtin-report.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 8c767c6..a6fd309 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -24,6 +24,7 @@ #include "util/evlist.h" #include "util/evsel.h" #include "util/header.h" +#include "util/trace-event.h" #include "util/session.h" #include "util/tool.h" @@ -314,7 +315,8 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist, list_for_each_entry(pos, &evlist->entries, node) { struct hists *hists = &pos->hists;
const char *evname = event_name(pos);
struct event_format *event = trace_find_event(pos->attr.config);const char *evname = event ? event->name : event_name(pos);hists__fprintf_nr_sample_events(hists, evname, stdout); hists__fprintf(hists, NULL, false, true, 0, 0, stdout);
Em Fri, Jun 08, 2012 at 04:23:27PM +0400, Dmitry Antipov escreveu:
Use trace_find_event to find event name before looking through /sys files. This helps 'perf report' to show real event names instead of 'unknown:unknown' when processing perf.data recorded on another machine.
We have to somehow tell perf_evlist__tty_browse_hists that it should try to figure out the name of the event by looking at _either_ /sys (local events) or what came in the perf.data file.
That is because 'perf top' and 'perf report' uses perf_evlist__tty_browse_hists. One is for local events (top) and the other for perf.data files, that may or not be for local (in the sense of running the same kernel for record + report) or for "remote" (running on the same machine but with a different kernel at record than the one used at report) or from a different machine altogether, perhaps even different arch.
So I think that what needs to be done is to cache the right event name before getting to the hists browser. I.e. when setting up the events locally or when reading the events from the perf.data header, this way perf_evsel__name() can be used everywhere and will just return event->name, BUG_ON if event->name is NULL, i.e. wasn't resolved already.
- Arnaldo
Signed-off-by: Dmitry Antipov dmitry.antipov@linaro.org
tools/perf/builtin-report.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 8c767c6..a6fd309 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -24,6 +24,7 @@ #include "util/evlist.h" #include "util/evsel.h" #include "util/header.h" +#include "util/trace-event.h" #include "util/session.h" #include "util/tool.h" @@ -314,7 +315,8 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist, list_for_each_entry(pos, &evlist->entries, node) { struct hists *hists = &pos->hists;
const char *evname = event_name(pos);
struct event_format *event = trace_find_event(pos->attr.config);const char *evname = event ? event->name : event_name(pos);hists__fprintf_nr_sample_events(hists, evname, stdout); hists__fprintf(hists, NULL, false, true, 0, 0, stdout); -- 1.7.7.6
Hi,
On Mon, 11 Jun 2012 11:14:16 -0300, Arnaldo Carvalho de Melo wrote:
Em Fri, Jun 08, 2012 at 04:23:27PM +0400, Dmitry Antipov escreveu:
Use trace_find_event to find event name before looking through /sys files. This helps 'perf report' to show real event names instead of 'unknown:unknown' when processing perf.data recorded on another machine.
We have to somehow tell perf_evlist__tty_browse_hists that it should try to figure out the name of the event by looking at _either_ /sys (local events) or what came in the perf.data file.
That is because 'perf top' and 'perf report' uses perf_evlist__tty_browse_hists. One is for local events (top) and the other for perf.data files, that may or not be for local (in the sense of running the same kernel for record + report) or for "remote" (running on the same machine but with a different kernel at record than the one used at report) or from a different machine altogether, perhaps even different arch.
I just thought that we should always consider the remote case first and falls back to local case because if we looked for local events, the remote events (perf.data) would not exist so that it can falls to the local case safely.
Now I think that we need a session method to check whether the current session is local or remote, and acts something based on that info.
Thanks, Namhyung
Em Tue, Jun 12, 2012 at 03:34:22PM +0900, Namhyung Kim escreveu:
On Mon, 11 Jun 2012 11:14:16 -0300, Arnaldo Carvalho de Melo wrote:
Em Fri, Jun 08, 2012 at 04:23:27PM +0400, Dmitry Antipov escreveu:
Use trace_find_event to find event name before looking through /sys files. This helps 'perf report' to show real event names instead of 'unknown:unknown' when processing perf.data recorded on another machine.
We have to somehow tell perf_evlist__tty_browse_hists that it should try to figure out the name of the event by looking at _either_ /sys (local events) or what came in the perf.data file.
That is because 'perf top' and 'perf report' uses perf_evlist__tty_browse_hists. One is for local events (top) and the other for perf.data files, that may or not be for local (in the sense of running the same kernel for record + report) or for "remote" (running on the same machine but with a different kernel at record than the one used at report) or from a different machine altogether, perhaps even different arch.
I just thought that we should always consider the remote case first and falls back to local case because if we looked for local events, the remote events (perf.data) would not exist so that it can falls to the local case safely.
Now I think that we need a session method to check whether the current session is local or remote, and acts something based on that info.
We just need to get the data from the perf.data file as early as possible, i.e. just after processing the perf.data headers, like in the attached patch.
Dmitry, can you please try it?
- Arnaldo
-
Arnaldo Carvalho de Melo -
Dmitry Antipov -
Namhyung Kim