Messages in this thread Patch in this message |  | | Date | Mon, 30 Jan 2012 16:55:32 -0200 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel |
| |
Em Sun, Jan 29, 2012 at 05:55:52PM +0900, Namhyung Kim escreveu: > If multiple events are specified on old kernel, > perf_evlist__id2evsel() returns NULL for non-sampling events > since the sample.id doesn't contain valid value, and it triggers > assert below. If only one event is given, the function returns > the evsel regardless of sample.id, this is why most case cause > no problem on old kernel. > > Fix it by using active evsel.
How about this one instead:
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index a6d50e3..3ffb320 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -349,6 +349,10 @@ struct perf_evsel *perf_evlist__id2evsel(struct perf_evlist *evlist, u64 id) hlist_for_each_entry(sid, pos, head, node) if (sid->id == id) return sid->evsel; + + if (!perf_evlist__sample_id_all(evlist)) + return list_entry(evlist->entries.next, struct perf_evsel, node); + return NULL; }
That way we won't have to patch other tools that would get the same problem in such new tool/old kernel combos, right? Do you see any problem with not checking the header type?
Also please try to avoid using perf_session fields, prefer to use perf_{evlist,evsel} when possible, in this case you could use perf_evlist__sample_id_all(evlist), for instance. - Arnaldo
> Signed-off-by: Namhyung Kim <namhyung@gmail.com> > --- > tools/perf/builtin-top.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index e8b033c074f9..f68fba52c8d8 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -771,6 +771,14 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx) > } > > evsel = perf_evlist__id2evsel(session->evlist, sample.id); > + if (evsel == NULL && !session->sample_id_all && > + event->header.type != PERF_RECORD_SAMPLE) { > + /* > + * Old kernel, no sample_id_all field. > + * Just use active evsel. > + */ > + evsel = top->sym_evsel; > + } > assert(evsel != NULL); > > origin = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK; > -- > 1.7.8.2
|  |