From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757910AbbGHBbQ (ORCPT ); Tue, 7 Jul 2015 21:31:16 -0400 Received: from mail-pa0-f51.google.com ([209.85.220.51]:34977 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757081AbbGHBbK (ORCPT ); Tue, 7 Jul 2015 21:31:10 -0400 Message-ID: <559C7D5B.3040009@plumgrid.com> Date: Tue, 07 Jul 2015 18:31:07 -0700 From: Alexei Starovoitov User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: He Kuang , rostedt@goodmis.org, masami.hiramatsu.pt@hitachi.com, acme@kernel.org, a.p.zijlstra@chello.nl, mingo@redhat.com, namhyung@kernel.org, jolsa@kernel.org CC: wangnan0@huawei.com, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v3 0/2] Make eBPF programs output data to perf event References: <1436269386-72037-1-git-send-email-hekuang@huawei.com> In-Reply-To: <1436269386-72037-1-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 List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/7/15 4:43 AM, He Kuang wrote: > Hi, > > The two previous versions tried to combine bpf output data with the > sample event of the attached kprobe point, which leads to problems > about perf_trace_buf. > > After discussion we found it's not necessary to combine those two > parts of information, even we do not need the orignial kprobe output > event at all. Based on this idea, the implementation becomes simple, > just like what perf do with ftrace:functions, we set up a bpf ftrace > entry for perf tools to poll and collect data on it, eBpf program use > a helper function to submit data to ring-buffer, that's all. This > implementation also leaves all issues such as sample-types to perf > commandline. > > Currently, we just use raw data in the format fields to not interfere > perf sample parser, because the raw-data can be parsed by perf script > plugin easily. Looks much better! In general I think splitting it into two patches is confusing, since 1st patch is meaningless without 2nd. I would squash it. Other comments inline. > bpf_output_sample(&del_time, sizeof(del_time)); typo? You meant bpf_output_data(&del_time, sizeof(del_time), ctx) ? To match the rest of helpers, please make ctx to be first argument. Also I think bpf_output_trace_data() name is better. bpf_output_data name doesn't indicate that it's tracing only helper and might be confusing with networking helpers. > Record bpf events: > > $ perf record -e ftrace:bpf -e sample.o -- dd if=/dev/zero of=test bs=4k count=3 > > The results showed in perf-script: > > $ perf script > dd 994 [000] 166.686779: ftrace:bpf: 8: (000000000542b426, ...) > dd 994 [000] 166.686779: ftrace:bpf: 8: (00000000001011ef, ...) > dd 994 [000] 166.686779: ftrace:bpf: 8: (000000000007a2b6, ...) nice!