Messages in this thread Patch in this message | | | Date | Tue, 1 Sep 2015 18:37:40 +0800 | From | "Wangnan (F)" <> | Subject | Re: [PATCH 02/31] perf tools: Don't set cmdline_group_boundary if no evsel is collected |
| |
On 2015/9/1 3:20, Arnaldo Carvalho de Melo wrote: > Em Sat, Aug 29, 2015 at 04:21:36AM +0000, Wang Nan escreveu: >> If parse_events__scanner() collects no entry, perf_evlist__last(evlist) >> is invalid. Then setting of cmdline_group_boundary touches invalid. >> >> It could happend in currect BPF implementation. See [1]. Although it >> can be fixed, for safety reason it whould be better to introduce this >> check. >> >> Instead of checking number of entries, check data.list instead, so we >> can add dummy evsel here. > Event parsing fixes should have Jiri Olsa on the CC list, Jiri, is this > ok? > > From what I can see it looks Ok, my question, just from looking at this > patch, is if it is valid to get to this point with an empty data.list, > i.e. was it ever possible and this is a bug irrespective of eBPF?
It should not be a existing bug in perf. There are other places rely on non-empty of the list. For example, in parse_events__set_leader(). Furtunately, it won't triggered problem because we don't allow a BPF object to be wrapped with "{}" lexically ("{./aaa.o}" will be interpreterd as file '{./aaa.o' and a extra '}').
> - Arnaldo > >> [1]: http://lkml.kernel.org/n/1436445342-1402-19-git-send-email-wangnan0@huawei.com >> >> Signed-off-by: Wang Nan <wangnan0@huawei.com> >> Cc: Alexei Starovoitov <ast@plumgrid.com> >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Zefan Li <lizefan@huawei.com> >> Cc: pi3orama@163.com >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> >> Link: http://lkml.kernel.org/r/1440742821-44548-3-git-send-email-wangnan0@huawei.com >> --- >> tools/perf/util/parse-events.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c >> index d826e6f..14cd7e3 100644 >> --- a/tools/perf/util/parse-events.c >> +++ b/tools/perf/util/parse-events.c >> @@ -1143,10 +1143,14 @@ int parse_events(struct perf_evlist *evlist, const char *str, >> int entries = data.idx - evlist->nr_entries; >> struct perf_evsel *last; >> >> + if (!list_empty(&data.list)) { >> + last = list_entry(data.list.prev, >> + struct perf_evsel, node); >> + last->cmdline_group_boundary = true; >> + } >> + >> perf_evlist__splice_list_tail(evlist, &data.list, entries); >> evlist->nr_groups += data.nr_groups; >> - last = perf_evlist__last(evlist); >> - last->cmdline_group_boundary = true; >> >> return 0; >> } >> -- >> 2.1.0
| |