Message-ID: <555A70D9.1000709@hitachi.com>
Date:	Tue, 19 May 2015 08:08:09 +0900
From:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To:	He Kuang <hekuang@...wei.com>, paulus@...ba.org,
	a.p.zijlstra@...llo.nl, mingo@...hat.com, acme@...nel.org,
	namhyung@...nel.org, jolsa@...nel.org, dsahern@...il.com,
	ast@...mgrid.com, daniel@...earbox.net, brendan.d.gregg@...il.com
CC:	wangnan0@...wei.com, lizefan@...wei.com,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 0/5] Fetching local variables for bpf prog

On 2015/05/18 14:30, He Kuang wrote:
> This patch is based on https://lkml.org/lkml/2015/5/17/84 (perf tools:
> introduce 'perf bpf' command to load eBPF programs).
> 
> Previous discusions on perf bpf: Probing with local variable:
> https://lkml.org/lkml/2015/5/5/260. In that patch, we tried to
> generate a bpf bytecode prologue in perf, this prologue fetches and
> places variables as bpf function parameters, for making it easier to
> fetch variables in bpf prog.
> 
> Alexei's comments:
> 
>  - Argument limitation is <=3, which is OK but should be documented.
>  - Support it without debug info when kprobe is placed at the top
>    of the function.
>  - Concise the 'config' section.
> 
> Masami has metioned:
> 
>  - The redundant functionality of both userspace and kernel variable
>    parsing.
>  - The possibility of replacing the old fetch_arg functions with these
>    byte code
> 
> I've made a new version of userspace prologue which fixes the problems
> in that RFC series(not sent yet), but when trying to resolve Alexei's
> 2nd suggestion, we found it is in contradiction to the argument number
> limitation. By a rough statistics, there're 13.5 percent fucntions
> have 4 or more arguments in kernel. BPF calling convention limits the
> maximum number of argument number to 5(R1~R5), besides the R1 for
> 'ctx', there're 4 registers left for arguments passing. It is not
> reasonable to pass the first 4 arguments when probing a function which
> has more than 4 arguments.
> 
> Consider Masami's suggestion to do the work in kernel, we found that
> adding a helper proto-type function for fetching bpf variables is a
> more easier way to reach our goals. Embed trace_probe pointer to 'ctx'
> for bpf prog, then we can use the existing code for fetching args in
> kernel. Just like the 2nd suggestion, but here we do not generate any
> bytecode, but use the existing call_fetch() results directly. Example
> code can be found in [RPF PATCH 5/5].

Hmm, what I suggested was that optimizing call_fetch methods with BPF,
yours seems opposite. Since BPF can be optimized by x86 native instructions
by using JIT, it is much faster than current call-chain fetch method.
I'm still not sure all the fetch method can be covered with BPF, e.g.
fetching a bitfield requires bitmasks and bitshift ops.

Thank you,

> 
> Moreover, this method removes the argument number limitation caused by
> bpf calling convention(R2-R5 for placing variables). And leaves the
> users free to decide whether or not do the arguments/variables
> fetching. They can use this helper function in their own conditions.
> 
> Also need to note:
> 
>  - We can generate a syntax sugar which can convert the 'structure
>    param' to function args, this can reduce the users' extra work.
>  - An extra verification needs to be implemented to be sure that user
>    provides enough space for arguments fetching.
> 
> This method's pros & cons:
> 
> pros:
>  - Remove arugment number limitation. 
>  - User free to choose whether or not do the fetch and decide where to
>    execute the fetch.
>  - Remove kernel/userspace redundant functionality of parsing args.
> 
> cons:
>  - User should add the 'structure param' code themselves.
> 
> Looking forward for disscusions.
> 
> He Kuang (5):
>   perf bpf: Add -k option for testing convenience
>   bpf: Pass trace_probe to bpf_prog for variable fetching
>   bpf: Add helper function for fetching variables at probe point
>   samples/bpf: Add proper prefix to objects in Makefile
>   samples/bpf: Add sample for testing bpf fetch args
> 
>  include/uapi/linux/bpf.h            |  1 +
>  kernel/trace/bpf_trace.c            | 38 ++++++++++++++++++++++++++++++++
>  kernel/trace/trace_kprobe.c         | 11 ++++++++--
>  kernel/trace/trace_probe.h          |  5 +++++
>  samples/bpf/Makefile                |  3 ++-
>  samples/bpf/bpf_helpers.h           |  2 ++
>  samples/bpf/sample_bpf_fetch_args.c | 43 +++++++++++++++++++++++++++++++++++++
>  tools/perf/builtin-bpf.c            |  3 +++
>  8 files changed, 103 insertions(+), 3 deletions(-)
>  create mode 100644 samples/bpf/sample_bpf_fetch_args.c
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@...achi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists