Re: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Mon, Nov 26, 2018 at 04:02:17PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Nov 05, 2018 at 07:53:17PM +0000, Hunter, Adrian escreveu:
> > > From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
> > > Em Mon, Nov 05, 2018 at 07:21:44PM +0000, Hunter, Adrian escreveu:
> > > > > > +static bool machine__single_ku_as(struct machine *machine) {
> > > > > > +	return strcmp(perf_env__arch(machine->env), "sparc"); }

> > > > > Can we avoid having this strcmp be done repeatedly?

> > > > It is only done if there are mapping errors

> > > > > Can we avoid having this strcmp be done repeatedly? I.e. just
> > > > > make this a boolean initialized at session start, when
> > > > > machine->env is setup, so we'd have:

> > > > >    machine->single_address_space

> > > > > Instead of a function?

> > > > Sure  thing.
> 
> Have you sent an update to this patch series addressing the concerns? I
> think that renaming that _fb to __fallback and having this
> machine->single_address_space is what is missing, I'll try to take a
> stab at this now and have it in my perf/core branch soon.
 
> But you have this already done, please send it.

So, here is the patch on top of yours and at the end a patch that I'm
inserting just before yours, please check and ack, thanks:

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index da523d99be4b..4506aa7fa557 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2592,25 +2592,13 @@ int machine__get_kernel_start(struct machine *machine)
 	return err;
 }
 
-/*
- * machine__single_ku_as - Machine has same address space for kernel and user.
- * @machine: machine object
- *
- * Some architectures have a single address space for kernel and user addresses,
- * which makes it possible to determine if an address is in kernel space or user
- * space.
- */
-static bool machine__single_ku_as(struct machine *machine)
-{
-	return strcmp(perf_env__arch(machine->env), "sparc");
-}
 
 u8 machine__addr_cpumode(struct machine *machine, u8 cpumode, u64 addr)
 {
 	u8 addr_cpumode = cpumode;
 	bool kernel_ip;
 
-	if (!machine__single_ku_as(machine))
+	if (!machine->env->single_address_space)
 		goto out;
 
 	kernel_ip = machine__kernel_ip(machine, addr);


commit 5aabc18da787f7f6785b4027c63a89592e4e2c03
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Mon Nov 26 16:25:25 2018 -0300

    perf env: Record if a arch has a single user/kernel address space
    
    Some architectures have a single address space for kernel and user
    addresses, which makes it possible to determine if an address is in
    kernel space or user space. Some don't, e.g.: sparc.
    
    Cache that info in perf_env so that, for instance, code needing to
    fallback failed symbol lookups at the kernel space in single address
    space arches can lookup at userspace.
    
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Andi Kleen <ak@linux.intel.com>
    Cc: David Ahern <dsahern@gmail.com>
    Cc: David Miller <davem@davemloft.net>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Leo Yan <leo.yan@linaro.org>
    Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Wang Nan <wangnan0@huawei.com>
    Link: https://lkml.kernel.org/n/tip-4jb2rusqml7utgebiaf51k77@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 59f38c7693f8..d1cae06a2dfc 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -8,6 +8,16 @@
 
 struct perf_env perf_env;
 
+/*
+ * Some architectures have a single address space for kernel and user
+ * addresses, which makes it possible to determine if an address is in kernel
+ * space or user space. Some don't, e.g.: sparc
+ */
+void perf_env__init_single_address_space(struct perf_env *env)
+{
+	env->single_address_space = strcmp(perf_env__arch(env), "sparc");
+}
+
 void perf_env__exit(struct perf_env *env)
 {
 	int i;
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index d01b8355f4ca..a79898f6417d 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -60,6 +60,12 @@ struct perf_env {
 	struct cpu_topology_map	*cpu;
 	struct cpu_cache_level	*caches;
 	int			 caches_cnt;
+	/*
+         * Some architectures have a single address space for kernel and user
+         * addresses, which makes it possible to determine if an address is in
+         * kernel space or user space. Some don't, e.g.: sparc
+         */
+	bool			single_address_space;
 	struct numa_node	*numa_nodes;
 	struct memory_node	*memory_nodes;
 	unsigned long long	 memory_bsize;
@@ -72,6 +78,8 @@ void perf_env__exit(struct perf_env *env);
 
 int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[]);
 
+void perf_env__init_single_address_space(struct perf_env *env);
+
 int perf_env__read_cpu_topology_map(struct perf_env *env);
 
 void cpu_cache_level__free(struct cpu_cache_level *cache);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7d2c8ce6cfad..164d56229135 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -150,6 +150,8 @@ struct perf_session *perf_session__new(struct perf_data *data,
 		session->machines.host.env = &perf_env;
 	}
 
+	perf_env__init_single_address_space(session->machines.host.env);
+
 	if (!data || perf_data__is_write(data)) {
 		/*
 		 * In O_RDONLY mode this will be performed when reading the