Date: Mon, 18 May 2015 17:44:16 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Alexei Starovoitov <ast@plumgrid.com>
Cc: Wang Nan <wangnan0@huawei.com>, paulus@samba.org, a.p.zijlstra@chello.nl,
        mingo@redhat.com, namhyung@kernel.org, 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: <20150518204416.GJ18563@kernel.org>
References: <1431860222-61636-1-git-send-email-wangnan0@huawei.com>
 <555A3FC2.8060805@plumgrid.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <555A3FC2.8060805@plumgrid.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2941
Lines: 77

Em Mon, May 18, 2015 at 12:38:42PM -0700, Alexei Starovoitov escreveu:
> On 5/17/15 3:56 AM, Wang Nan wrote:
> >This is the 3rd version of 'perf bpf' patch series, based on
> >v4.1-rc3.
> >
> >The goal of this series of patches is to integrate eBPF with perf.
> >After applying these patches, users are allowed to use following
> >command to load eBPF program compiled by LLVM into kernel then start
> >recording with filters on:
> >
> >  # perf bpf record --object sample_bpf.o -- -a sleep 4
> 
> I think using programs are sophisticated filters is a good start
> and are useful already. Let's focus on that at the moment.
> I wouldn't grow the patchset any bigger.

Right, I am just now trying to slowly get involved, and my first
impression would be like that, i.e. we have:

perf record --filter, to pass a filter to tracepoints, if I could
instead of a filter expression pass, say, filter_bpf.o, that would seem
natural for me, i.e. no new option, just an alternative type of filter,
one way more powerful.

If i could write it as a C expression that would then get wrapped up as
a bpf, compiled, turned into an object, and then inserted in the kernel
to be used as my filter, then that would be almost like a tracepoint
filter.
 
> >Other than the previous change, v3 patch series drops the '|' event
> >syntax introduced in v2, because I realized that in v2 users are
> >allowed to pass any bpf fd by using it, like:
> >
> >  # perf bpf record -- -e sched:sched_switch|100| sleep 1

So, what was this supposed to achieve? What does 100 mean there?

> >
> >which may become trouble maker.
> 
> passing fd number as a string is an odd interface anyway.
> So I think that was the right call. We can improve it later.
> 
> >  Are we actually need a 'perf bpf' command? We can get similar result by
> >  modifying 'perf record' to make it load eBPF program before recording.
> >
> >  I suggest to keep 'perf bpf', group all eBPF stuffs together using a
> >  uniform entry. Also, eBPF programs can act not only as filters but also
> >  data aggregator. It is possible to make something link 'perf bpf run'
> >  to simply make it run, and dump result after user hit 'C-c' or timeout.
> 
> Though it's tempting to group under 'perf bpf'. I think it's cleaner to
> add --object flag to 'perf record'

I'd say keep it in --filter, that noticing it is a bpf object would
dtrt:

  perf record --filter bpf_thing.o usleep 1


> Since it will avoid unnecessary '--'.
> Unless we can drop it? Like
> perf bpf record --object sample_bpf.o -a sleep 4
> should work?
> If not, then the following is better:
> perf record --object sample_bpf.o -a sleep 4
> 
> Thank you for the hard work!

Ditto!

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