[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181126194145.GE18491@kernel.org>
Date: Mon, 26 Nov 2018 16:41:45 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Jiri Olsa <jolsa@...hat.com>, Andi Kleen <ak@...ux.intel.com>,
linux-kernel@...r.kernel.org, Leo Yan <leo.yan@...aro.org>,
David Miller <davem@...emloft.net>,
Mathieu Poirier <mathieu.poirier@...aro.org>
Subject: Re: [PATCH 1/5] perf tools: Add fallback functions for cases where
cpumode is insufficient
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@...nel.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@...hat.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@...el.com>
Cc: Andi Kleen <ak@...ux.intel.com>
Cc: David Ahern <dsahern@...il.com>
Cc: David Miller <davem@...emloft.net>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Leo Yan <leo.yan@...aro.org>
Cc: Mathieu Poirier <mathieu.poirier@...aro.org>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Wang Nan <wangnan0@...wei.com>
Link: https://lkml.kernel.org/n/tip-4jb2rusqml7utgebiaf51k77@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.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
Powered by blists - more mailing lists