From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757826AbbJ2Whx (ORCPT ); Thu, 29 Oct 2015 18:37:53 -0400 Received: from mail.kernel.org ([198.145.29.136]:35965 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756475AbbJ2Whv (ORCPT ); Thu, 29 Oct 2015 18:37:51 -0400 Date: Thu, 29 Oct 2015 19:37:44 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: ast@plumgrid.com, brendan.d.gregg@gmail.com, a.p.zijlstra@chello.nl, daniel@iogearbox.net, dsahern@gmail.com, hekuang@huawei.com, jolsa@kernel.org, lizefan@huawei.com, masami.hiramatsu.pt@hitachi.com, namhyung@kernel.org, paulus@samba.org, linux-kernel@vger.kernel.org, pi3orama@163.com, xiakaixu@huawei.com Subject: Re: [PATCH 10/31] perf test: Enforce LLVM test for BPF test Message-ID: <20151029223744.GK2923@kernel.org> References: <1444826502-49291-1-git-send-email-wangnan0@huawei.com> <1444826502-49291-11-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1444826502-49291-11-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Wed, Oct 14, 2015 at 12:41:21PM +0000, Wang Nan escreveu: > This patch replaces the original toy BPF program with previous introduced > bpf-script-example.c. Dynamically embedded it into 'llvm-src.c'. > > The newly introduced BPF program attaches a BPF program at > 'sys_epoll_pwait()', and collect half samples from it. perf itself never "collect half samples from it"? Can you rephrase this? > use that syscall, so further test can verify their result with it. > > Since BPF program require LINUX_VERSION_CODE of runtime kernel, this > patch computes that code from uname. > > Since the resuling BPF object is useful for further testcases, this patch > introduces 'prepare' and 'cleanup' method to tests, and makes test__llvm() > create a MAP_SHARED memory array to hold the resulting object. I think you're doing lots of things in just one patch, please split it, for instance, that ->prepare()/->cleanup() should stand on its own, with a proper changelog, etc. The LINUX_VERSION_CODE part should probably also be on its own patch, working much like __FILE__ or __func__, i.e. gcc predefines those, the bpf code that calls clang to build the provided .c file should do the same for LINUX_VERSION_CODE, so that bpf scriptlets can rely on it being defined. I.e. that compose_source() function, but not just for the 'perf test' entry, for any bpf scriptlet. > Signed-off-by: He Kuang > Signed-off-by: Wang Nan > Cc: Arnaldo Carvalho de Melo > Cc: Alexei Starovoitov > Cc: Brendan Gregg > Cc: Daniel Borkmann > Cc: David Ahern > Cc: Jiri Olsa > Cc: Kaixu Xia > Cc: Masami Hiramatsu > Cc: Namhyung Kim > Cc: Peter Zijlstra > Cc: Zefan Li > Cc: pi3orama@163.com > Link: http://lkml.kernel.org/n/ebpf-6yw9eg0ej3l4jnqhinngkw86@git.kernel.org > --- > tools/perf/tests/Build | 9 +++- > tools/perf/tests/builtin-test.c | 6 +++ > tools/perf/tests/llvm.c | 104 +++++++++++++++++++++++++++++++++++----- > tools/perf/tests/llvm.h | 14 ++++++ > tools/perf/tests/tests.h | 4 ++ > 5 files changed, 123 insertions(+), 14 deletions(-) > create mode 100644 tools/perf/tests/llvm.h > > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build > index 50de225..4afc8c8 100644 > --- a/tools/perf/tests/Build > +++ b/tools/perf/tests/Build > @@ -31,9 +31,16 @@ perf-y += sample-parsing.o > perf-y += parse-no-sample-id-all.o > perf-y += kmod-path.o > perf-y += thread-map.o > -perf-y += llvm.o > +perf-y += llvm.o llvm-src.o > perf-y += topology.o > > +$(OUTPUT)tests/llvm-src.c: tests/bpf-script-example.c > + $(call rule_mkdir) > + $(Q)echo '#include ' > $@ > + $(Q)echo 'const char test_llvm__bpf_prog[] =' >> $@ > + $(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@ > + $(Q)echo ';' >> $@ > + > ifeq ($(ARCH),$(filter $(ARCH),x86 arm arm64)) > perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o > endif > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > index 66f72d3..e812a0c 100644 > --- a/tools/perf/tests/builtin-test.c > +++ b/tools/perf/tests/builtin-test.c > @@ -160,6 +160,8 @@ static struct test generic_tests[] = { > { > .desc = "Test LLVM searching and compiling", > .func = test__llvm, > + .prepare = test__llvm_prepare, > + .cleanup = test__llvm_cleanup, > }, > { > .desc = "Test topology in session", > @@ -261,7 +263,11 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist) > } > > pr_debug("\n--- start ---\n"); > + if (t->prepare) > + t->prepare(); > err = run_test(t); > + if (t->cleanup) > + t->cleanup(); > pr_debug("---- end ----\n%s:", t->desc); > > switch (err) { > diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c > index 52d5597..236bf39 100644 > --- a/tools/perf/tests/llvm.c > +++ b/tools/perf/tests/llvm.c > @@ -1,9 +1,13 @@ > #include > +#include > #include > #include > #include > +#include > +#include > #include "tests.h" > #include "debug.h" > +#include "llvm.h" > > static int perf_config_cb(const char *var, const char *val, > void *arg __maybe_unused) > @@ -11,16 +15,6 @@ static int perf_config_cb(const char *var, const char *val, > return perf_default_config(var, val, arg); > } > > -/* > - * Randomly give it a "version" section since we don't really load it > - * into kernel > - */ > -static const char test_bpf_prog[] = > - "__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;"; > - > #ifdef HAVE_LIBBPF_SUPPORT > static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz) > { > @@ -41,12 +35,44 @@ static int test__bpf_parsing(void *obj_buf __maybe_unused, > } > #endif > > +static char * > +compose_source(void) > +{ > + struct utsname utsname; > + int version, patchlevel, sublevel, err; > + unsigned long version_code; > + char *code; > + > + if (uname(&utsname)) > + return NULL; > + > + err = sscanf(utsname.release, "%d.%d.%d", > + &version, &patchlevel, &sublevel); > + if (err != 3) { > + fprintf(stderr, " (Can't get kernel version from uname '%s')", > + utsname.release); > + return NULL; > + } > + > + version_code = (version << 16) + (patchlevel << 8) + sublevel; > + err = asprintf(&code, "#define LINUX_VERSION_CODE 0x%08lx;\n%s", > + version_code, test_llvm__bpf_prog); > + if (err < 0) > + return NULL; > + > + return code; > +} > + > +#define SHARED_BUF_INIT_SIZE (1 << 20) > +struct test_llvm__bpf_result *p_test_llvm__bpf_result; > + > int test__llvm(void) > { > char *tmpl_new, *clang_opt_new; > void *obj_buf; > size_t obj_buf_sz; > int err, old_verbose; > + char *source; > > perf_config(perf_config_cb, NULL); > > @@ -73,10 +99,22 @@ int test__llvm(void) > if (!llvm_param.clang_opt) > llvm_param.clang_opt = strdup(""); > > - err = asprintf(&tmpl_new, "echo '%s' | %s", test_bpf_prog, > - llvm_param.clang_bpf_cmd_template); > - if (err < 0) > + source = compose_source(); > + if (!source) { > + pr_err("Failed to compose source code\n"); > + return -1; > + } > + > + /* Quote __EOF__ so strings in source won't be expanded by shell */ > + err = asprintf(&tmpl_new, "cat << '__EOF__' | %s\n%s\n__EOF__\n", > + llvm_param.clang_bpf_cmd_template, source); > + free(source); > + source = NULL; > + if (err < 0) { > + pr_err("Failed to alloc new template\n"); > return -1; > + } > + > err = asprintf(&clang_opt_new, "-xc %s", llvm_param.clang_opt); > if (err < 0) > return -1; > @@ -93,6 +131,46 @@ int test__llvm(void) > } > > err = test__bpf_parsing(obj_buf, obj_buf_sz); > + if (!err && p_test_llvm__bpf_result) { > + if (obj_buf_sz > SHARED_BUF_INIT_SIZE) { > + pr_err("Resulting object too large\n"); > + } else { > + p_test_llvm__bpf_result->size = obj_buf_sz; > + memcpy(p_test_llvm__bpf_result->object, > + obj_buf, obj_buf_sz); > + } > + } > free(obj_buf); > return err; > } > + > +void test__llvm_prepare(void) > +{ > + p_test_llvm__bpf_result = mmap(NULL, SHARED_BUF_INIT_SIZE, > + PROT_READ | PROT_WRITE, > + MAP_SHARED | MAP_ANONYMOUS, -1, 0); > + if (!p_test_llvm__bpf_result) > + return; > + memset((void *)p_test_llvm__bpf_result, '\0', SHARED_BUF_INIT_SIZE); > +} > + > +void test__llvm_cleanup(void) > +{ > + unsigned long boundary, buf_end; > + > + if (!p_test_llvm__bpf_result) > + return; > + if (p_test_llvm__bpf_result->size == 0) { > + munmap((void *)p_test_llvm__bpf_result, SHARED_BUF_INIT_SIZE); > + p_test_llvm__bpf_result = NULL; > + return; > + } > + > + buf_end = (unsigned long)p_test_llvm__bpf_result + SHARED_BUF_INIT_SIZE; > + > + boundary = (unsigned long)(p_test_llvm__bpf_result); > + boundary += p_test_llvm__bpf_result->size; > + boundary = (boundary + (page_size - 1)) & > + (~((unsigned long)page_size - 1)); > + munmap((void *)boundary, buf_end - boundary); > +} > diff --git a/tools/perf/tests/llvm.h b/tools/perf/tests/llvm.h > new file mode 100644 > index 0000000..1e89e46 > --- /dev/null > +++ b/tools/perf/tests/llvm.h > @@ -0,0 +1,14 @@ > +#ifndef PERF_TEST_LLVM_H > +#define PERF_TEST_LLVM_H > + > +#include /* for size_t */ > + > +struct test_llvm__bpf_result { > + size_t size; > + char object[]; > +}; > + > +extern struct test_llvm__bpf_result *p_test_llvm__bpf_result; > +extern const char test_llvm__bpf_prog[]; > + > +#endif > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h > index c804869..a848802 100644 > --- a/tools/perf/tests/tests.h > +++ b/tools/perf/tests/tests.h > @@ -27,6 +27,8 @@ enum { > struct test { > const char *desc; > int (*func)(void); > + void (*prepare)(void); > + void (*cleanup)(void); > }; > > /* Tests */ > @@ -66,6 +68,8 @@ int test__fdarray__add(void); > int test__kmod_path__parse(void); > int test__thread_map(void); > int test__llvm(void); > +void test__llvm_prepare(void); > +void test__llvm_cleanup(void); > int test_session_topology(void); > > #if defined(__arm__) || defined(__aarch64__) > -- > 1.8.3.4