Messages in this thread | | | Date | Tue, 31 Jan 2012 09:31:13 +0900 | From | Namhyung Kim <> | Subject | Re: [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel |
| |
2012-01-31 3:55 AM, Arnaldo Carvalho de Melo wrote: > 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? >
Right, looks good to me.
> Do you see any problem with not checking the header type? >
No, I don't, at least for now :)
> 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 >
OK, will do that later.
Thanks, Namhyung
| |