From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Naohiro Aota <naota@elisp.net>,
Peter Zijlstra <peterz@infradead.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
David Ahern <dsahern@gmail.com>,
namhyung@kernel.org, Jiri Olsa <jolsa@redhat.com>,
Ingo Molnar <mingo@kernel.org>
Subject: [PATCH perf/core v3 2/3] [BUGFIX] perf probe: Fix to return error if no-probe is added
Date: Tue, 16 Jun 2015 20:50:55 +0900 [thread overview]
Message-ID: <20150616115055.19906.31359.stgit@localhost.localdomain> (raw)
In-Reply-To: <20150616115050.19906.77371.stgit@localhost.localdomain>
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 <masami.hiramatsu.pt@hitachi.com>
---
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++) {
next prev parent reply other threads:[~2015-06-16 11:54 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 11:50 [PATCH perf/core v3 0/3] perf-probe bugfixes Masami Hiramatsu
2015-06-16 11:50 ` [PATCH perf/core v3 1/3] [BUGFIX] perf probe: List probes in stdout Masami Hiramatsu
2015-06-16 11:50 ` Masami Hiramatsu [this message]
2015-06-18 8:15 ` [tip:perf/core] perf probe: Fix to return error if no probe is added tip-bot for Masami Hiramatsu
2015-06-16 11:50 ` [PATCH perf/core v3 3/3] [BUGFIX] perf probe: Show usage even if the last event is skipped Masami Hiramatsu
2015-06-16 14:46 ` Arnaldo Carvalho de Melo
2015-06-17 5:31 ` Masami Hiramatsu
2015-06-17 14:58 ` Masami Hiramatsu
2015-06-17 14:58 ` [PATCH perf/core ] perf probe: Speed up perf probe --list by caching debuginfo Masami Hiramatsu
2015-06-18 8:17 ` [tip:perf/core] " tip-bot for Masami Hiramatsu
2015-06-17 15:00 ` Re: [PATCH perf/core v3 3/3] [BUGFIX] perf probe: Show usage even if the last event is skipped Arnaldo Carvalho de Melo
2015-06-18 8:16 ` [tip:perf/core] " tip-bot for Masami Hiramatsu
2015-06-16 13:48 ` [PATCH perf/core v3 0/3] perf-probe bugfixes Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150616115055.19906.31359.stgit@localhost.localdomain \
--to=masami.hiramatsu.pt@hitachi.com \
--cc=acme@kernel.org \
--cc=dsahern@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=naota@elisp.net \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.