Re: [PATCH 21/31] perf test: Enforce LLVM test, add kbuild test

[Arnaldo Carvalho de Melo <acme@redhat.com>]

Em Tue, Oct 20, 2015 at 08:06:54PM +0800, Wangnan (F) escreveu:
> Hi Namhyung and Arnaldo,
> 
> I changed my testing related patches in my local git tree. All
> changed combined together is at the end of this mail. I remove
> all stderr output, and improve debug message. Please have a look
> at it.
> 
> Thank you.

Yeah, from a quick look it looks sane, i.e. 'perf test' should state
just 'Ok', "FAIL" or "Skip", -v provides a reason, -vv will provide more
detail and so on, i.e. a verbosity level, for that we have 'pr_debug(),
pr_debug2(), pr_debug3(), etc, and can always, for some specialized use,
like grouping multiple warnings or deciding if we need to call some
other function only when the verbosity level is greater than some level,
check the 'verbose' variable value.

- Arnaldo
 
> 
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 5a6290a..453eff8 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -49,7 +49,7 @@ static struct bpf_object *prepare_bpf(const char
> *name, void *obj_buf,
> 
>      obj = bpf__prepare_load_buffer(obj_buf, obj_buf_sz, name);
>      if (IS_ERR(obj)) {
> -        fprintf(stderr, " (compile failed)");
> +        pr_debug("Compile BPF program failed.\n");
>          return NULL;
>      }
>      return obj;
> @@ -82,7 +82,7 @@ static int do_test(struct bpf_object *obj, int
> (*func)(void), int expect)
> 
>      err = parse_events_load_bpf_obj(&parse_evlist,
> &parse_evlist.list, obj);
>      if (err || list_empty(&parse_evlist.list)) {
> -        fprintf(stderr, " (Failed to add events selected by BPF)");
> +        pr_debug("Failed to add events selected by BPF\n");
>          if (!err)
>              err = -EINVAL;
>          goto out;
> @@ -140,7 +140,7 @@ static int do_test(struct bpf_object *obj, int
> (*func)(void), int expect)
>      }
> 
>      if (count != expect) {
> -        fprintf(stderr, " (filter result incorrect: %d != %d)",
> count, expect);
> +        pr_debug("BPF filter result incorrect: %d != %d\n", count, expect);
>          err = -EBADF;
>      }
> 
> @@ -164,16 +164,14 @@ static int __test__bpf(int index, const char *name,
> 
>      test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz, index);
>      if (!obj_buf || !obj_buf_sz) {
> -        if (verbose == 0)
> -            fprintf(stderr, " (%s)", message_compile);
> +        pr_debug("Failed to compile: %s\n", message_compile);
>          return TEST_SKIP;
>      }
> 
>      obj = prepare_bpf(name, obj_buf, obj_buf_sz);
>      if (!obj) {
>          err = -EINVAL;
> -        if ((verbose == 0) && (message_load[0] != '\0'))
> -            fprintf(stderr, " (%s)", message_load);
> +        pr_debug("Failed to load: %s", message_load);
>          goto out;
>      }
> 
> @@ -192,7 +190,7 @@ int test__bpf(void)
>      int err;
> 
>      if (geteuid() != 0) {
> -        fprintf(stderr, " (try run as root)");
> +        pr_debug("Only root can run BPF test\n");
>          return TEST_SKIP;
>      }
> 
> @@ -214,7 +212,7 @@ int test__bpf(void)
>                (NR_ITERS + 1) / 4);
>      return err;
>  #else
> -    fprintf(stderr, " (skip BPF prologue test)");
> +    pr_debug("BPF prologue is disabled when compiling, skip this test\n");
>      return TEST_OK;
>  #endif
>  }
> diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
> index e722e8a..7c3b2c3 100644
> --- a/tools/perf/tests/llvm.c
> +++ b/tools/perf/tests/llvm.c
> @@ -13,19 +13,23 @@
>  struct llvm_testcase {
>      const char *source;
>      const char *errmsg;
> +    const char *hintmsg;
>      struct test_llvm__bpf_result *result;
>      bool tried;
>  } llvm_testcases[NR_LLVM_TESTCASES + 1] = {
>      [LLVM_TESTCASE_BASE]    = {.source = test_llvm__bpf_prog,
>                     .errmsg = "Basic LLVM compiling failed",
> +                   .hintmsg = "Check llvm.clang-path option in
> ~/.perfconfig",
>                     .tried = false},
>      [LLVM_TESTCASE_KBUILD]    = {.source =
> test_llvm__bpf_test_kbuild_prog,
> -                   .errmsg = "llvm.kbuild-dir can be fixed",
> +                   .errmsg = "Unable to find usable kbuild dir",
> +                   .hintmsg = "Check llvm.kbuild-dir option in
> ~/.perfconfig",
>                     .tried = false},
>      /* Don't output if this one fail. */
>      [LLVM_TESTCASE_BPF_PROLOGUE]    = {
>                     .source = test_llvm__bpf_test_prologue_prog,
> -                   .errmsg = "failed for unknown reason",
> +                   .errmsg = "Unable to compile BPF prologue
> testing program",
> +                   .hintmsg = "This is an internal error, please
> report it",
>                     .tried = false},
>      {.source = NULL}
>  };
> @@ -43,16 +47,16 @@ static int test__bpf_parsing(void *obj_buf,
> size_t obj_buf_sz)
> 
>      obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, NULL);
>      if (!obj)
> -        return -1;
> +        return TEST_FAIL;
>      bpf_object__close(obj);
> -    return 0;
> +    return TEST_OK;
>  }
>  #else
>  static int test__bpf_parsing(void *obj_buf __maybe_unused,
>                   size_t obj_buf_sz __maybe_unused)
>  {
> -    fprintf(stderr, " (skip bpf parsing)");
> -    return 0;
> +    pr_debug("BPF support is not compiled, skip BPF parsing\n");
> +    return TEST_SKIP;
>  }
>  #endif
> 
> @@ -70,8 +74,8 @@ compose_source(const char *raw_source)
>      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);
> +        pr_debug("Unablt to get kernel version from uname '%s'\n",
> +             utsname.release);
>          return NULL;
>      }
> 
> @@ -104,7 +108,7 @@ static int __test__llvm(int i)
>       * and clang is not found in $PATH, and this is not perf test -v
>       */
>      if (verbose == 0 && !llvm_param.user_set_param &&
> llvm__search_clang()) {
> -        fprintf(stderr, " (no clang, try 'perf test -v LLVM')");
> +        pr_debug("Unable to find workable clang\n");
>          return TEST_SKIP;
>      }
> 
> @@ -155,7 +159,7 @@ static int __test__llvm(int i)
> 
>      verbose = old_verbose;
>      if (err)
> -        return -1;
> +        return TEST_FAIL;
> 
>      err = test__bpf_parsing(obj_buf, obj_buf_sz);
>      if (!err && result) {
> @@ -181,13 +185,12 @@ int test__llvm(void)
>               * First testcase tests basic LLVM compiling. If it
>               * fails, no need to check others.
>               */
> -            if (!verbose)
> -                fprintf(stderr, " (use -v to see error message)");
> -            return ret;
> +            return TEST_FAIL;
>          } else if (ret) {
> -            if (!verbose && llvm_testcases[i].errmsg)
> -                fprintf(stderr, " (%s)", llvm_testcases[i].errmsg);
> -            return 0;
> +            pr_debug("%s\n", llvm_testcases[i].errmsg);
> +            if (llvm_testcases[i].hintmsg)
> +                pr_debug("Hint:\t%s\n", llvm_testcases[i].hintmsg);
> +            return TEST_SKIP;
>          }
>      }
>      return 0;
> @@ -199,12 +202,15 @@ void test__llvm_prepare(void)
> 
>      for (i = 0; llvm_testcases[i].source; i++) {
>          struct test_llvm__bpf_result *result;
> +        void *p;
> 
> -        result = mmap(NULL, SHARED_BUF_INIT_SIZE,
> -                  PROT_READ | PROT_WRITE,
> -                  MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> -        if (!result)
> +        p = mmap(NULL, SHARED_BUF_INIT_SIZE,
> +             PROT_READ | PROT_WRITE,
> +             MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +        if (p == MAP_FAILED)
>              return;
> +
> +        result = p;
>          memset((void *)result, '\0', SHARED_BUF_INIT_SIZE);
> 
>          llvm_testcases[i].result = result;
> diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
> index e4adb18..36093d9 100644
> --- a/tools/perf/util/bpf-prologue.c
> +++ b/tools/perf/util/bpf-prologue.c
> @@ -337,8 +337,10 @@ int bpf__gen_prologue(struct probe_trace_arg
> *args, int nargs,
>          return 0;
>      }
> 
> -    if (nargs > BPF_PROLOGUE_MAX_ARGS)
> +    if (nargs > BPF_PROLOGUE_MAX_ARGS) {
> +        pr_warning("bpf: prologue: too many arguments\n");
>          nargs = BPF_PROLOGUE_MAX_ARGS;
> +    }
>      if (cnt_space > BPF_MAXINSNS)
>          cnt_space = BPF_MAXINSNS;
> 
>