[PATCHv2 0/4] perf tools: Various fixes

[PATCHv2 0/4] perf tools: Various fixes

[Jiri Olsa]

hi,
this is v2 for original post:
  http://marc.info/?t=146711345400003&r=1&w=2

v2 changes:
  - keep nr_numa_nodes in patch 1
  - added patch 2,3,4

thanks,
jirka

---
Jiri Olsa (4):
      perf tools: Transform nodes string info to struct
      perf tests: Fix hist accumulation test
      perf tools: Add initialized arg into unwind__prepare_access
      perf tools: Call unwind__prepare_access for forked thread

 tools/perf/tests/hists_cumulate.c  |  4 ++++
 tools/perf/util/callchain.h        |  1 +
 tools/perf/util/env.c              |  5 ++++-
 tools/perf/util/env.h              | 10 +++++++++-
 tools/perf/util/header.c           | 76 ++++++++++++++++++++++++++--------------------------------------------------
 tools/perf/util/map.c              |  9 ++++++++-
 tools/perf/util/map.h              |  2 +-
 tools/perf/util/thread.c           | 39 ++++++++++++++++++++++++++++++++++++---
 tools/perf/util/unwind-libunwind.c | 11 +++++++++--
 tools/perf/util/unwind.h           |  9 ++++++---
 tools/perf/util/util.c             | 19 +++++++++++++------
 11 files changed, 117 insertions(+), 68 deletions(-)

[PATCH 1/4] perf tools: Transform nodes string info to struct

[Jiri Olsa]

Storing NUMA info within struct numa_node instead
of strings. This way it's usable in future patches.

Also it turned out it's slightly less code involved
than using strings.

Link: http://lkml.kernel.org/n/tip-ka37sax3gfaxwvytfxi0ycy1@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/env.c    |  5 +++-
 tools/perf/util/env.h    | 10 ++++++-
 tools/perf/util/header.c | 76 +++++++++++++++++-------------------------------
 3 files changed, 39 insertions(+), 52 deletions(-)

diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 49a11d9d8b8f..bb964e86b09d 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -18,10 +18,13 @@ void perf_env__exit(struct perf_env *env)
 	zfree(&env->cmdline_argv);
 	zfree(&env->sibling_cores);
 	zfree(&env->sibling_threads);
-	zfree(&env->numa_nodes);
 	zfree(&env->pmu_mappings);
 	zfree(&env->cpu);
 
+	for (i = 0; i < env->nr_numa_nodes; i++)
+		cpu_map__put(env->numa_nodes[i].map);
+	zfree(&env->numa_nodes);
+
 	for (i = 0; i < env->caches_cnt; i++)
 		cpu_cache_level__free(&env->caches[i]);
 	zfree(&env->caches);
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 56cffb60a0b4..b164dfd2dcbf 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -2,6 +2,7 @@
 #define __PERF_ENV_H
 
 #include <linux/types.h>
