Date:   Tue, 19 Mar 2019 11:14:35 -0300
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Song Liu <songliubraving@...com>
Cc:     "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>,
        Peter Zijlstra <peterz@...radead.org>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Stanislav Fomichev <sdf@...ichev.me>
Subject: Re: [PATCH v9 perf,bpf 12/15] perf, bpf: enable annotation of bpf
 program

Em Tue, Mar 19, 2019 at 10:58:13AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Mar 19, 2019 at 06:05:30AM +0000, Song Liu escreveu:
> > test-disassembler-four-args.c is an existing test used by bpftool.
> > It was added in fb982666e380c1632a74495b68b3c33a66e76430 .  

> > > After applying this test it "works":

> > >  make: Entering directory '/home/acme/git/perf/tools/perf'
> > >    BUILD:   Doing 'make -j8' parallel build

> > >  Auto-detecting system features:
> > >  ...                         dwarf: [ on  ]
> > >  ...            dwarf_getlocations: [ on  ]
> > >  <SNIP>
> > >  ...                           bpf: [ on  ]
> > >  ...                        libaio: [ on  ]
> > >  ...        disassembler-four-args: [ on  ]
> > > 
> > > Because you added it to FEATURE_TESTS_BASIC, and this means that if
> > > tools/build/feature/test-all.c builds, then what is in
> > > FEATURE_TESTS_BASIC is set to 'on', but you didn't add anything to
> > > tools/build/feature/test-all.c, so it works because all the other
> > > features are present.

> > > Take a look at:

> > >  2a07d814747b ("tools build feature: Check if libaio is available")

> > > To see what needs to be done.

> > I used different versions of gcc to verify that current version of the 
> > patch works for both test-succeed and test-fail cases. I guess something 
> > like the following is needed? But the test does work without it. 

> Even with the file below, it fails, as:

> [acme@...co perf]$ cat /tmp/build/perf/feature/test-all.make.output
> /usr/bin/ld: /tmp/ccTj7iV6.o: in function `main_test_disassembler_four_args':
> /home/acme/git/perf/tools/build/feature/test-disassembler-four-args.c:9: undefined reference to `disassembler'
> collect2: error: ld returned 1 exit status
> [acme@...co perf]$

> Then we do the test for each of the features and _then_ this works:

> [acme@...co perf]$ ls -la /tmp/build/perf/feature/test-disassembler-four-args.*
> -rwxrwxr-x. 1 acme acme 2098312 Mar 19 10:51 /tmp/build/perf/feature/test-disassembler-four-args.bin
> -rw-rw-r--. 1 acme acme    1079 Mar 19 10:51 /tmp/build/perf/feature/test-disassembler-four-args.d
> -rw-rw-r--. 1 acme acme       0 Mar 19 10:51 /tmp/build/perf/feature/test-disassembler-four-args.make.output
> [acme@...co perf]$
> 
> So there is still something missing, i.e. which objects to add to the
> test-all case, I'll fix it.

This was missing:

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index c51b59e43dcc..4f491ad4f492 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -227,6 +227,8 @@ FEATURE_CHECK_LDFLAGS-libpython-version := $(PYTHON_EMBED_LDOPTS)
 
 FEATURE_CHECK_LDFLAGS-libaio = -lrt
 
+FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lopcodes
+
 CFLAGS += -fno-omit-frame-pointer
 CFLAGS += -ggdb3
 CFLAGS += -funwind-tables

----------------------

After that, and with a clean /tmp/build/perf, re-running:

  make -C tools/perf O=/tmp/build/perf

we get:

[acme@...co perf]$ ls -la /tmp/build/perf/feature/
total 2888
drwxrwxr-x.  2 acme acme     400 Mar 19 11:08 .
drwxrwxr-x. 16 acme acme    4280 Mar 19 11:08 ..
-rwxrwxr-x.  1 acme acme 2680352 Mar 19 11:07 test-all.bin
-rw-rw-r--.  1 acme acme   38753 Mar 19 11:07 test-all.d
-rw-rw-r--.  1 acme acme       0 Mar 19 11:07 test-all.make.output
-rw-rw-r--.  1 acme acme     168 Mar 19 11:07 test-bionic.make.output
-rwxrwxr-x.  1 acme acme  140960 Mar 19 11:08 test-cplus-demangle.bin
-rw-rw-r--.  1 acme acme     101 Mar 19 11:08 test-cplus-demangle.d
-rw-rw-r--.  1 acme acme       0 Mar 19 11:08 test-cplus-demangle.make.output
-rwxrwxr-x.  1 acme acme   17976 Mar 19 11:08 test-jvmti.bin
-rwxrwxr-x.  1 acme acme   17984 Mar 19 11:08 test-jvmti-cmlr.bin
-rw-rw-r--.  1 acme acme    1107 Mar 19 11:08 test-jvmti-cmlr.d
-rw-rw-r--.  1 acme acme       0 Mar 19 11:08 test-jvmti-cmlr.make.output
-rw-rw-r--.  1 acme acme    1014 Mar 19 11:08 test-jvmti.d
-rw-rw-r--.  1 acme acme       0 Mar 19 11:08 test-jvmti.make.output
-rwxrwxr-x.  1 acme acme   18368 Mar 19 11:07 test-libbabeltrace.bin
-rw-rw-r--.  1 acme acme    1470 Mar 19 11:07 test-libbabeltrace.d
-rw-rw-r--.  1 acme acme       0 Mar 19 11:07 test-libbabeltrace.make.output
-rw-rw-r--.  1 acme acme     179 Mar 19 11:07 test-libunwind-aarch64.make.output
-rw-rw-r--.  1 acme acme     163 Mar 19 11:07 test-libunwind-x86.make.output
[acme@...co perf]$

I.e. test-all.bin got built, and if we look at it:

[acme@...co perf]$ nm /tmp/build/perf/feature/test-all.bin  | grep disassembler
0000000000061f90 T disassembler
0000000000062110 T disassembler_options_cmp
0000000000061fb0 T disassembler_usage
0000000000061f40 T main_test_disassembler_four_args
000000000006b7c0 T print_i386_disassembler_options
[acme@...co perf]$ 

I.e. the -lopcodes was added to the test-all.bin, and as we include that
itest-disassembler-four-args.c in the test-all.c file, it gets linked
and that test-all.c build doesn't fail, so all that is in
FEATURE_TESTS_BASIC is marked as present, CFLAGS +=
-DDISASM_FOUR_ARGS_SIGNATURE gets set, etc.

Now the test is really being performed, the test-all.bin feature
detection fast path is working, proceeding.

- Arnaldo

Powered by blists - more mailing lists