From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756516AbbFPNj5 (ORCPT ); Tue, 16 Jun 2015 09:39:57 -0400 Received: from mail.kernel.org ([198.145.29.136]:50447 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756125AbbFPNjs (ORCPT ); Tue, 16 Jun 2015 09:39:48 -0400 Date: Tue, 16 Jun 2015 10:39:37 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: He Kuang , a.p.zijlstra@chello.nl, mingo@redhat.com, namhyung@kernel.org, wangnan0@huawei.com, linux-kernel@vger.kernel.org Subject: Re: Re: [PATCH] perf probe: Fix failure to probe events on arm Message-ID: <20150616133937.GB10374@kernel.org> References: <1434355613-110216-1-git-send-email-hekuang@huawei.com> <20150615144911.GA5845@kernel.org> <557FBC9D.3070501@hitachi.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <557FBC9D.3070501@hitachi.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 Tue, Jun 16, 2015 at 03:05:17PM +0900, Masami Hiramatsu escreveu: > On 2015/06/15 23:49, Arnaldo Carvalho de Melo wrote: > > Em Mon, Jun 15, 2015 at 08:06:53AM +0000, He Kuang escreveu: > >> Fix failure to probe events on arm, problem is introduced by commit > >> 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of > >> .text"). For some architectures, label '_etext' is not in the .text > >> section(in .notes section for arm/arm64). Label out of .text section is > >> not loaded as symbols and we got a zero value when look up its address, > >> which causes all events be wrongly skiped. > >> > >> This patch uses kernel map->end when failed to get the address of > >> '_etext' and fixes the problem. > > > > Masami, can't we always use map->end then? Can you please take a look at > > this patch and ack/nack it? > > Yeah, it looks good to me, and we finally will replace "_etext" with it :) > > ---- > void map__fixup_end(struct map *map) > { > struct rb_root *symbols = &map->dso->symbols[map->type]; > struct rb_node *nd = rb_last(symbols); > if (nd != NULL) { > struct symbol *sym = rb_entry(nd, struct symbol, rb_node); > map->end = sym->end; > } > } > ---- > So, the map->end shows the end address of the symbols. > > Acked-by: Masami Hiramatsu I can and will apply it, but the question remains, if this approach is good then when should we bother looking at "_etext" at all, i.e. why not just use map->end all the time? I have not thought this thru, this is just based on this ``its ok to use map->end if "_etext" is not found'' idea that He proposed and you acked. - Arnaldo > > Thanks! > > > > > - Arnaldo > > > > > >> Problem can be reproduced on arm as following: > >> > >> # perf probe --add='generic_perform_write' > >> generic_perform_write+0 is out of .text, skip it. > >> Probe point 'generic_perform_write' not found. > >> Error: Failed to add events. Reason: No such file or directory (Code: -2) > >> > >> After this patch: > >> > >> # perf probe --add='generic_perform_write' > >> Added new event: > >> probe:generic_perform_write (on generic_perform_write) > >> > >> You can now use it in all perf tools, such as: > >> > >> perf record -e probe:generic_perform_write -aR sleep 1 > >> > >> Signed-off-by: He Kuang > >> --- > >> tools/perf/util/probe-event.c | 16 +++++++++++++++- > >> 1 file changed, 15 insertions(+), 1 deletion(-) > >> > >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > >> index daa24a2..ee26961 100644 > >> --- a/tools/perf/util/probe-event.c > >> +++ b/tools/perf/util/probe-event.c > >> @@ -575,8 +575,22 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs, > >> pr_warning("Relocated base symbol is not found!\n"); > >> return -EINVAL; > >> } > >> - /* Get the address of _etext for checking non-probable text symbol */ > >> + /* Get the address of _etext for checking non-probable text symbol, > >> + for some architectures (e.g. arm, arm64), _etext is out of .text > >> + section and not loaded as symbols, use kernel map->end instead. > >> + */ > >> etext_addr = kernel_get_symbol_address_by_name("_etext", false); > >> + if (etext_addr == 0) { > >> + struct map *map; > >> + > >> + map = kernel_get_module_map(NULL); > >> + if (!map) { > >> + pr_err("Failed to get a map for kernel\n"); > >> + return -EINVAL; > >> + } > >> + > >> + etext_addr = map->end; > >> + } > >> > >> for (i = 0; i < ntevs; i++) { > >> if (tevs[i].point.address && !tevs[i].point.retprobe) { > >> -- > >> 1.8.5.2 > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > > > -- > Masami HIRAMATSU > Linux Technology Research Center, System Productivity Research Dept. > Center for Technology Innovation - Systems Engineering > Hitachi, Ltd., Research & Development Group > E-mail: masami.hiramatsu.pt@hitachi.com