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 <masami.hiramatsu...@hitachi.com>
>> ---
>>  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...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to