Message-ID: <5549914F.9070808@huawei.com>
Date: Wed, 6 May 2015 11:58:07 +0800
From: Wang Nan <wangnan0@huawei.com>
User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.0.1
MIME-Version: 1.0
To: Alexei Starovoitov <ast@plumgrid.com>, He Kuang <hekuang@huawei.com>,
        <davem@davemloft.net>, <acme@kernel.org>, <mingo@redhat.com>,
        <a.p.zijlstra@chello.nl>, <masami.hiramatsu.pt@hitachi.com>,
        <jolsa@kernel.org>
CC: <linux-kernel@vger.kernel.org>, <pi3orama@163.com>
Subject: Re: [RFC PATCH 0/6] perf bpf: Probing with local variable
References: <1430820612-84443-1-git-send-email-hekuang@huawei.com> <554944C1.3060705@plumgrid.com>
In-Reply-To: <554944C1.3060705@plumgrid.com>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2460
Lines: 54

On 2015/5/6 6:31, Alexei Starovoitov wrote:
> On 5/5/15 3:10 AM, He Kuang wrote:
>> This patch set is based on https://lkml.org/lkml/2015/4/30/264
>>
>> By using bpf 'config' section like this:
>>
>>    char _config2[] SEC("config") = "generic_perform_write=generic_perform_write+122 file->f_mapping->a_ops bytes offset";
>>    SEC("generic_perform_write")
>>    int NODE_generic_perform_write (struct pt_regs *ctx, void *a_ops, void *bytes, void* offset) {
>>            char fmt[] = "NODE_generic_perform_write, a_ops=%p, bytes=%p, offset=%p\n";
>>            bpf_trace_printk(fmt, sizeof(fmt), a_ops, bytes, offset);
>>            return 1;
>>    }
>>
>> In this example, 'bytes' and 'offset' are local variables, a_ops is in
>> the structure field of file parameter, and we probe in the body of the
>> generic_perform_write() function.
>>
>> Perf can fetch and convert all the arguments and then we translate them
>> into bpf bytecode as a prologue before calling bpf body functions. In
>> the prologue, we fetch arguments from bpf context register and place
>> them according to bpf calling conventions so the body function can
>> access them as formal parameters.
> 
> great idea! Like it a lot.
> Looking at current implementation I think the limit is <=3 arguments,
> which I think is perfectly fine for now. Just worth mentioning in
> the doc.
> 
> Two high level comments:
> - can you collapse SEC("config") with SEC("func_name") ?
> It seems that "func_name" is only used as reference inside "config".
> I understand that you're proposing one "config" section where multiple
> descriptions are strcat together, but why? Something like:
> SEC("kprobe/generic_perform_write+122(file->f_mapping->a_ops, bytes, offset)")
> int func(...) { ... }
> should be enough and more concise.
> 

Is it possible to use such a long section name? I introduce 'config' section
since it contains C strings so I can put things to it freely. By using macro trick,
we can still use not very complex code to describe probing position like this:

#define PROBE(name, config) \
SEC("config") char name##_config[] = #name config ; \
SEC(#name)
PROBE(generic_perform_write, "kprobe: +122(file->f_mapping->a_ops, bytes, offset)")


--
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/