Re: [PATCH perf/core v2 2/3] [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map

From: Arnaldo Carvalho de Melo
Date: Mon Jun 15 2015 - 14:57:58 EST


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 <masami.hiramatsu.pt@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/