All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.