Message-ID: <20150731153522.GA16700@kernel.org>
Date:	Fri, 31 Jul 2015 12:35:22 -0300
From:	Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:	"Wangnan (F)" <wangnan0@...wei.com>
Cc:	He Kuang <hekuang@...wei.com>, Li Zefan <lizefan@...wei.com>,
	ast@...mgrid.com, pi3orama@....com, xiakaixu@...wei.com,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
	Jiri Olsa <jolsa@...hat.com>, David Ahern <dsahern@...il.com>,
	Namhyung Kim <namhyung@...nel.org>
Subject: perf eBPF patch ordering.  was: Re: perf test LLVM was: Re: [GIT
 PULL 00/39] perf tools: filtering events using eBPF programs

Em Wed, Jul 22, 2015 at 12:40:05PM +0800, Wangnan (F) escreveu:
> I noticed that you have rebase your perf/ebpf branch to 4.2-rc3. I have also
> done it in my tree. Please see:
> 
>  https://github.com/WangNan0/linux.git ebpf
> 
> Also, please recollect patch "perf tests: Add LLVM test for eBPF on-the-fly
> compiling", which adds verbose control. I'll also post it again.

I did it and some other rebases, expect more, but now I've got to:

  commit f0f99c45295e67787ad532492d9bebd6d9b56d3a
  Author: Wang Nan <wangnan0@...wei.com>
  Date:   Wed Jul 8 11:50:57 2015 +0000

    perf record: Enable passing bpf object file to --event

Which I split in 3 patches, no changes to the code, just avoiding doing
multiple things per patch, but point here is, when I see "enable passing
bpf object file to --event" I apply the patch and expect to be able to
go straight away and do:

    perf record -e foo.o sleep 1

And get some informative message as to hey, yes, I managed to do what
you asked or something is wrong, but I get:

  [root@...icio ~]# perf record --event foo.o sleep
  event syntax error: 'foo.o'
                       \___ parser error
  Run 'perf list' for a list of valid events

   usage: perf record [<options>] [<command>]
      or: perf record [<options>] -- <command> [<options>]

      -e, --event <event>   event selector. use 'perf list' to list available events
  [root@...icio ~]#

So, is it enabled or is it not? Doesn't look like. I.e. I think the best
thing is to only expose this when it can be used. Till that time, we can
go on adding infrastructure to enable that and test that infrastructure
via 'perf test', like you did with 'perf test LLVM', thanks for doing
that:

  [root@...icio ~]# perf test LLVM
  38: Test LLVM searching and compiling                        : Ok

And in verbose mode we can see that it is finding everything it needs on this
RHEL7.1 test machine, after I got all the pieces from the llvm project git
repos and built it:

  [root@...icio ~]# perf test -v LLVM
  38: Test LLVM searching and compiling                        :
  --- start ---
  test child forked, pid 24457
  Kernel build dir is set to /lib/modules/4.2.0-rc3+/build
  set env: KBUILD_DIR=/lib/modules/4.2.0-rc3+/build
  unset env: KBUILD_OPTS
  include option is set to  -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/4.8.3/include -I/home/acme/git/linux/arch/x86/include -Iarch/x86/include/generated/uapi -Iarch/x86/include/generated  -I/home/acme/git/linux/include -Iinclude -I/home/acme/git/linux/arch/x86/include/uapi -Iarch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi -Iinclude/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h 
  set env: CLANG_EXEC=/usr/local/bin/clang
  set env: CLANG_OPTIONS=-xc 
  set env: KERNEL_INC_OPTIONS= -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/4.8.3/include -I/home/acme/git/linux/arch/x86/include -Iarch/x86/include/generated/uapi -Iarch/x86/include/generated  -I/home/acme/git/linux/include -Iinclude -I/home/acme/git/linux/arch/x86/include/uapi -Iarch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi -Iinclude/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h 
  set env: WORKING_DIR=/lib/modules/4.2.0-rc3+/build
  set env: CLANG_SOURCE=-
  llvm compiling command template: echo '__attribute__((section("do_fork"), used)) int fork(void *ctx) {return 0;} char _license[] __attribute__((section("license"), used)) = "GPL";int _version __attribute__((section("version"), used)) = 0x40100;' | $CLANG_EXEC -D__KERNEL__ $CLANG_OPTIONS $KERNEL_INC_OPTIONS -Wno-unused-value -Wno-pointer-sign -working-directory $WORKING_DIR -c "$CLANG_SOURCE" -target bpf -O2 -o -
  test child finished with 0
  ---- end ----
  Test LLVM searching and compiling: Ok
  [root@...icio ~]#

Then, the next patch is even more exciting:

  perf record: Compile scriptlets if pass '.c' to --event

But its changelog doesn't come with a "hello_bpf.c" for me to use nor
the output of using it... bummer, I expected to use this new feature
right now :-)

Also, originally it had this name:

  [acme@...icio linux]$ git show --oneline f0f99c45295e67787ad532492d9bebd6d9b56d3a  | head -1
  f0f99c45295e perf record: Enable passing bpf object file to --event

But it doesn't even touch 'perf record':

  [acme@...icio linux]$ git show f0f99c45295e67787ad532492d9bebd6d9b56d3a | grep builtin-record
  [acme@...icio linux]$ 

So perhaps this was an artifact of refactoring previous patchkits?

I.e. I haven't looked at the other patches, but conceptually I can't think
_why_ would we expose a new feature that would only later be wired up and
then _really_ work.

Please reorder the patches to only expose --event foo.[oc] to when we can use
it.

Please continue from what I have in my perf/ebpf branch, it is all rebased to
latest tip/perf/core and passes most of the tests I put it thru (make -C
tools/perf build-test, 'perf test', fedora 21, RHEL 7.1).

Thanks!

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