From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755441AbcKOFif (ORCPT <rfc822;w@1wt.eu>);
        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
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 15 Nov 2016 00:38:34 -0500
Subject: Re: [PATCH 00/34] perf clang: Builtin clang and perfhook support
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
References: <CAADnVQL=_vPKSK2t5iU7P=wqR6VzD0T-WJ1pY21pcDKKEvw+vQ@mail.gmail.com>
CC: Arnaldo Carvalho de Melo <acme@redhat.com>,
        Alexei Starovoitov <ast@fb.com>, Zefan Li <lizefan@huawei.com>,
        He Kuang <hekuang@huawei.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        pi3orama <pi3orama@163.com>, Jiri Olsa <jolsa@kernel.org>
From: "Wangnan (F)" <wangnan0@huawei.com>
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: <CAADnVQL=_vPKSK2t5iU7P=wqR6VzD0T-WJ1pY21pcDKKEvw+vQ@mail.gmail.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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org



On 2016/11/15 13:21, Alexei Starovoitov wrote:
> On Mon, Nov 14, 2016 at 9:03 PM, Wangnan (F) <wangnan0@huawei.com> wrote:
>>
>> On 2016/11/15 12:57, Alexei Starovoitov wrote:
>>> On Mon, Nov 14, 2016 at 8:05 PM, Wang Nan <wangnan0@huawei.com> 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.