From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>, Andi Kleen <ak@linux.intel.com>,
linux-kernel@vger.kernel.org, Leo Yan <leo.yan@linaro.org>,
David Miller <davem@davemloft.net>,
Mathieu Poirier <mathieu.poirier@linaro.org>
Subject: Re: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
Date: Mon, 26 Nov 2018 16:41:45 -0300 [thread overview]
Message-ID: <20181126194145.GE18491@kernel.org> (raw)
In-Reply-To: <20181126190217.GC18491@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
next prev parent reply other threads:[~2018-11-27 12:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-31 9:10 [PATCH 0/5] perf tools: Fix for cases where cpumode is incorrect or insufficient Adrian Hunter
2018-10-31 9:10 ` [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient Adrian Hunter
2018-11-05 17:29 ` Arnaldo Carvalho de Melo
2018-11-05 18:19 ` David Miller
2018-11-05 19:21 ` Hunter, Adrian
2018-11-05 19:36 ` Arnaldo Carvalho de Melo
2018-11-05 19:53 ` Hunter, Adrian
2018-11-05 20:31 ` Arnaldo Carvalho de Melo
2018-11-26 19:02 ` Arnaldo Carvalho de Melo
2018-11-26 19:41 ` Arnaldo Carvalho de Melo [this message]
2018-11-26 19:44 ` Arnaldo Carvalho de Melo
2018-10-31 9:10 ` [PATCH 2/5] perf tools: Use fallback for sample_addr_correlates_sym() cases Adrian Hunter
2018-11-03 17:46 ` Hunter, Adrian
2018-10-31 9:10 ` [PATCH 3/5] perf tools: Use fallbacks for branch stacks Adrian Hunter
2018-11-05 17:56 ` Arnaldo Carvalho de Melo
2018-10-31 9:10 ` [PATCH 4/5] perf intel-pt: Insert callchain context into synthesized callchains Adrian Hunter
2018-10-31 13:28 ` Arnaldo Carvalho de Melo
2018-10-31 14:15 ` Adrian Hunter
2018-10-31 14:20 ` Adrian Hunter
2018-10-31 14:27 ` Arnaldo Carvalho de Melo
2018-10-31 22:12 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
2018-10-31 14:26 ` [PATCH 4/5] " Arnaldo Carvalho de Melo
2018-10-31 9:10 ` [PATCH 5/5] perf intel-pt/bts: Calculate cpumode for synthesized samples Adrian Hunter
2018-10-31 22:13 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
2018-10-31 13:35 ` [PATCH 0/5] perf tools: Fix for cases where cpumode is incorrect or insufficient Jiri Olsa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181126194145.GE18491@kernel.org \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=davem@davemloft.net \
--cc=jolsa@redhat.com \
--cc=leo.yan@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).