* [PATCH 1/5] perf symbols: Fix dso lookup by long name and missing buildids
2015-11-13 20:57 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
@ 2015-11-13 20:57 ` Arnaldo Carvalho de Melo
2015-11-13 20:57 ` [PATCH 2/5] perf buildid-list: Requires ordered events Arnaldo Carvalho de Melo
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-13 20:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Adrian Hunter, Don Zickus, Douglas Hatch,
Jiri Olsa, Namhyung Kim, Peter Zijlstra, Scott J Norton,
Waiman Long, Arnaldo Carvalho de Melo
From: Adrian Hunter <adrian.hunter@intel.com>
Commit 4598a0a6d22f ("perf symbols: Improve DSO long names lookup speed
with rbtree") Added a tree to lookup dsos by long name. That tree gets
corrupted whenever a dso long name is changed because the tree is not
updated.
One effect of that is buildid-list does not work with the 'with-hits'
option because dso lookup fails and results in two structs for the same
dso. The first has the buildid but no hits, the second has hits but no
buildid. e.g.
Before:
$ tools/perf/perf record ls
arch certs CREDITS Documentation firmware include
ipc Kconfig lib Makefile net REPORTING-BUGS
scripts sound usr block COPYING crypto
drivers fs init Kbuild kernel MAINTAINERS
mm README samples security tools virt
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.012 MB perf.data (11 samples) ]
$ tools/perf/perf buildid-list
574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms]
30c94dc66a1fe95180c3d68d2b89e576d5ae213c /lib/x86_64-linux-gnu/libc-2.19.so
$ tools/perf/perf buildid-list -H
574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms]
0000000000000000000000000000000000000000 /lib/x86_64-linux-gnu/libc-2.19.so
After:
$ tools/perf/perf buildid-list -H
574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms]
30c94dc66a1fe95180c3d68d2b89e576d5ae213c /lib/x86_64-linux-gnu/libc-2.19.so
The fix is to record the root of the tree on the dso so that
dso__set_long_name() can update the tree when the long name changes.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Douglas Hatch <doug.hatch@hp.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Scott J Norton <scott.norton@hp.com>
Cc: Waiman Long <Waiman.Long@hp.com>
Fixes: 4598a0a6d22f ("perf symbols: Improve DSO long names lookup speed with rbtree")
Link: http://lkml.kernel.org/r/1447408112-1920-2-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/dso.c | 17 +++++++++++++++++
tools/perf/util/dso.h | 1 +
tools/perf/util/machine.c | 1 +
3 files changed, 19 insertions(+)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 7c0c08386a1d..425df5c86c9c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -933,6 +933,7 @@ static struct dso *__dso__findlink_by_longname(struct rb_root *root,
/* Add new node and rebalance tree */
rb_link_node(&dso->rb_node, parent, p);
rb_insert_color(&dso->rb_node, root);
+ dso->root = root;
}
return NULL;
}
@@ -945,15 +946,30 @@ static inline struct dso *__dso__find_by_longname(struct rb_root *root,
void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated)
{
+ struct rb_root *root = dso->root;
+
if (name == NULL)
return;
if (dso->long_name_allocated)
free((char *)dso->long_name);
+ if (root) {
+ rb_erase(&dso->rb_node, root);
+ /*
+ * __dso__findlink_by_longname() isn't guaranteed to add it
+ * back, so a clean removal is required here.
+ */
+ RB_CLEAR_NODE(&dso->rb_node);
+ dso->root = NULL;
+ }
+
dso->long_name = name;
dso->long_name_len = strlen(name);
dso->long_name_allocated = name_allocated;
+
+ if (root)
+ __dso__findlink_by_longname(root, dso, NULL);
}
void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated)
@@ -1046,6 +1062,7 @@ struct dso *dso__new(const char *name)
dso->kernel = DSO_TYPE_USER;
dso->needs_swap = DSO_SWAP__UNSET;
RB_CLEAR_NODE(&dso->rb_node);
+ dso->root = NULL;
INIT_LIST_HEAD(&dso->node);
INIT_LIST_HEAD(&dso->data.open_entry);
pthread_mutex_init(&dso->lock, NULL);
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index fc8db9c764ac..45ec4d0a50ed 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -135,6 +135,7 @@ struct dso {
pthread_mutex_t lock;
struct list_head node;
struct rb_node rb_node; /* rbtree node sorted by long name */
+ struct rb_root *root; /* root of rbtree that rb_node is in */
struct rb_root symbols[MAP__NR_TYPES];
struct rb_root symbol_names[MAP__NR_TYPES];
struct {
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 5ef90be2a249..8b303ff20289 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -91,6 +91,7 @@ static void dsos__purge(struct dsos *dsos)
list_for_each_entry_safe(pos, n, &dsos->head, node) {
RB_CLEAR_NODE(&pos->rb_node);
+ pos->root = NULL;
list_del_init(&pos->node);
dso__put(pos);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/5] perf buildid-list: Requires ordered events
2015-11-13 20:57 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
2015-11-13 20:57 ` [PATCH 1/5] perf symbols: Fix dso lookup by long name and missing buildids Arnaldo Carvalho de Melo
@ 2015-11-13 20:57 ` Arnaldo Carvalho de Melo
2015-11-13 20:57 ` [PATCH 3/5] perf inject: Also re-pipe lost_samples event Arnaldo Carvalho de Melo
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-13 20:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Adrian Hunter, Jiri Olsa, Arnaldo Carvalho de Melo
From: Adrian Hunter <adrian.hunter@intel.com>
'perf buildid-list' processes events to determine hits (i.e. with-hits
option). That may not work if events are not sorted in order. i.e. MMAP
events must be processed before the samples that depend on them so that
sample processing can 'hit' the DSO to which the MMAP refers.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/1447408112-1920-3-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/build-id.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index d909459fb54c..217b5a60e2ab 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -76,6 +76,7 @@ struct perf_tool build_id__mark_dso_hit_ops = {
.exit = perf_event__exit_del_thread,
.attr = perf_event__process_attr,
.build_id = perf_event__process_build_id,
+ .ordered_events = true,
};
int build_id__sprintf(const u8 *build_id, int len, char *bf)
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/5] perf inject: Also re-pipe lost_samples event
2015-11-13 20:57 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
2015-11-13 20:57 ` [PATCH 1/5] perf symbols: Fix dso lookup by long name and missing buildids Arnaldo Carvalho de Melo
2015-11-13 20:57 ` [PATCH 2/5] perf buildid-list: Requires ordered events Arnaldo Carvalho de Melo
@ 2015-11-13 20:57 ` Arnaldo Carvalho de Melo
2015-11-13 20:57 ` [PATCH 4/5] perf probe: Fix memory leaking on failure by clearing all probe_trace_events Arnaldo Carvalho de Melo
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-13 20:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Adrian Hunter, Jiri Olsa, Arnaldo Carvalho de Melo
From: Adrian Hunter <adrian.hunter@intel.com>
perf inject must re-pipe all events otherwise they get dropped from the
output file.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/1447408112-1920-4-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-inject.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 0a945d2e8ca5..99d127fe9c35 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -675,6 +675,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
.fork = perf_event__repipe,
.exit = perf_event__repipe,
.lost = perf_event__repipe,
+ .lost_samples = perf_event__repipe,
.aux = perf_event__repipe,
.itrace_start = perf_event__repipe,
.context_switch = perf_event__repipe,
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/5] perf probe: Fix memory leaking on failure by clearing all probe_trace_events
2015-11-13 20:57 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2015-11-13 20:57 ` [PATCH 3/5] perf inject: Also re-pipe lost_samples event Arnaldo Carvalho de Melo
@ 2015-11-13 20:57 ` Arnaldo Carvalho de Melo
2015-11-13 20:57 ` [PATCH 5/5] perf probe: Clear probe_trace_event when add_probe_trace_event() fails Arnaldo Carvalho de Melo
2015-11-18 5:57 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-13 20:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Masami Hiramatsu, Alexei Starovoitov, Zefan Li,
pi3orama, Arnaldo Carvalho de Melo
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Fix memory leaking on the debuginfo__find_trace_events() failure path
which frees an array of probe_trace_events but doesn't clears all the
allocated sub-structures and strings.
So, before doing zfree(tevs), clear all the array elements which may
have allocated resources.
Reported-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1447417761-156094-2-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/probe-finder.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index bd8f03de5e40..63993d7e0fac 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1246,7 +1246,7 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
struct trace_event_finder tf = {
.pf = {.pev = pev, .callback = add_probe_trace_event},
.max_tevs = probe_conf.max_probes, .mod = dbg->mod};
- int ret;
+ int ret, i;
/* Allocate result tevs array */
*tevs = zalloc(sizeof(struct probe_trace_event) * tf.max_tevs);
@@ -1258,6 +1258,8 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
ret = debuginfo__find_probes(dbg, &tf.pf);
if (ret < 0) {
+ for (i = 0; i < tf.ntevs; i++)
+ clear_probe_trace_event(&tf.tevs[i]);
zfree(tevs);
return ret;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5/5] perf probe: Clear probe_trace_event when add_probe_trace_event() fails
2015-11-13 20:57 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2015-11-13 20:57 ` [PATCH 4/5] perf probe: Fix memory leaking on failure by clearing all probe_trace_events Arnaldo Carvalho de Melo
@ 2015-11-13 20:57 ` Arnaldo Carvalho de Melo
2015-11-18 5:57 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-13 20:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Wang Nan, Alexei Starovoitov, Masami Hiramatsu,
Zefan Li, pi3orama, Arnaldo Carvalho de Melo
From: Wang Nan <wangnan0@huawei.com>
When probing with a glob, errors in add_probe_trace_event() won't be
passed to debuginfo__find_trace_events() because it would be modified by
probe_point_search_cb(). It causes a segfault if perf fails to find an
argument for a probe point matched by the glob. For example:
# ./perf probe -v -n 'SyS_dup? oldfd'
probe-definition(0): SyS_dup? oldfd
symbol:SyS_dup? file:(null) line:0 offset:0 return:0 lazy:(null)
parsing arg: oldfd into oldfd
1 arguments
Looking at the vmlinux_path (7 entries long)
Using /lib/modules/4.3.0-rc4+/build/vmlinux for symbols
Open Debuginfo file: /lib/modules/4.3.0-rc4+/build/vmlinux
Try to find probe point from debuginfo.
Matched function: SyS_dup3
found inline addr: 0xffffffff812095c0
Probe point found: SyS_dup3+0
Searching 'oldfd' variable in context.
Converting variable oldfd into trace event.
oldfd type is long int.
found inline addr: 0xffffffff812096d4
Probe point found: SyS_dup2+36
Searching 'oldfd' variable in context.
Failed to find 'oldfd' in this function.
Matched function: SyS_dup3
Probe point found: SyS_dup3+0
Searching 'oldfd' variable in context.
Converting variable oldfd into trace event.
oldfd type is long int.
Matched function: SyS_dup2
Probe point found: SyS_dup2+0
Searching 'oldfd' variable in context.
Converting variable oldfd into trace event.
oldfd type is long int.
Found 4 probe_trace_events.
Opening /sys/kernel/debug/tracing//kprobe_events write=1
Writing event: p:probe/SyS_dup3 _text+2135488 oldfd=%di:s64
Segmentation fault (core dumped)
#
This patch ensures that add_probe_trace_event() doesn't touches
tf->ntevs and tf->tevs if those functions fail.
After the patch:
# perf probe 'SyS_dup? oldfd'
Failed to find 'oldfd' in this function.
Added new events:
probe:SyS_dup3 (on SyS_dup? with oldfd)
probe:SyS_dup3_1 (on SyS_dup? with oldfd)
probe:SyS_dup2 (on SyS_dup? with oldfd)
You can now use it in all perf tools, such as:
perf record -e probe:SyS_dup2 -aR sleep 1
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1447417761-156094-3-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/probe-finder.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 63993d7e0fac..05012bb178d7 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1183,7 +1183,7 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
container_of(pf, struct trace_event_finder, pf);
struct perf_probe_point *pp = &pf->pev->point;
struct probe_trace_event *tev;
- struct perf_probe_arg *args;
+ struct perf_probe_arg *args = NULL;
int ret, i;
/* Check number of tevs */
@@ -1198,19 +1198,23 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
ret = convert_to_trace_point(&pf->sp_die, tf->mod, pf->addr,
pp->retprobe, pp->function, &tev->point);
if (ret < 0)
- return ret;
+ goto end;
tev->point.realname = strdup(dwarf_diename(sc_die));
- if (!tev->point.realname)
- return -ENOMEM;
+ if (!tev->point.realname) {
+ ret = -ENOMEM;
+ goto end;
+ }
pr_debug("Probe point found: %s+%lu\n", tev->point.symbol,
tev->point.offset);
/* Expand special probe argument if exist */
args = zalloc(sizeof(struct perf_probe_arg) * MAX_PROBE_ARGS);
- if (args == NULL)
- return -ENOMEM;
+ if (args == NULL) {
+ ret = -ENOMEM;
+ goto end;
+ }
ret = expand_probe_args(sc_die, pf, args);
if (ret < 0)
@@ -1234,6 +1238,10 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
}
end:
+ if (ret) {
+ clear_probe_trace_event(tev);
+ tf->ntevs--;
+ }
free(args);
return ret;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [GIT PULL 0/5] perf/urgent fixes
2015-11-13 20:57 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2015-11-13 20:57 ` [PATCH 5/5] perf probe: Clear probe_trace_event when add_probe_trace_event() fails Arnaldo Carvalho de Melo
@ 2015-11-18 5:57 ` Ingo Molnar
5 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2015-11-18 5:57 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Adrian Hunter, Alexei Starovoitov, Don Zickus,
Douglas Hatch, Jiri Olsa, Masami Hiramatsu, Namhyung Kim,
Peter Zijlstra, pi3orama, Scott J Norton, Waiman Long, Wang Nan,
Zefan Li, Arnaldo Carvalho de Melo
* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Hi Ingo,
>
> Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit 2059fc7a5a9e667797b8ec503bfb4685afee48d8:
>
> perf symbols: Allow forcing reading of non-root owned files by root (2015-11-12 18:58:18 -0300)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo
>
> for you to fetch changes up to 092b1f0b5f9f797812da0de927c3aa26acbe8762:
>
> perf probe: Clear probe_trace_event when add_probe_trace_event() fails (2015-11-13 12:28:09 -0300)
>
> ----------------------------------------------------------------
> perf/urgent fixes:
>
> - Do not change the key of an object in a rbtree, this time it was
> the one for DSOs lookup by its long_name, and the noticed symptom was
> with 'perf buildid-list --with-hits' (Adrian Hunter)
>
> - 'perf inject' is a pipe, events it doesn't touch should be passed
> on, PERF_RECORD_LOST wasn't, fix it (Adrian Hunter)
>
> - Make 'perf buildid-list' request event ordering, as it needs to
> first get the mmap events to be able to mark wich DSOs had hits
> (Adrian Hunter)
>
> - Fix memory leaks on failure in 'perf probe' (Masami Hiramatsu, Wang Nan)
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> ----------------------------------------------------------------
> Adrian Hunter (3):
> perf symbols: Fix dso lookup by long name and missing buildids
> perf buildid-list: Requires ordered events
> perf inject: Also re-pipe lost_samples event
>
> Masami Hiramatsu (1):
> perf probe: Fix memory leaking on failure by clearing all probe_trace_events
>
> Wang Nan (1):
> perf probe: Clear probe_trace_event when add_probe_trace_event() fails
>
> tools/perf/builtin-inject.c | 1 +
> tools/perf/util/build-id.c | 1 +
> tools/perf/util/dso.c | 17 +++++++++++++++++
> tools/perf/util/dso.h | 1 +
> tools/perf/util/machine.c | 1 +
> tools/perf/util/probe-finder.c | 24 +++++++++++++++++-------
> 6 files changed, 38 insertions(+), 7 deletions(-)
Pulled, thanks a lot Arnaldo!
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread