From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755922AbbIAKuv (ORCPT ); Tue, 1 Sep 2015 06:50:51 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:41703 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755370AbbIAKuu (ORCPT ); Tue, 1 Sep 2015 06:50:50 -0400 Message-ID: <55E57FF4.1010408@huawei.com> Date: Tue, 1 Sep 2015 18:37:40 +0800 From: "Wangnan (F)" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Arnaldo Carvalho de Melo , Jiri Olsa CC: , , , , , Masami Hiramatsu , Namhyung Kim Subject: Re: [PATCH 02/31] perf tools: Don't set cmdline_group_boundary if no evsel is collected References: <1440822125-52691-1-git-send-email-wangnan0@huawei.com> <1440822125-52691-3-git-send-email-wangnan0@huawei.com> <20150831192003.GA2443@redhat.com> In-Reply-To: <20150831192003.GA2443@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.66.109] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020205.55E582ED.01F7,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 7b33d467086871a86d03938cbd9d8a02 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> Cc: Alexei Starovoitov >> Cc: Masami Hiramatsu >> Cc: Namhyung Kim >> Cc: Zefan Li >> Cc: pi3orama@163.com >> Cc: Arnaldo Carvalho de Melo >> 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