Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755462AbbFMBQo (ORCPT ); Fri, 12 Jun 2015 21:16:44 -0400 Received: from [133.145.228.5] ([133.145.228.5]:59864 "EHLO mail4.hitachi.co.jp" rhost-flags-FAIL-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1751330AbbFMBQn (ORCPT ); Fri, 12 Jun 2015 21:16:43 -0400 Message-ID: <557B845F.8040803@hitachi.com> Date: Sat, 13 Jun 2015 10:16:15 +0900 From: Masami Hiramatsu Organization: Hitachi, Ltd., Japan User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Arnaldo Carvalho de Melo CC: Naohiro Aota , Peter Zijlstra , Linux Kernel Mailing List , David Ahern , namhyung@kernel.org, Jiri Olsa , Ingo Molnar Subject: Re: [PATCH perf/core 3/4] [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map References: <20150612050806.20548.12371.stgit@localhost.localdomain> <20150612050827.20548.83722.stgit@localhost.localdomain> <20150612191722.GD6850@kernel.org> In-Reply-To: <20150612191722.GD6850@kernel.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9047 Lines: 262 On 2015/06/13 4:17, Arnaldo Carvalho de Melo wrote: > Em Fri, Jun 12, 2015 at 02:08:27PM +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. >> ---- >> >> 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. >> ---- > > Not applying to my perf/core branch, maybe it needs the first patch in > this series, that is not applied so far? Yes, I'll resend the series without 2/4, with fixing previous bug... Thank you!! > > - Arnaldo > >> 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 7aa3efe..aa2479e 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++) { >> > -- 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 -- 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/