From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756079AbbCSOD4 (ORCPT ); Thu, 19 Mar 2015 10:03:56 -0400 Received: from mail.kernel.org ([198.145.29.136]:57396 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753584AbbCSODz (ORCPT ); Thu, 19 Mar 2015 10:03:55 -0400 Date: Thu, 19 Mar 2015 11:03:52 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: Wang Nan , Ingo Molnar , Namhyung Kim , "linux-kernel@vger.kernel.org" , Li Zefan Subject: Re: [PATCH] perf callchain: separate eh/debug frame offset cache. Message-ID: <20150319140352.GJ2983@kernel.org> References: <55028BA0.1030701@huawei.com> <20150313094444.GB10836@danjae.skbroadband> <550A6AA0.1000005@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <550A6AA0.1000005@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Thu, Mar 19, 2015 at 02:20:16PM +0800, Wang Nan escreveu: > Hi Ingo, > >> Could you please collect this patch? It fixes a bug which prevent unwind for ARM. Hi Jiri, can I have your Acked-by for this change, since you worked on this unwind code the most? - Arnaldo > > On 2015/3/13 17:44, Namhyung Kim wrote: > > Hi Wang, > > > > On Fri, Mar 13, 2015 at 03:02:56PM +0800, Wang Nan wrote: > >> Commit f1f13af99a90 ("perf callchain: Cache eh/debug frame offset for > >> dwarf unwind") introduces a cache for .debug_frame and .eh_frame_hdr. > >> Unfortunately, it makes them share a same cache (dso->frame_offset). > >> Which causes unwind failure on ARM: > >> > >> $ perf test unwind > >> Test dwarf unwind: FAILED! > >> > >> The reason is that, if a dso has '.debug_frame' but doesn't have > >> '.eh_frame_hdr' (like ARM), dso->frame_offset will be filled by offset > >> of '.debug_frame' during the first time calling of find_proc_info() -> > >> read_unwind_spec_debug_frame(), and be regarded to '.eh_frame_hdr' when > >> the second time calling of find_proc_info() -> > >> read_unwind_spec_eh_frame(), since '.eh_frame_hdr' is checked prior to > >> '.debug_frame'. > >> > >> This patch solves the problem by creating two cache fields for > >> '.eh_frame_hdr' and '.debug_frame'. > >> > >> Signed-off-by: Wang Nan > > > > Acked-by: Namhyung Kim > > > > Thanks, > > Namhyung > > > > > >> --- > >> tools/perf/util/dso.h | 3 ++- > >> tools/perf/util/unwind-libunwind.c | 8 ++++---- > >> 2 files changed, 6 insertions(+), 5 deletions(-) > >> > >> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > >> index ced9284..408c65f 100644 > >> --- a/tools/perf/util/dso.h > >> +++ b/tools/perf/util/dso.h > >> @@ -139,7 +139,8 @@ struct dso { > >> u32 status_seen; > >> size_t file_size; > >> struct list_head open_entry; > >> - u64 frame_offset; > >> + u64 debug_frame_offset; > >> + u64 eh_frame_hdr_offset; > >> } data; > >> > >> union { /* Tool specific area */ > >> diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c > >> index e3c40a5..7b09a44 100644 > >> --- a/tools/perf/util/unwind-libunwind.c > >> +++ b/tools/perf/util/unwind-libunwind.c > >> @@ -266,7 +266,7 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine, > >> u64 *fde_count) > >> { > >> int ret = -EINVAL, fd; > >> - u64 offset = dso->data.frame_offset; > >> + u64 offset = dso->data.eh_frame_hdr_offset; > >> > >> if (offset == 0) { > >> fd = dso__data_fd(dso, machine); > >> @@ -275,7 +275,7 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine, > >> > >> /* Check the .eh_frame section for unwinding info */ > >> offset = elf_section_offset(fd, ".eh_frame_hdr"); > >> - dso->data.frame_offset = offset; > >> + dso->data.eh_frame_hdr_offset = offset; > >> } > >> > >> if (offset) > >> @@ -291,7 +291,7 @@ static int read_unwind_spec_debug_frame(struct dso *dso, > >> struct machine *machine, u64 *offset) > >> { > >> int fd; > >> - u64 ofs = dso->data.frame_offset; > >> + u64 ofs = dso->data.debug_frame_offset; > >> > >> if (ofs == 0) { > >> fd = dso__data_fd(dso, machine); > >> @@ -300,7 +300,7 @@ static int read_unwind_spec_debug_frame(struct dso *dso, > >> > >> /* Check the .debug_frame section for unwinding info */ > >> ofs = elf_section_offset(fd, ".debug_frame"); > >> - dso->data.frame_offset = ofs; > >> + dso->data.debug_frame_offset = ofs; > >> } > >> > >> *offset = ofs; > >> -- > >> 1.8.3.4 > >> >