Message-ID: <555A4334.90706@plumgrid.com>
Date: Mon, 18 May 2015 12:53:24 -0700
From: Alexei Starovoitov <ast@plumgrid.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0
MIME-Version: 1.0
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, daniel@iogearbox.net,
        brendan.d.gregg@gmail.com, masami.hiramatsu.pt@hitachi.com
CC: wangnan0@huawei.com, lizefan@huawei.com, linux-kernel@vger.kernel.org,
        pi3orama@163.com
Subject: Re: [RFC PATCH 3/5] bpf: Add helper function for fetching variables
 at probe point
References: <1431927047-35144-1-git-send-email-hekuang@huawei.com> <1431927047-35144-4-git-send-email-hekuang@huawei.com>
In-Reply-To: <1431927047-35144-4-git-send-email-hekuang@huawei.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1444
Lines: 38

On 5/17/15 10:30 PM, He Kuang wrote:
> This helper function uses kernel structure trace_probe and related fetch
> functions for fetching variables described in 'SEC' to bpf stack.
>
> Signed-off-by: He Kuang <hekuang@huawei.com>
...
> +/* Store the value of each argument */
> +static void
> +bpf_store_trace_args(struct pt_regs *regs, struct trace_probe *tp,
> +		u8 *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < tp->nr_args; i++) {
> +		/* Just fetching data normally */
> +		call_fetch(&tp->args[i].fetch, regs,
> +			data + tp->args[i].offset);

that is slower than generating bpf by user space, but more importantly
that's invalid. There is no size check.
r2 in fetch_args points to stack, but nothing checks the stack limits.
You need to add code here to dynamically check it as well.
which will be adding runtime overhead as well.

Your first approach of generating argument accessors in user space
was better.
I think the limit of 3 or 4 arguments was fine.
We need to generate the code for non-debug case anyway,
like my earlier suggestion:
SEC("kprobe/generic_perform_write(void*, void*, long long)")
without debug info it will copy ctx->di into r2, ctx->si into r3
and ctx->dx into r4.

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