Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755441AbcKOFif (ORCPT ); Tue, 15 Nov 2016 00:38:35 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:27591 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752423AbcKOFie (ORCPT ); Tue, 15 Nov 2016 00:38:34 -0500 Subject: Re: [PATCH 00/34] perf clang: Builtin clang and perfhook support To: Alexei Starovoitov References: CC: Arnaldo Carvalho de Melo , Alexei Starovoitov , Zefan Li , He Kuang , "linux-kernel@vger.kernel.org" , pi3orama , Jiri Olsa From: "Wangnan (F)" Message-ID: <582A9F0D.30607@huawei.com> Date: Tue, 15 Nov 2016 13:37:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: 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: 1994 Lines: 49 On 2016/11/15 13:21, Alexei Starovoitov wrote: > On Mon, Nov 14, 2016 at 9:03 PM, Wangnan (F) wrote: >> >> On 2016/11/15 12:57, Alexei Starovoitov wrote: >>> On Mon, Nov 14, 2016 at 8:05 PM, Wang Nan wrote: >>>> This is version 2 of perf builtin clang patch series. Compare to v1, >>>> add an exciting feature: jit compiling perf hook functions. This >>>> features allows script writer report result through BPF map in a >>>> customized way. >>> looks great. >>> >>>> SEC("perfhook:record_start") >>>> void record_start(void *ctx) >>>> { >>>> int perf_pid = getpid(), key = G_perf_pid; >>>> printf("Start count, perfpid=%d\n", perf_pid); >>>> jit_helper__map_update_elem(ctx, &GVALS, &key, &perf_pid, 0); >>> the name, I think, is too verbose. >>> Why not to keep them as bpf_map_update_elem >>> even for user space programs? >> >> I can make it shorter by give it a better name or use a wrapper like >> >> BPF_MAP(update_elem) > the macro isn't pretty, since function calls won't look like calls. > >> but the only thing I can't do is to make perfhook and in-kernel script >> use a uniform name for these bpf_map functions, because >> bpf_map_update_elem is already defined: >> >> "static long (*bpf_map_update_elem)(void *, void *, void *, unsigned long) = >> (void *)2;\n" > right. i guess you could have #ifdef it, so it's different for bpf backend > and for native. Then the '.c' -> LLVM IR compiling should be done twice for BPF and for JIT to make the macro work. In current implementation we have only one LLVM IR. It is faster and can make sure the data layout ("maps" section) is identical. > Another alternative is to call it map_update_elem or map_update > or bpf_map_update. Something shorter is already a win. > 'jit_helper__' prefix is an implementation detail. The users don't > need to know and don't need to spell it out everywhere. Good. Let choose a better name for them. Thank you.