+#include "cpumap.h"
 
 struct cpu_topology_map {
 	int	socket_id;
@@ -18,6 +19,13 @@ struct cpu_cache_level {
 	char	*map;
 };
 
+struct numa_node {
+	u32		 node;
+	u64		 mem_total;
+	u64		 mem_free;
+	struct cpu_map	*map;
+};
+
 struct perf_env {
 	char			*hostname;
 	char			*os_release;
@@ -40,11 +48,11 @@ struct perf_env {
 	const char		**cmdline_argv;
 	char			*sibling_cores;
 	char			*sibling_threads;
-	char			*numa_nodes;
 	char			*pmu_mappings;
 	struct cpu_topology_map	*cpu;
 	struct cpu_cache_level	*caches;
 	int			 caches_cnt;
+	struct numa_node	*numa_nodes;
 };
 
 extern struct perf_env perf_env;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index c5cd2698281f..8f0db4007282 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1306,42 +1306,19 @@ static void print_total_mem(struct perf_header *ph, int fd __maybe_unused,
 static void print_numa_topology(struct perf_header *ph, int fd __maybe_unused,
 				FILE *fp)
 {
-	u32 nr, c, i;
-	char *str, *tmp;
-	uint64_t mem_total, mem_free;
-
-	/* nr nodes */
-	nr = ph->env.nr_numa_nodes;
-	str = ph->env.numa_nodes;
-
-	for (i = 0; i < nr; i++) {
-		/* node number */
-		c = strtoul(str, &tmp, 0);
-		if (*tmp != ':')
-			goto error;
-
-		str = tmp + 1;
-		mem_total = strtoull(str, &tmp, 0);
-		if (*tmp != ':')
-			goto error;
+	int i;
+	struct numa_node *n;
 
-		str = tmp + 1;
-		mem_free = strtoull(str, &tmp, 0);
-		if (*tmp != ':')
-			goto error;
+	for (i = 0; i < ph->env.nr_numa_nodes; i++) {
+		n = &ph->env.numa_nodes[i];
 
 		fprintf(fp, "# node%u meminfo  : total = %"PRIu64" kB,"
 			    " free = %"PRIu64" kB\n",
-			c, mem_total, mem_free);
+			n->node, n->mem_total, n->mem_free);
 
-		str = tmp + 1;
-		fprintf(fp, "# node%u cpu list : %s\n", c, str);
-
-		str += strlen(str) + 1;
+		fprintf(fp, "# node%u cpu list : ", n->node);
+		cpu_map__fprintf(n->map, fp);
 	}
-	return;
-error:
-	fprintf(fp, "# numa topology : not available\n");
 }
 
 static void print_cpuid(struct perf_header *ph, int fd __maybe_unused, FILE *fp)
@@ -1906,11 +1883,10 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 				 struct perf_header *ph, int fd,
 				 void *data __maybe_unused)
 {
+	struct numa_node *nodes, *n;
 	ssize_t ret;
-	u32 nr, node, i;
+	u32 nr, i;
 	char *str;
-	uint64_t mem_total, mem_free;
-	struct strbuf sb;
 
 	/* nr nodes */
 	ret = readn(fd, &nr, sizeof(nr));
@@ -1921,47 +1897,47 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 		nr = bswap_32(nr);
 
 	ph->env.nr_numa_nodes = nr;
-	if (strbuf_init(&sb, 256) < 0)
-		return -1;
+	nodes = zalloc(sizeof(*nodes) * nr);
+	if (!nodes)
+		return -ENOMEM;
 
 	for (i = 0; i < nr; i++) {
+		n = &nodes[i];
+
 		/* node number */
-		ret = readn(fd, &node, sizeof(node));
-		if (ret != sizeof(node))
+		ret = readn(fd, &n->node, sizeof(u32));
+		if (ret != sizeof(n->node))
 			goto error;
 
-		ret = readn(fd, &mem_total, sizeof(u64));
+		ret = readn(fd, &n->mem_total, sizeof(u64));
 		if (ret != sizeof(u64))
 			goto error;
 
-		ret = readn(fd, &mem_free, sizeof(u64));
+		ret = readn(fd, &n->mem_free, sizeof(u64));
 		if (ret != sizeof(u64))
 			goto error;
 
 		if (ph->needs_swap) {
-			node = bswap_32(node);
-			mem_total = bswap_64(mem_total);
-			mem_free = bswap_64(mem_free);
+			n->node      = bswap_32(n->node);
+			n->mem_total = bswap_64(n->mem_total);
+			n->mem_free  = bswap_64(n->mem_free);
 		}
 
-		if (strbuf_addf(&sb, "%u:%"PRIu64":%"PRIu64":",
-				node, mem_total, mem_free) < 0)
-			goto error;
-
 		str = do_read_string(fd, ph);
 		if (!str)
 			goto error;
 
-		/* include a NULL character at the end */
-		if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
+		n->map = cpu_map__new(str);
+		if (!n->map)
 			goto error;
+
 		free(str);
 	}
-	ph->env.numa_nodes = strbuf_detach(&sb, NULL);
+	ph->env.numa_nodes = nodes;
 	return 0;
 
 error:
-	strbuf_release(&sb);
+	free(nodes);
 	return -1;
 }
 
-- 
2.4.11

[PATCH 2/4] perf tests: Fix hist accumulation test

[Jiri Olsa]

User's values from .perfconfig could overload the default
callchain setup and cause this test to fail.  Making sure
the test is using default callchain_param values.

Link: http://lkml.kernel.org/n/tip-7jyzfrdoglqfmht1xt2f83ntp@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/hists_cumulate.c |  4 ++++
 tools/perf/util/callchain.h       |  1 +
 tools/perf/util/util.c            | 19 +++++++++++++------
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index a9e3db3afac4..9fd54b79a788 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -216,6 +216,8 @@ static int do_test(struct hists *hists, struct result *expected, size_t nr_expec
 
 		/* check callchain entries */
 		root = &he->callchain->node.rb_root;
+
+		TEST_ASSERT_VAL("callchains expected", !RB_EMPTY_ROOT(root));
 		cnode = rb_entry(rb_first(root), struct callchain_node, rb_node);
 
 		c = 0;
@@ -666,6 +668,8 @@ static int test4(struct perf_evsel *evsel, struct machine *machine)
 	perf_evsel__set_sample_bit(evsel, CALLCHAIN);
 
 	setup_sorting(NULL);
+
+	callchain_param = callchain_param_default;
 	callchain_register_param(&callchain_param);
 
 	err = add_hist_entries(hists, machine);
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index a70f6b54eb92..13e75549c440 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -106,6 +106,7 @@ struct callchain_param {
 };
 
 extern struct callchain_param callchain_param;
+extern struct callchain_param callchain_param_default;
 
 struct callchain_list {
 	u64			ip;
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index e08b9a092a23..5f44a21955cd 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -19,12 +19,19 @@
 #include "callchain.h"
 #include "strlist.h"
 
-struct callchain_param	callchain_param = {
-	.mode	= CHAIN_GRAPH_ABS,
-	.min_percent = 0.5,
-	.order  = ORDER_CALLEE,
-	.key	= CCKEY_FUNCTION,
-	.value	= CCVAL_PERCENT,
+#define CALLCHAIN_PARAM_DEFAULT			\
+	.mode		= CHAIN_GRAPH_ABS,	\
+	.min_percent	= 0.5,			\
+	.order		= ORDER_CALLEE,		\
+	.key		= CCKEY_FUNCTION,	\
+	.value		= CCVAL_PERCENT,	\
+
+struct callchain_param callchain_param = {
+	CALLCHAIN_PARAM_DEFAULT
+};
+
+struct callchain_param callchain_param_default = {
+	CALLCHAIN_PARAM_DEFAULT
 };
 
 /*
-- 
2.4.11

[PATCH 3/4] perf tools: Add initialized arg into unwind__prepare_access

[Jiri Olsa]

Adding initialized arg into unwind__prepare_access
to get feedback about the initialization state.

It's not possible to get it from error code, because
we return 0 even in case we don't recognize dso, which
is valid.

The 'initialized' value is used in following patch
to speedup unwind__prepare_access calls logic in
fork path.

Cc: He Kuang <hekuang@huawei.com>
Link: http://lkml.kernel.org/n/tip-vzmw5piz7diqa7rd6c49mjph@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/thread.c           |  2 +-
 tools/perf/util/unwind-libunwind.c | 11 +++++++++--
 tools/perf/util/unwind.h           |  9 ++++++---
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index f30f9566fddc..2439b122a4e4 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -202,7 +202,7 @@ int thread__insert_map(struct thread *thread, struct map *map)
 {
 	int ret;
 
-	ret = unwind__prepare_access(thread, map);
+	ret = unwind__prepare_access(thread, map, NULL);
 	if (ret)
 		return ret;
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 854711966cad..6d542a4e0648 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -14,15 +14,19 @@ static void unwind__register_ops(struct thread *thread,
 	thread->unwind_libunwind_ops = ops;
 }
 
-int unwind__prepare_access(struct thread *thread, struct map *map)
+int unwind__prepare_access(struct thread *thread, struct map *map,
+			   bool *initialized)
 {
 	const char *arch;
 	enum dso_type dso_type;
 	struct unwind_libunwind_ops *ops = local_unwind_libunwind_ops;
+	int err;
 
 	if (thread->addr_space) {
 		pr_debug("unwind: thread map already set, dso=%s\n",
 			 map->dso->name);
+		if (initialized)
+			*initialized = true;
 		return 0;
 	}
 
@@ -51,7 +55,10 @@ int unwind__prepare_access(struct thread *thread, struct map *map)
 out_register:
 	unwind__register_ops(thread, ops);
 
-	return thread->unwind_libunwind_ops->prepare_access(thread);
+	err = thread->unwind_libunwind_ops->prepare_access(thread);
+	if (initialized)
+		*initialized = err ? false : true;
+	return err;
 }
 
 void unwind__flush_access(struct thread *thread)
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 84c6d44d52f9..17ea1f928f13 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -42,12 +42,14 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 #endif
 
 int LIBUNWIND__ARCH_REG_ID(int regnum);
-int unwind__prepare_access(struct thread *thread, struct map *map);
+int unwind__prepare_access(struct thread *thread, struct map *map,
+			   bool *initialized);
 void unwind__flush_access(struct thread *thread);
 void unwind__finish_access(struct thread *thread);
 #else
 static inline int unwind__prepare_access(struct thread *thread __maybe_unused,
-					 struct map *map __maybe_unused)
+					 struct map *map __maybe_unused,
+					 bool *initialized __maybe_unused);
 {
 	return 0;
 }
@@ -67,7 +69,8 @@ unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
 }
 
 static inline int unwind__prepare_access(struct thread *thread __maybe_unused,
-					 struct map *map __maybe_unused)
+					 struct map *map __maybe_unused,
+					 bool *initialized __maybe_unused);
 {
 	return 0;
 }
-- 
2.4.11

[PATCH 4/4] perf tools: Call unwind__prepare_access for forked thread

[Jiri Olsa]

Currently we call unwind__prepare_access for map event.
In case we report fork event the thread inherits its
parent's maps and unwind__prepare_access is never called
for the thread.

This causes unwind__get_entries seeing uninitialized
unwind_libunwind_ops and thus returning no callchain.

Adding unwind__prepare_access calls for fork even
processing.

Cc: He Kuang <hekuang@huawei.com>
Link: http://lkml.kernel.org/n/tip-tzvjn5rd8dl5ki7b8mgdkrpu@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/map.c    |  9 ++++++++-
 tools/perf/util/map.h    |  2 +-
 tools/perf/util/thread.c | 37 +++++++++++++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index b19bcd3b7128..b39b12a1208d 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -15,6 +15,7 @@
 #include "debug.h"
 #include "machine.h"
 #include <linux/string.h>
+#include "unwind.h"
 
 static void __maps__insert(struct maps *maps, struct map *map);
 
@@ -744,9 +745,10 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
 /*
  * XXX This should not really _copy_ te maps, but refcount them.
  */
-int map_groups__clone(struct map_groups *mg,
+int map_groups__clone(struct thread *thread,
 		      struct map_groups *parent, enum map_type type)
 {
+	struct map_groups *mg = thread->mg;
 	int err = -ENOMEM;
 	struct map *map;
 	struct maps *maps = &parent->maps[type];
@@ -757,6 +759,11 @@ int map_groups__clone(struct map_groups *mg,
 		struct map *new = map__clone(map);
 		if (new == NULL)
 			goto out_unlock;
+
+		err = unwind__prepare_access(thread, new, NULL);
+		if (err)
+			goto out_unlock;
+
 		map_groups__insert(mg, new);
 		map__put(new);
 	}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 7309d64ce39e..d83396ceecba 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -194,7 +194,7 @@ struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
                                          struct map **mapp, symbol_filter_t filter);
 void map_groups__init(struct map_groups *mg, struct machine *machine);
 void map_groups__exit(struct map_groups *mg);
-int map_groups__clone(struct map_groups *mg,
+int map_groups__clone(struct thread *thread,
 		      struct map_groups *parent, enum map_type type);
 size_t map_groups__fprintf(struct map_groups *mg, FILE *fp);
 
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 2439b122a4e4..8b10a55410a2 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -212,6 +212,39 @@ int thread__insert_map(struct thread *thread, struct map *map)
 	return 0;
 }
 
+static int __thread__prepare_access(struct thread *thread)
+{
+	bool initialized = false;
+	int i, err = 0;
+
+	for (i = 0; i < MAP__NR_TYPES; ++i) {
+		struct maps *maps = &thread->mg->maps[i];
+		struct map *map;
+
+		pthread_rwlock_rdlock(&maps->lock);
+
+		for (map = maps__first(maps); map; map = map__next(map)) {
+			err = unwind__prepare_access(thread, map, &initialized);
+			if (err || initialized)
+				break;
+		}
+
+		pthread_rwlock_unlock(&maps->lock);
+	}
+
+	return err;
+}
+
+static int thread__prepare_access(struct thread *thread)
+{
+	int err = 0;
+
+	if (symbol_conf.use_callchain)
+		err = __thread__prepare_access(thread);
+
+	return err;
+}
+
 static int thread__clone_map_groups(struct thread *thread,
 				    struct thread *parent)
 {
@@ -219,7 +252,7 @@ static int thread__clone_map_groups(struct thread *thread,
 
 	/* This is new thread, we share map groups for process. */
 	if (thread->pid_ == parent->pid_)
-		return 0;
+		return thread__prepare_access(thread);
 
 	if (thread->mg == parent->mg) {
 		pr_debug("broken map groups on thread %d/%d parent %d/%d\n",
@@ -229,7 +262,7 @@ static int thread__clone_map_groups(struct thread *thread,
 
 	/* But this one is new process, copy maps. */
 	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
+		if (map_groups__clone(thread, parent->mg, i) < 0)
 			return -ENOMEM;
 
 	return 0;
-- 
2.4.11

Re: [PATCH 3/4] perf tools: Add initialized arg into unwind__prepare_access

[Arnaldo Carvalho de Melo]

Em Mon, Jul 04, 2016 at 02:16:22PM +0200, Jiri Olsa escreveu:
> Adding initialized arg into unwind__prepare_access
> to get feedback about the initialization state.
> 
> It's not possible to get it from error code, because
> we return 0 even in case we don't recognize dso, which
> is valid.
> 
> The 'initialized' value is used in following patch
> to speedup unwind__prepare_access calls logic in
> fork path.
> 
> Cc: He Kuang <hekuang@huawei.com>
> Link: http://lkml.kernel.org/n/tip-vzmw5piz7diqa7rd6c49mjph@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/thread.c           |  2 +-
>  tools/perf/util/unwind-libunwind.c | 11 +++++++++--
>  tools/perf/util/unwind.h           |  9 ++++++---
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index f30f9566fddc..2439b122a4e4 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -202,7 +202,7 @@ int thread__insert_map(struct thread *thread, struct map *map)
>  {
>  	int ret;
>  
> -	ret = unwind__prepare_access(thread, map);
> +	ret = unwind__prepare_access(thread, map, NULL);
>  	if (ret)
>  		return ret;
>  
> diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
> index 854711966cad..6d542a4e0648 100644
> --- a/tools/perf/util/unwind-libunwind.c
> +++ b/tools/perf/util/unwind-libunwind.c
> @@ -14,15 +14,19 @@ static void unwind__register_ops(struct thread *thread,
>  	thread->unwind_libunwind_ops = ops;
>  }
>  
> -int unwind__prepare_access(struct thread *thread, struct map *map)
> +int unwind__prepare_access(struct thread *thread, struct map *map,
> +			   bool *initialized)
>  {
>  	const char *arch;
>  	enum dso_type dso_type;
>  	struct unwind_libunwind_ops *ops = local_unwind_libunwind_ops;
> +	int err;
>  
>  	if (thread->addr_space) {
>  		pr_debug("unwind: thread map already set, dso=%s\n",
>  			 map->dso->name);
> +		if (initialized)
> +			*initialized = true;
>  		return 0;
>  	}
>  
> @@ -51,7 +55,10 @@ int unwind__prepare_access(struct thread *thread, struct map *map)
>  out_register:
>  	unwind__register_ops(thread, ops);
>  
> -	return thread->unwind_libunwind_ops->prepare_access(thread);
> +	err = thread->unwind_libunwind_ops->prepare_access(thread);
> +	if (initialized)
> +		*initialized = err ? false : true;
> +	return err;
>  }
>  
>  void unwind__flush_access(struct thread *thread)
> diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
> index 84c6d44d52f9..17ea1f928f13 100644
> --- a/tools/perf/util/unwind.h
> +++ b/tools/perf/util/unwind.h
> @@ -42,12 +42,14 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
>  #endif
>  
>  int LIBUNWIND__ARCH_REG_ID(int regnum);
> -int unwind__prepare_access(struct thread *thread, struct map *map);
> +int unwind__prepare_access(struct thread *thread, struct map *map,
> +			   bool *initialized);
>  void unwind__flush_access(struct thread *thread);
>  void unwind__finish_access(struct thread *thread);
>  #else
>  static inline int unwind__prepare_access(struct thread *thread __maybe_unused,
> -					 struct map *map __maybe_unused)
> +					 struct map *map __maybe_unused,
> +					 bool *initialized __maybe_unused);
>  {
>  	return 0;
>  }
> @@ -67,7 +69,8 @@ unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
>  }
>  
>  static inline int unwind__prepare_access(struct thread *thread __maybe_unused,
> -					 struct map *map __maybe_unused)
> +					 struct map *map __maybe_unused,
> +					 bool *initialized __maybe_unused);
>  {
>  	return 0;
>  }

That extra ; breaks the build on centos5 (i.e. on an arch where the
above code gets compiled:

  CC       /tmp/build/perf/bench/sched-pipe.o
In file included from util/machine.c:14:
util/unwind.h:74: error: expected identifier or '(' before '{' token
  MKDIR    /tmp/build/perf/tests/
  CC       /tmp/build/perf/tests/parse-events.o
mv: cannot stat `/tmp/build/perf/util/.machine.o.tmp': No such file or directory
make[3]: *** [/tmp/build/perf/util/machine.o] Error 1
make[3]: *** Waiting for unfinished jobs....
  MKDIR    /tmp/build/perf/tests/
  CC       /tmp/build/perf/tests/dso-data.o
make[2]: *** [util] Error 2
make[1]: *** [/tmp/build/perf/libperf-in.o] Error 2
make[1]: *** Waiting for unfinished jobs....
  MKDIR    /tmp/build/perf/tests/


Fixing it.
> -- 
> 2.4.11

Re: [PATCH 3/4] perf tools: Add initialized arg into unwind__prepare_access

[Jiri Olsa]

On Mon, Jul 04, 2016 at 04:25:11PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> >  }
> > @@ -67,7 +69,8 @@ unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
> >  }
> >  
> >  static inline int unwind__prepare_access(struct thread *thread __maybe_unused,
> > -					 struct map *map __maybe_unused)
> > +					 struct map *map __maybe_unused,
> > +					 bool *initialized __maybe_unused);
> >  {
> >  	return 0;
> >  }
> 
> That extra ; breaks the build on centos5 (i.e. on an arch where the
> above code gets compiled:
> 
>   CC       /tmp/build/perf/bench/sched-pipe.o
> In file included from util/machine.c:14:
> util/unwind.h:74: error: expected identifier or '(' before '{' token
>   MKDIR    /tmp/build/perf/tests/
>   CC       /tmp/build/perf/tests/parse-events.o
> mv: cannot stat `/tmp/build/perf/util/.machine.o.tmp': No such file or directory
> make[3]: *** [/tmp/build/perf/util/machine.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
>   MKDIR    /tmp/build/perf/tests/
>   CC       /tmp/build/perf/tests/dso-data.o
> make[2]: *** [util] Error 2
> make[1]: *** [/tmp/build/perf/libperf-in.o] Error 2
> make[1]: *** Waiting for unfinished jobs....
>   MKDIR    /tmp/build/perf/tests/
> 
> 
> Fixing it.

oops, thanks

jirka

[tip:perf/core] perf header: Transform nodes string info to struct

[tip-bot for Jiri Olsa]

Commit-ID:  c60da22aca8755b77b7f4d4caf57ada8654db939
Gitweb:     http://git.kernel.org/tip/c60da22aca8755b77b7f4d4caf57ada8654db939
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 4 Jul 2016 14:16:20 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Jul 2016 19:39:01 -0300

perf header: Transform nodes string info to struct

Storing NUMA info within struct numa_node instead of strings. This way
it's usable in future patches.

Also it turned out it's slightly less code involved than using strings.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1467634583-29147-2-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/env.c    |  5 +++-
 tools/perf/util/env.h    | 10 ++++++-
 tools/perf/util/header.c | 76 +++++++++++++++++-------------------------------
 3 files changed, 39 insertions(+), 52 deletions(-)

diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 49a11d9..bb964e8 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -18,10 +18,13 @@ void perf_env__exit(struct perf_env *env)
 	zfree(&env->cmdline_argv);
 	zfree(&env->sibling_cores);
 	zfree(&env->sibling_threads);
-	zfree(&env->numa_nodes);
 	zfree(&env->pmu_mappings);
 	zfree(&env->cpu);
 
+	for (i = 0; i < env->nr_numa_nodes; i++)
+		cpu_map__put(env->numa_nodes[i].map);
+	zfree(&env->numa_nodes);
+
 	for (i = 0; i < env->caches_cnt; i++)
 		cpu_cache_level__free(&env->caches[i]);
 	zfree(&env->caches);
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 56cffb6..b164dfd 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -2,6 +2,7 @@
 #define __PERF_ENV_H
 
 #include <linux/types.h>
+#include "cpumap.h"
 
 struct cpu_topology_map {
 	int	socket_id;
@@ -18,6 +19,13 @@ struct cpu_cache_level {
 	char	*map;
 };
 
+struct numa_node {
+	u32		 node;
+	u64		 mem_total;
+	u64		 mem_free;
+	struct cpu_map	*map;
+};
+
 struct perf_env {
 	char			*hostname;
 	char			*os_release;
@@ -40,11 +48,11 @@ struct perf_env {
 	const char		**cmdline_argv;
 	char			*sibling_cores;
 	char			*sibling_threads;
-	char			*numa_nodes;
 	char			*pmu_mappings;
 	struct cpu_topology_map	*cpu;
 	struct cpu_cache_level	*caches;
 	int			 caches_cnt;
+	struct numa_node	*numa_nodes;
 };
 
 extern struct perf_env perf_env;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index c5cd269..8f0db40 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1306,42 +1306,19 @@ static void print_total_mem(struct perf_header *ph, int fd __maybe_unused,
 static void print_numa_topology(struct perf_header *ph, int fd __maybe_unused,
 				FILE *fp)
 {
-	u32 nr, c, i;
-	char *str, *tmp;
-	uint64_t mem_total, mem_free;
-
-	/* nr nodes */
-	nr = ph->env.nr_numa_nodes;
-	str = ph->env.numa_nodes;
-
-	for (i = 0; i < nr; i++) {
-		/* node number */
-		c = strtoul(str, &tmp, 0);
-		if (*tmp != ':')
-			goto error;
-
-		str = tmp + 1;
-		mem_total = strtoull(str, &tmp, 0);
-		if (*tmp != ':')
-			goto error;
+	int i;
+	struct numa_node *n;
 
-		str = tmp + 1;
-		mem_free = strtoull(str, &tmp, 0);
-		if (*tmp != ':')
-			goto error;
+	for (i = 0; i < ph->env.nr_numa_nodes; i++) {
+		n = &ph->env.numa_nodes[i];
 
 		fprintf(fp, "# node%u meminfo  : total = %"PRIu64" kB,"
 			    " free = %"PRIu64" kB\n",
-			c, mem_total, mem_free);
+			n->node, n->mem_total, n->mem_free);
 
-		str = tmp + 1;
-		fprintf(fp, "# node%u cpu list : %s\n", c, str);
-
-		str += strlen(str) + 1;
+		fprintf(fp, "# node%u cpu list : ", n->node);
+		cpu_map__fprintf(n->map, fp);
 	}
-	return;
-error:
-	fprintf(fp, "# numa topology : not available\n");
 }
 
 static void print_cpuid(struct perf_header *ph, int fd __maybe_unused, FILE *fp)
@@ -1906,11 +1883,10 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 				 struct perf_header *ph, int fd,
 				 void *data __maybe_unused)
 {
+	struct numa_node *nodes, *n;
 	ssize_t ret;
-	u32 nr, node, i;
+	u32 nr, i;
 	char *str;
-	uint64_t mem_total, mem_free;
-	struct strbuf sb;
 
 	/* nr nodes */
 	ret = readn(fd, &nr, sizeof(nr));
@@ -1921,47 +1897,47 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 		nr = bswap_32(nr);
 
 	ph->env.nr_numa_nodes = nr;
-	if (strbuf_init(&sb, 256) < 0)
-		return -1;
+	nodes = zalloc(sizeof(*nodes) * nr);
+	if (!nodes)
+		return -ENOMEM;
 
 	for (i = 0; i < nr; i++) {
+		n = &nodes[i];
+
 		/* node number */
-		ret = readn(fd, &node, sizeof(node));
-		if (ret != sizeof(node))
+		ret = readn(fd, &n->node, sizeof(u32));
+		if (ret != sizeof(n->node))
 			goto error;
 
-		ret = readn(fd, &mem_total, sizeof(u64));
+		ret = readn(fd, &n->mem_total, sizeof(u64));
 		if (ret != sizeof(u64))
 			goto error;
 
-		ret = readn(fd, &mem_free, sizeof(u64));
+		ret = readn(fd, &n->mem_free, sizeof(u64));
 		if (ret != sizeof(u64))
 			goto error;
 
 		if (ph->needs_swap) {
-			node = bswap_32(node);
-			mem_total = bswap_64(mem_total);
-			mem_free = bswap_64(mem_free);
+			n->node      = bswap_32(n->node);
+			n->mem_total = bswap_64(n->mem_total);
+			n->mem_free  = bswap_64(n->mem_free);
 		}
 
-		if (strbuf_addf(&sb, "%u:%"PRIu64":%"PRIu64":",
-				node, mem_total, mem_free) < 0)
-			goto error;
-
 		str = do_read_string(fd, ph);
 		if (!str)
 			goto error;
 
-		/* include a NULL character at the end */
-		if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
+		n->map = cpu_map__new(str);
+		if (!n->map)
 			goto error;
+
 		free(str);
 	}
-	ph->env.numa_nodes = strbuf_detach(&sb, NULL);
+	ph->env.numa_nodes = nodes;
 	return 0;
 
 error:
-	strbuf_release(&sb);
+	free(nodes);
 	return -1;
 }
 

[tip:perf/core] perf tests: Fix hist accumulation test

[tip-bot for Jiri Olsa]

Commit-ID:  347ca878062d5fb0e16db1fae81b0994f2457efd
Gitweb:     http://git.kernel.org/tip/347ca878062d5fb0e16db1fae81b0994f2457efd
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 4 Jul 2016 14:16:21 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Jul 2016 19:39:01 -0300

perf tests: Fix hist accumulation test

User's values from .perfconfig could overload the default callchain
setup and cause this test to fail.  Making sure the test is using
default callchain_param values.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1467634583-29147-3-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/hists_cumulate.c |  4 ++++
 tools/perf/util/callchain.h       |  1 +
 tools/perf/util/util.c            | 19 +++++++++++++------
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index a9e3db3..9fd54b7 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -216,6 +216,8 @@ static int do_test(struct hists *hists, struct result *expected, size_t nr_expec
 
 		/* check callchain entries */
 		root = &he->callchain->node.rb_root;
+
+		TEST_ASSERT_VAL("callchains expected", !RB_EMPTY_ROOT(root));
 		cnode = rb_entry(rb_first(root), struct callchain_node, rb_node);
 
 		c = 0;
@@ -666,6 +668,8 @@ static int test4(struct perf_evsel *evsel, struct machine *machine)
 	perf_evsel__set_sample_bit(evsel, CALLCHAIN);
 
 	setup_sorting(NULL);
+
+	callchain_param = callchain_param_default;
 	callchain_register_param(&callchain_param);
 
 	err = add_hist_entries(hists, machine);
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index a70f6b5..13e7554 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -106,6 +106,7 @@ struct callchain_param {
 };
 
 extern struct callchain_param callchain_param;
+extern struct callchain_param callchain_param_default;
 
 struct callchain_list {
 	u64			ip;
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index e08b9a0..5f44a21 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -19,12 +19,19 @@
 #include "callchain.h"
 #include "strlist.h"
 
-struct callchain_param	callchain_param = {
-	.mode	= CHAIN_GRAPH_ABS,
-	.min_percent = 0.5,
-	.order  = ORDER_CALLEE,
-	.key	= CCKEY_FUNCTION,
-	.value	= CCVAL_PERCENT,
+#define CALLCHAIN_PARAM_DEFAULT			\
+	.mode		= CHAIN_GRAPH_ABS,	\
+	.min_percent	= 0.5,			\
+	.order		= ORDER_CALLEE,		\
+	.key		= CCKEY_FUNCTION,	\
+	.value		= CCVAL_PERCENT,	\
+
+struct callchain_param callchain_param = {
+	CALLCHAIN_PARAM_DEFAULT
+};
+
+struct callchain_param callchain_param_default = {
+	CALLCHAIN_PARAM_DEFAULT
 };
 
 /*

[tip:perf/core] perf unwind: Add initialized arg into unwind__prepare_access

[tip-bot for Jiri Olsa]

Commit-ID:  a2873325ffb21cecca8032673eb698cb4d778dc6
Gitweb:     http://git.kernel.org/tip/a2873325ffb21cecca8032673eb698cb4d778dc6
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 4 Jul 2016 14:16:22 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Jul 2016 20:27:12 -0300

perf unwind: Add initialized arg into unwind__prepare_access

Adding initialized arg into unwind__prepare_access to get feedback about
the initialization state.

It's not possible to get it from error code, because we return 0 even in
case we don't recognize dso, which is valid.

The 'initialized' value is used in following patch to speedup
unwind__prepare_access calls logic in fork path.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1467634583-29147-4-git-send-email-jolsa@kernel.org
[ Remove ; after static inline function signatures, fixes build break ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread.c           |  2 +-
 tools/perf/util/unwind-libunwind.c | 11 +++++++++--
 tools/perf/util/unwind.h           |  9 ++++++---
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index f30f956..2439b12 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -202,7 +202,7 @@ int thread__insert_map(struct thread *thread, struct map *map)
 {
 	int ret;
 
-	ret = unwind__prepare_access(thread, map);
+	ret = unwind__prepare_access(thread, map, NULL);
 	if (ret)
 		return ret;
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 8547119..6d542a4 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -14,15 +14,19 @@ static void unwind__register_ops(struct thread *thread,
 	thread->unwind_libunwind_ops = ops;
 }
 
-int unwind__prepare_access(struct thread *thread, struct map *map)
+int unwind__prepare_access(struct thread *thread, struct map *map,
+			   bool *initialized)
 {
 	const char *arch;
 	enum dso_type dso_type;
 	struct unwind_libunwind_ops *ops = local_unwind_libunwind_ops;
+	int err;
 
 	if (thread->addr_space) {
 		pr_debug("unwind: thread map already set, dso=%s\n",
 			 map->dso->name);
+		if (initialized)
+			*initialized = true;
 		return 0;
 	}
 
@@ -51,7 +55,10 @@ int unwind__prepare_access(struct thread *thread, struct map *map)
 out_register:
 	unwind__register_ops(thread, ops);
 
-	return thread->unwind_libunwind_ops->prepare_access(thread);
+	err = thread->unwind_libunwind_ops->prepare_access(thread);
+	if (initialized)
+		*initialized = err ? false : true;
+	return err;
 }
 
 void unwind__flush_access(struct thread *thread)
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 84c6d44..61fb1e9 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -42,12 +42,14 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 #endif
 
 int LIBUNWIND__ARCH_REG_ID(int regnum);
-int unwind__prepare_access(struct thread *thread, struct map *map);
+int unwind__prepare_access(struct thread *thread, struct map *map,
+			   bool *initialized);
 void unwind__flush_access(struct thread *thread);
 void unwind__finish_access(struct thread *thread);
 #else
 static inline int unwind__prepare_access(struct thread *thread __maybe_unused,
-					 struct map *map __maybe_unused)
+					 struct map *map __maybe_unused,
+					 bool *initialized __maybe_unused)
 {
 	return 0;
 }
@@ -67,7 +69,8 @@ unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
 }
 
 static inline int unwind__prepare_access(struct thread *thread __maybe_unused,
-					 struct map *map __maybe_unused)
+					 struct map *map __maybe_unused,
+					 bool *initialized __maybe_unused)
 {
 	return 0;
 }

[tip:perf/core] perf unwind: Call unwind__prepare_access for forked thread

[tip-bot for Jiri Olsa]

Commit-ID:  6c502584438bda63fc1a67606854fb0b300465cd
Gitweb:     http://git.kernel.org/tip/6c502584438bda63fc1a67606854fb0b300465cd
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 4 Jul 2016 14:16:23 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Jul 2016 20:27:25 -0300

perf unwind: Call unwind__prepare_access for forked thread

Currently we call unwind__prepare_access for map event.  In case we
report fork event the thread inherits its parent's maps and
unwind__prepare_access is never called for the thread.

This causes unwind__get_entries seeing uninitialized
unwind_libunwind_ops and thus returning no callchain.

Adding unwind__prepare_access calls for fork even processing.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1467634583-29147-5-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.c    |  9 ++++++++-
 tools/perf/util/map.h    |  2 +-
 tools/perf/util/thread.c | 37 +++++++++++++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index b19bcd3..b39b12a 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -15,6 +15,7 @@
 #include "debug.h"
 #include "machine.h"
 #include <linux/string.h>
+#include "unwind.h"
 
 static void __maps__insert(struct maps *maps, struct map *map);
 
@@ -744,9 +745,10 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
 /*
  * XXX This should not really _copy_ te maps, but refcount them.
  */
-int map_groups__clone(struct map_groups *mg,
+int map_groups__clone(struct thread *thread,
 		      struct map_groups *parent, enum map_type type)
 {
+	struct map_groups *mg = thread->mg;
 	int err = -ENOMEM;
 	struct map *map;
 	struct maps *maps = &parent->maps[type];
@@ -757,6 +759,11 @@ int map_groups__clone(struct map_groups *mg,
 		struct map *new = map__clone(map);
 		if (new == NULL)
 			goto out_unlock;
+
+		err = unwind__prepare_access(thread, new, NULL);
+		if (err)
+			goto out_unlock;
+
 		map_groups__insert(mg, new);
 		map__put(new);
 	}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 7309d64..d83396c 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -194,7 +194,7 @@ struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
                                          struct map **mapp, symbol_filter_t filter);
 void map_groups__init(struct map_groups *mg, struct machine *machine);
 void map_groups__exit(struct map_groups *mg);
-int map_groups__clone(struct map_groups *mg,
+int map_groups__clone(struct thread *thread,
 		      struct map_groups *parent, enum map_type type);
 size_t map_groups__fprintf(struct map_groups *mg, FILE *fp);
 
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 2439b12..8b10a55 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -212,6 +212,39 @@ int thread__insert_map(struct thread *thread, struct map *map)
 	return 0;
 }
 
+static int __thread__prepare_access(struct thread *thread)
+{
+	bool initialized = false;
+	int i, err = 0;
+
+	for (i = 0; i < MAP__NR_TYPES; ++i) {
+		struct maps *maps = &thread->mg->maps[i];
+		struct map *map;
+
+		pthread_rwlock_rdlock(&maps->lock);
+
+		for (map = maps__first(maps); map; map = map__next(map)) {
+			err = unwind__prepare_access(thread, map, &initialized);
+			if (err || initialized)
+				break;
+		}
+
+		pthread_rwlock_unlock(&maps->lock);
+	}
+
+	return err;
+}
+
+static int thread__prepare_access(struct thread *thread)
+{
+	int err = 0;
+
+	if (symbol_conf.use_callchain)
+		err = __thread__prepare_access(thread);
+
+	return err;
+}
+
 static int thread__clone_map_groups(struct thread *thread,
 				    struct thread *parent)
 {
@@ -219,7 +252,7 @@ static int thread__clone_map_groups(struct thread *thread,
 
 	/* This is new thread, we share map groups for process. */
 	if (thread->pid_ == parent->pid_)
-		return 0;
+		return thread__prepare_access(thread);
 
 	if (thread->mg == parent->mg) {
 		pr_debug("broken map groups on thread %d/%d parent %d/%d\n",
@@ -229,7 +262,7 @@ static int thread__clone_map_groups(struct thread *thread,
 
 	/* But this one is new process, copy maps. */
 	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
+		if (map_groups__clone(thread, parent->mg, i) < 0)
 			return -ENOMEM;
 
 	return 0;