public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: He Kuang <hekuang@huawei.com>,
	paulus@samba.org, a.p.zijlstra@chello.nl, mingo@redhat.com,
	acme@kernel.org, namhyung@kernel.org, jolsa@kernel.org,
	dsahern@gmail.com, ast@plumgrid.com, daniel@iogearbox.net,
	brendan.d.gregg@gmail.com
Cc: wangnan0@huawei.com, lizefan@huawei.com, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/5] Fetching local variables for bpf prog
Date: Tue, 19 May 2015 08:08:09 +0900	[thread overview]
Message-ID: <555A70D9.1000709@hitachi.com> (raw)
In-Reply-To: <1431927047-35144-1-git-send-email-hekuang@huawei.com>

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@hitachi.com

      parent reply	other threads:[~2015-05-18 23:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-18  5:30 [RFC PATCH 0/5] Fetching local variables for bpf prog He Kuang
2015-05-18  5:30 ` [RFC PATCH 1/5] perf bpf: Add -k option for testing convenience He Kuang
2015-05-18  5:30 ` [RFC PATCH 2/5] bpf: Pass trace_probe to bpf_prog for variable fetching He Kuang
2015-05-18 19:43   ` Alexei Starovoitov
2015-05-18  5:30 ` [RFC PATCH 3/5] bpf: Add helper function for fetching variables at probe point He Kuang
2015-05-18 19:53   ` Alexei Starovoitov
2015-05-18  5:30 ` [RFC PATCH 4/5] samples/bpf: Add proper prefix to objects in Makefile He Kuang
2015-05-18 19:59   ` Alexei Starovoitov
2015-05-18  5:30 ` [RFC PATCH 5/5] samples/bpf: Add sample for testing bpf fetch args He Kuang
2015-05-18 23:08 ` Masami Hiramatsu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=555A70D9.1000709@hitachi.com \
    --to=masami.hiramatsu.pt@hitachi.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=ast@plumgrid.com \
    --cc=brendan.d.gregg@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=dsahern@gmail.com \
    --cc=hekuang@huawei.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=paulus@samba.org \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox