From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756195AbbFPLyg (ORCPT ); Tue, 16 Jun 2015 07:54:36 -0400 Received: from mail4.hitachi.co.jp ([133.145.228.5]:33090 "EHLO mail4.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754908AbbFPLyV (ORCPT ); Tue, 16 Jun 2015 07:54:21 -0400 X-AuditID: 85900ec0-9e1cab9000001a57-23-55800e576b0e Subject: [PATCH perf/core v3 2/3] [BUGFIX] perf probe: Fix to return error if no-probe is added From: Masami Hiramatsu To: Arnaldo Carvalho de Melo Cc: Naohiro Aota , Peter Zijlstra , Linux Kernel Mailing List , David Ahern , namhyung@kernel.org, Jiri Olsa , Ingo Molnar Date: Tue, 16 Jun 2015 20:50:55 +0900 Message-ID: <20150616115055.19906.31359.stgit@localhost.localdomain> In-Reply-To: <20150616115050.19906.77371.stgit@localhost.localdomain> References: <20150616115050.19906.77371.stgit@localhost.localdomain> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: AAAAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Fix perf probe to return an error if no-probe is added since the given probe point is on the blacklist. To fix this problem, this moves blacklist checking right after finding symbols/probe-points and marks it as skipped. If all the symbols are skipped, perf-probe returns error as it fails to find corresponding probe address. e.g. currently if we give a blacklisted probe is given, ---- # perf probe do_trap && echo 'succeed' Added new event: Warning: Skipped probing on blacklisted function: sync_regs succeed ---- No! it must be failed. With this patch, it correctly failed. ---- # perf probe do_trap && echo 'succeed' do_trap is blacklisted function, skip it. Probe point 'do_trap' not found. Error: Failed to add events. ---- Signed-off-by: Masami Hiramatsu --- Changes in v3: - Rewrite the comment and ensure what was fixed. --- 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++) {