Re: [PATCH v7 04/14] perf tools: Move unwind__prepare_access from thread_new into thread__insert_map

Jiri Olsa
2016-06-01T08:39:38Z

On Tue, May 31, 2016 at 11:19:02AM +0000, He Kuang wrote:
> For determine the libunwind methods to use, we should get the
> 32bit/64bit information from maps of a thread. When a thread is newly
> created, the information is not prepared. This patch moves
> unwind__prepare_access() into thread__insert_map() so we can get the
> information we need from maps. Meanwhile, let thread__insert_map()
> return value and show messages on error.
>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
> tools/perf/util/machine.c | 10 ++++++++--
> tools/perf/util/thread.c | 13 +++----------
> tools/perf/util/thread.h | 2 +-
> tools/perf/util/unwind-libunwind.c | 7 +++----
> 4 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index bdc33ce..0bd6aac 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1353,7 +1353,10 @@ int machine__process_mmap2_event(struct machine *machine,
> if (map == NULL)
> goto out_problem_map;
>
> - thread__insert_map(thread, map);
> + ret = thread__insert_map(thread, map);
> + if (ret)
> + pr_err("Thread insert map error\n");
> +

I think we should fail the function, not just display error

> thread__put(thread);
> map__put(map);
> return 0;
> @@ -1403,7 +1406,10 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
> if (map == NULL)
> goto out_problem_map;
>
> - thread__insert_map(thread, map);
> + ret = thread__insert_map(thread, map);
> + if (ret)
> + pr_err("Thread insert map error\n");

same here

jirka