Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753248AbbF0LRj (ORCPT ); Sat, 27 Jun 2015 07:17:39 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:6713 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753094AbbF0LRc (ORCPT ); Sat, 27 Jun 2015 07:17:32 -0400 Message-ID: <558E85CE.80408@huawei.com> Date: Sat, 27 Jun 2015 19:15:26 +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: Masami Hiramatsu , , , , , , , , , , CC: , , , , , Subject: Re: [RFC PATCH 1/3] perf probe: Init symbol as kprobe if any pev is kprobe References: <1435228645-116055-1-git-send-email-wangnan0@huawei.com> <1435228645-116055-2-git-send-email-wangnan0@huawei.com> <558E50C2.2030807@hitachi.com> <558E520E.6070301@huawei.com> <558E5F42.1060705@hitachi.com> In-Reply-To: <558E5F42.1060705@hitachi.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 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2966 Lines: 94 On 2015/6/27 16:30, Masami Hiramatsu wrote: > Hi Wang, > > On 2015/06/27 16:34, Wangnan (F) wrote: >> >> On 2015/6/27 15:29, Masami Hiramatsu wrote: >>> On 2015/06/25 19:37, Wang Nan wrote: >>>> Before this patch, add_perf_probe_events() init symbol maps only for >>>> uprobe if the first pev passed to it is a uprobe event. However, with >>>> the incoming BPF uprobe support, now it will be possible to pass an >>>> array with combined kprobe and uprobe events to add_perf_probe_events(). >>> This description is not correct. Actually, add_perf_probe_events already >>> supports mix of uprobes and kprobes. However, from the command line syntax >>> constrains the first elements of the probe_event arrays must be kprobes. >>> So, if the array starts with uprobes, no kprobes should be there. >>> >>>> This patch check all pevs instead of the first one, and init kernel >>>> symbol if any events is not uprobe. >>> Anyway, I prefer to call init_symbol_maps() with "false" :) >> I also prefer "false", so I try to find whether all events are uprobe >> instead >> of init_symbol_maps(true). >> >> So for this patch only commit message needs to be corrected, the code is >> no problem, right? >> > No, I meant you just need to change one line, as below. > > - ret = init_symbol_maps(pevs->uprobes); > + ret = init_symbol_maps(false); > > That's enough, I don't like to introduce a bool flag and loop. > > Thanks, > Then basic structure of kernel map will be build even if all probe points are uprobes. It costs some memory. But sure, we can skip the loop. Will post an updated version. Thank you. > >> Thank you. >> >>> Thank you, >>> >>>> Signed-off-by: Wang Nan >>>> --- >>>> tools/perf/util/probe-event.c | 15 ++++++++++++++- >>>> 1 file changed, 14 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >>>> index b386d2f..a2b3026 100644 >>>> --- a/tools/perf/util/probe-event.c >>>> +++ b/tools/perf/util/probe-event.c >>>> @@ -2802,8 +2802,21 @@ int cleanup_perf_probe_event(struct perf_probe_event *pev) >>>> int add_perf_probe_events(struct perf_probe_event *pevs, int npevs, bool cleanup) >>>> { >>>> int i, ret; >>>> + bool user_only = true; >>>> >>>> - ret = init_symbol_maps(pevs->uprobes); >>>> + /* If any pev is kprobe, init kernel symbols. */ >>>> + for (i = 0; i < npevs; i++) { >>>> + if (!pevs[i].uprobes) { >>>> + user_only = false; >>>> + break; >>>> + } >>>> + } >>>> + >>>> + /* >>>> + * Compiler can drop user_only: >>>> + * ret = init_symbol_maps(i >= npevs); >>>> + */ >>>> + ret = init_symbol_maps(user_only); >>>> if (ret < 0) >>>> return ret; >>>> >>>> >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/