Message-ID: <5571636F.1020403@huawei.com>
Date:	Fri, 5 Jun 2015 16:53:03 +0800
From:	"Wangnan (F)" <wangnan0@...wei.com>
To:	Ingo Molnar <mingo@...nel.org>,
	Alexei Starovoitov <ast@...mgrid.com>
CC:	pi3orama <pi3orama@....com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Brendan Gregg <brendan.d.gregg@...il.com>,
	Daniel Borkmann <daniel@...earbox.net>,
	David Ahern <dsahern@...il.com>,
	"He Kuang" <hekuang@...wei.com>, Jiri Olsa <jolsa@...hat.com>,
	Kaixu Xia <xiakaixu@...wei.com>,
	Madhavan Srinivasan <maddy@...ux.vnet.ibm.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>,
	Zefan Li <lizefan@...wei.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Alexei Starovoitov <alexei.starovoitov@...il.com>
Subject: Re: [GIT PULL 0/6] perf/core improvements and fixes



On 2015/6/5 14:41, Ingo Molnar wrote:
> * Alexei Starovoitov <ast@...mgrid.com> wrote:
>
>> On 6/4/15 7:04 AM, Ingo Molnar wrote:
>>>>>   # perf record -e bpf_source.c cmdline
>>>>>
>>>>>   to create a eBPF filter from source,
>>>>>
>>>>> Use
>>>>>
>>>>> # perf record -e bpf_object.o cmdline
>>>>>
>>>>> to create a eBPF filter from object intermedia.
>>>>>
>>>>> Use
>>>>>
>>>>> # perf bpf compile bpf_source.c --kbuild=kernel-build-dir -o bpf_object.o
>>>>>
>>>>> to create the .o
>>>>>
>>>>> I think this should be enough. Currently only the second case has been implemented.
>>> So if users cannot actually generate .o files then it's premature to merge
>>> this in such an incomplete form!
>>>
>>> It should be possible to use a feature that we are merging.
>> of course it's usable :) There is some confusion here.
>> To compile .c into .o one can easily use
>> clang -O2 -emit-llvm -c file.c -o - | llc -march=bpf -o file.o
> There's no confusion here: you guys are trying to sell me what at this stage is
> incomplete and hard to use, and I'm resisting it as I should! :-)
>
> We also have different definitions of 'easily'. It might be 'easy' to type:
>   
>      clang -O2 -emit-llvm -c file.c -o - | llc -march=bpf -o file.o
>
> ... for some tooling developer intimate with eBPF, but to the first time user who
> found an interesting looking eBPF scriptlet on the net or in the documentation and
> wants to try his luck? It's absolutely non-obvious!
>
> The current usage to get a _minimal_ eBPF script running is non-obvious and
> obscure to the level of being a show stopper.
>
> I don't understand why you guys are even wasting time arguing about it: it's not
> that hard to auto-build from source code. It's one of the basic features of
> tooling. If you ever built perf you'll know that typing 'make install' will type
> in all those quirky build lines automatically for you, without requiring you to
> perform any other step, no matter how trivial.
>
> Doubly annoying, you seem to have the UI principles wrong, you seem to think that
> a .o is a proper user interface. It absolutely is _not_ okay.
>
> The Linux kernel project and as an extension the perf project deals with source
> code, and I'm 100% suspicious of approaches that somehow think that .o objects are
> the right UI for _anything_ except temporary files that sometimes show up in
> object directories...
>
> Fix the 'newbie user' UI flow as a _first_ priority, not as a second thought!
>
> Every single quirky line or nonsensical option you require a first time user to
> type halves the number of new users we'll get. You need to understand why dtrace
> is so popular:
>
>     - it's bloody easy to use
>
>     - it's a safe environment you can deploy in critical environments
>
>     - it's flexible
>
>     - instrumentation hacks are very easy to share
>
> eBPF based scripting got 3 out of those 4 right, but please don't forget item 1
> either, because without that we have nothing but a bunch of unusable functionality
> in the kernel and in tooling that benefits only very few people. Okay?
>
>> So I think we need to support both 'perf record -e file.[co]'
> Why do you even need to ask? Of course!
>
> Think through how users will meet eBPF scripts and how they will interact with
> them:
>
>    - they'll see or download an eBPF scriptlet somewhere and will have a .c file.
>
>    - ideally there will be built-in eBPF scriptlets just like we have tracing
>      plugins, and there's a good UI to query them and see their description and
>      source code.
>
>    - then they will want to use it all with the minimum amount of fuss
>
>    - they don't care how the eBPF scriptlet gets to the kernel: whether the kernel
>      can read and build the .c files, or whether there's some user tooling that
>      turns it into bytecode. Most humans don't read bytecode!
>
>    - they will absolutely not download random .o's and we should not encourage that
>      in any case - these things should be source code based.
>
> These things compile in an eye blink, there's very little reason to ever deal with
> a .o, except some weird and rare usecases...
>
> In fact I'm NAK-ing the whole .o based interface until the .c interface is made
> the _primary_ one and works well and until I see that you have thought through
> basic usability questions...

OK. Let's start making a nice UI.

At this stage, what about wrapping current clang and llc workflow into perf,
let it call them to compile '.c' scripts? This is the way 'perf 
annotate' using
objdump. I can do this job, but firstly I'd like to know people's opinion on
it, and the value of the wrapper if Alexei Starovoitov's dynamic compiler
shared object is coming.

Following functions will be added:

  - perf searches clang and llc under current $PATH,

  - Users are allowed to pass the position of those programs,
    if perf failed to find the automatically,

  - Users are allowed to pass extra compiling options to clang and llc, like
    include directories,

  - 'perf record' automatically calls them if a '.c' is passed using 
'--event'.

  - 'perf bpf compile' command will be added to compile a '.c' find into 
'.o',

Further, basic header files should be shipped with kernel headers.

User interface update:

  - --llc, --clang, --llc-opt and --clang-opt option will be added to 
'perf record'
    to indicate the position and extra options to them. Ideally they can 
be leave
    blank.

  - 'perf bpf compile' will be added.

One problem I can find is that, the wrapper will make perf depend on 
llvm. I don't
think the compiler will be deployed in production environments... And 
also, the
embedded case...

Any suggestion? Do you think the above idea is on the right way?

Thanks.

> Thanks,
>
> 	Ingo


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists