Re: [PATCH 00/34] perf clang: Builtin clang and perfhook support
From: Wangnan (F)
Date: Tue Nov 15 2016 - 00:38:38 EST
On 2016/11/15 13:21, Alexei Starovoitov wrote:
On Mon, Nov 14, 2016 at 9:03 PM, Wangnan (F) <wangnan0@xxxxxxxxxx> wrote:
On 2016/11/15 12:57, Alexei Starovoitov wrote:
On Mon, Nov 14, 2016 at 8:05 PM, Wang Nan <wangnan0@xxxxxxxxxx> 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.