Date: Tue, 19 May 2015 18:10:33 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Alexei Starovoitov <ast@plumgrid.com>
Cc: Namhyung Kim <namhyung@kernel.org>, Wang Nan <wangnan0@huawei.com>,
        paulus@samba.org, a.p.zijlstra@chello.nl, mingo@redhat.com,
        jolsa@kernel.org, dsahern@gmail.com, daniel@iogearbox.net,
        brendan.d.gregg@gmail.com, masami.hiramatsu.pt@hitachi.com,
        lizefan@huawei.com, linux-kernel@vger.kernel.org, pi3orama@163.com
Subject: Re: [RFC PATCH v3 00/37] perf tools: introduce 'perf bpf' command to
 load eBPF programs.
Message-ID: <20150519211033.GF19921@kernel.org>
References: <1431860222-61636-1-git-send-email-wangnan0@huawei.com>
 <555A3FC2.8060805@plumgrid.com>
 <20150518204416.GJ18563@kernel.org>
 <555A541F.6090606@plumgrid.com>
 <20150518212013.GB13946@kernel.org>
 <555A5D96.2070509@plumgrid.com>
 <20150519134458.GC13946@kernel.org>
 <20150519160448.GD29162@danjae.kornet>
 <20150519164040.GB19921@kernel.org>
 <555BA112.6090505@plumgrid.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <555BA112.6090505@plumgrid.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4504
Lines: 118

Em Tue, May 19, 2015 at 01:46:10PM -0700, Alexei Starovoitov escreveu:
> On 5/19/15 9:40 AM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, May 20, 2015 at 01:04:48AM +0900, Namhyung Kim escreveu:
> >>If we go with 'perf record' rather than 'perf bpf record', I agree
> >>that --event option is more natural than --filter.  The --event option
> >>says that it will record - or enable, at least - a (kprobe) event for
> >>bpf programs in it and then do something with it. :)
> >>
> >>Maybe something like this?
> >>
> >>   perf record --event bpf:/path/to/object
> >
> >The syntax maybe one of many, say if it sees a ".o" suffix in the even
> >name, look if the provided event name is a file and if this file has the
> >ELF header, whatever.
> 
> agree. 'bpf:' prefix is redundant.

I would say unnecessarily exposing an implementation detail :-)

> To me the following syntax is fine:
> perf record --event bpf_file.o

Agreed, for something pre-compiled.

> In the future it can support automatically:
> perf record --event bpf_file.c

Right, to compile it, then use the resulting bpf_file.o just like in the
first case, then, on another patch, cache it and next time just check
that the contents of the file hasn't changed, so the .o file can be
used, i.e. ccache like stuff.
 
> Wang, thoughts?
> 
> >>Oh, this looks like an interesting approach.. are you saying something
> >>like below?
> >
> >No, those are way too many steps :-)
> >
> >What 'perf script' does? Right now you can ask for a script to run and
> >it will both start 'perf record' with the proper events, and then
> >"immediately" consume it, piping the output of the 'record' "script" to
> >the consumer, that is 'perf script' itself running an interpreter, perl
> >or python.
> 
> if you're proposing to do something like:
> perf script bpf_file.c
> that will do event creation, filtering, aggregation, reporting
> and printing results, then it's fine.
> This is pretty much what I thought 'perf bpf run' will do.

Agreed, that is what I think should be done, parts of what is in
bpf.file.c are related to the data collection, some are for filtering,
and parts are for reporting, etc.

This all should use infrastructure in perf for symbol resolving,
callcahins, etc.
 
> >I.e. the first part, say, failed-syscalls-record, would be done
> >internally, loading the bpf object, etc, the second part would be the
> >event massaging, but done in a C subset :-)
> 
> not sure that's doable. The task (perf) that loaded the program in the

Well,perhaps in this part I wasn't clear, but what I think should be
fone is what is above, i.e. end goal should be "perf script bpf_file.c"
does it all.

> step one should be still alive when 2nd part is running.
> For 'perf bpf run' use case, the whole record/report split is
> artificial. There is no need for perf.data.

Right, but if we do a:

perf script -i perf.data bpf_file.c 

Then there would be a short circuit effect of just doing the
aggregation and/or reporting.

> In my mind 'perf bpf run file.c' suppose to quickly compile .c,
> hook into kernel, collect and print something, then Ctrl-C of
> 'perf bpf run' should automatically stop everything.

That is what I thought about 'perf script file.c'. That would be stopped
either because it finishes due to some condition in the kernel, in the
reporting or by means of control+C, either by design or because it is
taking too long, etc.

> No perf.data anywhere. It should be quick single step
> tool for live debugging.

Well, we could have a tee like mode where perf.data file could be
generated so that we could run it again after doing some change on the
aggregation code, so that we wouldn't have to re-run the data collection
parts, that could be about some condition hard to capture, etc.

> At the same time 'perf record --event bpf_file.o' plus generic
> 'perf report' model works very well as well.
> They are different use cases.

Right.
 
> I guess I'm saying let's not get too much ahead of ourselves ;)

No problem, but then we need to talk at this moment not to add new stuff
that we think should not be added, like 'perf bpf' :-)

> I think Wang's current patchset is close enough to make
> 'perf record --event bpf_file.o' to be useful.
> Let's take this first step.

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