Date:   Thu, 6 Jun 2019 11:05:28 -0300
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Mike Leach <mike.leach@...aro.org>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org, Taeung Song <treeze.taeung@...il.com>
Subject: Re: [PATCH v2 3/4] perf augmented_raw_syscalls: Support arm64 raw
 syscalls

Em Thu, Jun 06, 2019 at 10:46:24AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jun 06, 2019 at 10:38:38AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jun 06, 2019 at 05:48:44PM +0800, Leo Yan escreveu:
> > > This patch adds support for arm64 raw syscall numbers so that we can use
> > > it on arm64 platform.

> > > After applied this patch, we need to specify macro -D__aarch64__ or
> > > -D__x86_64__ in compilation option so Clang can use the corresponding
> > > syscall numbers for arm64 or x86_64 respectively, other architectures
> > > will report failure when compilation.

> > So, please check what I have in my perf/core branch, I've completely
> > removed arch specific stuff from augmented_raw_syscalls.c.

> > What is done now is use a map to specify what to copy, that same map
> > that is used to state which syscalls should be traced.

> > It uses that tools/perf/arch/arm64/entry/syscalls/mksyscalltbl to figure
> > out the mapping of syscall names to ids, just like is done for x86_64
> > and other arches, falling back to audit-libs when that syscalltbl thing
> > is not present.
 
> Also added:
 
> Fixes: ac96287cae08 ("perf trace: Allow specifying a set of events to add in perfconfig")
 
> For the stable@...nel.org folks to automagically pick this.

And this extra patch is needed, with yours and this one we finally get
what we want, which points to the kernel verifier blocking something,
exactly what is it that is blocking
(/home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.o)
and who asked for it, (trace.add_events=...) in a config key-value pair:

[root@...co ~]# perf trace ls
event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.o'
                     \___ Kernel verifier blocks program loading

(add -v to see detail)
Run 'perf list' for a list of valid events
Error: wrong config key-value pair trace.add_events=/home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.o
[root@...co ~]#


commit 6455f983af2657b950d5dd5c45783e31e41ead4a
Author: Arnaldo Carvalho de Melo <acme@...hat.com>
Date:   Thu Jun 6 10:56:55 2019 -0300

    perf config: Bail out when a handler returns failure for a key-value pair
    
    So perf_config() uses:
    
      int ret = 0;
    
      perf_config_set__for_each_entry(config_set, section, item) {
              ...
              ret = fn();
              if (ret < 0)
                      break;
      }
    
      return ret;
    
    Expecting that that break will imediatelly go to function exit to return
    that error value (ret).
    
    The problem is that perf_config_set__for_each_entry() expands into two
    nested for() loops, one traversing the sections in a config and the
    second the items in each of those sections, so we have to change that
    'break' to a goto label right before that final 'return ret'.
    
    With that, for instance 'perf trace' now correctly bails out when a
    event that is requested to be added via its 'trace.add_events'
    ~/.perfconfig entry gets rejected by the kernel BPF verifier:
    
      # perf trace ls
      event syntax error: '/home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.o'
                           \___ Kernel verifier blocks program loading
    
      (add -v to see detail)
      Run 'perf list' for a list of valid events
      Error: wrong config key-value pair trace.add_events=/home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.o
      #
    
    While before it would continue and explode later, when trying to find
    maps that would have been in place had that augmented_raw_syscalls.o
    precompiled BPF proggie been accepted by the, humm, bast... rigorous
    kernel BPF verifier 8-)
    
    Cc: Adrian Hunter <adrian.hunter@...el.com>
    Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
    Cc: Alexei Starovoitov <ast@...nel.org>
    Cc: Daniel Borkmann <daniel@...earbox.net>
    Cc: Jiri Olsa <jolsa@...hat.com>
    Cc: Martin KaFai Lau <kafai@...com>
    Cc: Mathieu Poirier <mathieu.poirier@...aro.org>
    Cc: Mike Leach <mike.leach@...aro.org>
    Cc: Namhyung Kim <namhyung@...nel.org>
    Cc: Song Liu <songliubraving@...com>
    Cc: Suzuki Poulouse <suzuki.poulose@....com>
    Cc: Taeung Song <treeze.taeung@...il.com>
    Cc: Yonghong Song <yhs@...com>
    Fixes: 8a0a9c7e9146 ("perf config: Introduce new init() and exit()")
    Link: https://lkml.kernel.org/n/tip-qvqxfk9d0rn1l7lcntwiezrr@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 7e3c1b60120c..e7d2c08d263a 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -739,11 +739,15 @@ int perf_config(config_fn_t fn, void *data)
 			if (ret < 0) {
 				pr_err("Error: wrong config key-value pair %s=%s\n",
 				       key, value);
-				break;
+				/*
+				 * Can't be just a 'break', as perf_config_set__for_each_entry()
+				 * expands to two nested for() loops.
+				 */
+				goto out;
 			}
 		}
 	}
-
+out:
 	return ret;
 }
 

Powered by blists - more mailing lists