Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754589AbbFOS5z (ORCPT ); Mon, 15 Jun 2015 14:57:55 -0400 Received: from mail.kernel.org ([198.145.29.136]:50706 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752083AbbFOS5s (ORCPT ); Mon, 15 Jun 2015 14:57:48 -0400 Date: Mon, 15 Jun 2015 15:57:42 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: Naohiro Aota , Peter Zijlstra , Linux Kernel Mailing List , David Ahern , namhyung@kernel.org, Jiri Olsa , Ingo Molnar Subject: Re: [PATCH perf/core v2 2/3] [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map Message-ID: <20150615185742.GB5845@kernel.org> References: <20150613013113.24402.80334.stgit@localhost.localdomain> <20150613013118.24402.38076.stgit@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150613013118.24402.38076.stgit@localhost.localdomain> 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 Content-Length: 8878 Lines: 261 Em Sat, Jun 13, 2015 at 10:31:18AM +0900, Masami Hiramatsu escreveu: > Fix to check both of non-exist symbols and kprobe blacklist symbols at > symbol-map based converting too. > > E.g. without this fix, perf-probe failes to write the event. > ---- > # perf probe vfs_caches_init_early > Added new event: > Failed to write event: Invalid argument > Error: Failed to add events. > ---- Well, here, after I applied 1/3 in this series, before this patch, I get: [root@zoo ~]# grep vfs_caches_init_early /proc/kallsyms ffffffff81d8e60f T vfs_caches_init_early [root@zoo ~]# perf probe vfs_caches_init_early vfs_caches_init_early+0 is out of .text, skip it. Probe point 'vfs_caches_init_early' not found. Error: Failed to add events. [root@zoo ~]# What am I doind wrong? I am getting the message you said I should get after applying your patch, before I apply it... /me confused :-\ First patch applied, waiting for you on this and the next. - Arnaldo > > This fixes it to catch the error before adding probes. > ---- > # perf probe vfs_caches_init_early > vfs_caches_init_early is out of .text, skip it. > Error: Failed to add events. > ---- > > Signed-off-by: Masami Hiramatsu > --- > tools/perf/util/probe-event.c | 113 ++++++++++++++++++++++++++--------------- > 1 file changed, 71 insertions(+), 42 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index c4ab588..85c8207 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -246,6 +246,20 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs) > clear_probe_trace_event(tevs + i); > } > > +static bool kprobe_blacklist__listed(unsigned long address); > +static bool kprobe_warn_out_range(const char *symbol, unsigned long address) > +{ > + /* Get the address of _etext for checking non-probable text symbol */ > + if (kernel_get_symbol_address_by_name("_etext", false) < address) > + pr_warning("%s is out of .text, skip it.\n", symbol); > + else if (kprobe_blacklist__listed(address)) > + pr_warning("%s is blacklisted function, skip it.\n", symbol); > + else > + return false; > + > + return true; > +} > + > #ifdef HAVE_DWARF_SUPPORT > > static int kernel_get_module_dso(const char *module, struct dso **pdso) > @@ -559,7 +573,6 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs, > bool uprobe) > { > struct ref_reloc_sym *reloc_sym; > - u64 etext_addr; > char *tmp; > int i, skipped = 0; > > @@ -575,31 +588,28 @@ 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 */ > - etext_addr = kernel_get_symbol_address_by_name("_etext", false); > > for (i = 0; i < ntevs; i++) { > - if (tevs[i].point.address && !tevs[i].point.retprobe) { > - /* If we found a wrong one, mark it by NULL symbol */ > - if (etext_addr < tevs[i].point.address) { > - pr_warning("%s+%lu is out of .text, skip it.\n", > - tevs[i].point.symbol, tevs[i].point.offset); > - tmp = NULL; > - skipped++; > - } else { > - tmp = strdup(reloc_sym->name); > - if (!tmp) > - return -ENOMEM; > - } > - /* If we have no realname, use symbol for it */ > - if (!tevs[i].point.realname) > - tevs[i].point.realname = tevs[i].point.symbol; > - else > - free(tevs[i].point.symbol); > - tevs[i].point.symbol = tmp; > - tevs[i].point.offset = tevs[i].point.address - > - reloc_sym->unrelocated_addr; > + if (!tevs[i].point.address || tevs[i].point.retprobe) > + continue; > + /* If we found a wrong one, mark it by NULL symbol */ > + if (kprobe_warn_out_range(tevs[i].point.symbol, > + tevs[i].point.address)) { > + tmp = NULL; > + skipped++; > + } else { > + tmp = strdup(reloc_sym->name); > + if (!tmp) > + return -ENOMEM; > } > + /* If we have no realname, use symbol for it */ > + if (!tevs[i].point.realname) > + tevs[i].point.realname = tevs[i].point.symbol; > + else > + free(tevs[i].point.symbol); > + tevs[i].point.symbol = tmp; > + tevs[i].point.offset = tevs[i].point.address - > + reloc_sym->unrelocated_addr; > } > return skipped; > } > @@ -2126,6 +2136,27 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist, > return NULL; > } > > +static LIST_HEAD(kprobe_blacklist); > + > +static void kprobe_blacklist__init(void) > +{ > + if (!list_empty(&kprobe_blacklist)) > + return; > + > + if (kprobe_blacklist__load(&kprobe_blacklist) < 0) > + pr_debug("No kprobe blacklist support, ignored\n"); > +} > + > +static void kprobe_blacklist__release(void) > +{ > + kprobe_blacklist__delete(&kprobe_blacklist); > +} > + > +static bool kprobe_blacklist__listed(unsigned long address) > +{ > + return !!kprobe_blacklist__find_by_address(&kprobe_blacklist, address); > +} > + > static int perf_probe_event__sprintf(struct perf_probe_event *pev, > const char *module, > struct strbuf *result) > @@ -2409,8 +2440,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, > char buf[64]; > const char *event, *group; > struct strlist *namelist; > - LIST_HEAD(blacklist); > - struct kprobe_blacklist_node *node; > bool safename; > > if (pev->uprobes) > @@ -2430,28 +2459,15 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, > ret = -ENOMEM; > goto close_out; > } > - /* Get kprobe blacklist if exists */ > - if (!pev->uprobes) { > - ret = kprobe_blacklist__load(&blacklist); > - if (ret < 0) > - pr_debug("No kprobe blacklist support, ignored\n"); > - } > > safename = (pev->point.function && !strisglob(pev->point.function)); > ret = 0; > pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":"); > for (i = 0; i < ntevs; i++) { > tev = &tevs[i]; > - /* Skip if the symbol is out of .text (marked previously) */ > + /* Skip if the symbol is out of .text or blacklisted */ > if (!tev->point.symbol) > continue; > - /* Ensure that the address is NOT blacklisted */ > - node = kprobe_blacklist__find_by_address(&blacklist, > - tev->point.address); > - if (node) { > - pr_warning("Warning: Skipped probing on blacklisted function: %s\n", node->symbol); > - continue; > - } > > if (pev->event) > event = pev->event; > @@ -2513,7 +2529,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, > tev->event); > } > > - kprobe_blacklist__delete(&blacklist); > strlist__delete(namelist); > close_out: > close(fd); > @@ -2563,7 +2578,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev, > struct perf_probe_point *pp = &pev->point; > struct probe_trace_point *tp; > int num_matched_functions; > - int ret, i, j; > + int ret, i, j, skipped = 0; > > map = get_target_map(pev->target, pev->uprobes); > if (!map) { > @@ -2631,7 +2646,12 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev, > } > /* Add one probe point */ > tp->address = map->unmap_ip(map, sym->start) + pp->offset; > - if (reloc_sym) { > + /* If we found a wrong one, mark it by NULL symbol */ > + if (!pev->uprobes && > + kprobe_warn_out_range(sym->name, tp->address)) { > + tp->symbol = NULL; /* Skip it */ > + skipped++; > + } else if (reloc_sym) { > tp->symbol = strdup_or_goto(reloc_sym->name, nomem_out); > tp->offset = tp->address - reloc_sym->addr; > } else { > @@ -2667,6 +2687,10 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev, > } > arch__fix_tev_from_maps(pev, tev, map); > } > + if (ret == skipped) { > + ret = -ENOENT; > + goto err_out; > + } > > out: > put_target_map(map, pev->uprobes); > @@ -2737,6 +2761,9 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs) > /* Loop 1: convert all events */ > for (i = 0; i < npevs; i++) { > pkgs[i].pev = &pevs[i]; > + /* Init kprobe blacklist if needed */ > + if (!pkgs[i].pev->uprobes) > + kprobe_blacklist__init(); > /* Convert with or without debuginfo */ > ret = convert_to_probe_trace_events(pkgs[i].pev, > &pkgs[i].tevs); > @@ -2744,6 +2771,8 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs) > goto end; > pkgs[i].ntevs = ret; > } > + /* This just release blacklist only if allocated */ > + kprobe_blacklist__release(); > > /* Loop 2: add all events */ > for (i = 0; i < npevs; i++) { -- 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/