* [PATCH 3/5] perf: Catch and handle out-of-date hist entry maps.
@ 2012-03-25 20:28 David Miller
2012-03-27 7:14 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2012-03-25 20:28 UTC (permalink / raw)
To: acme; +Cc: linux-kernel
When a process exec()'s, all the maps are retired, but we keep the
hist entries around which hold references to those outdated maps.
If the same library gets mapped in for which we have hist entries, a
new map will be created. But when we take a perf entry hit within
that map, we'll find the existing hist entry with the older map.
This causes symbol translations to be done incorrectly. For example,
the perf entry processing will lookup the correct uptodate map entry
and use that to calculate the symbol and DSO relative address. But
later when we update the histogram we'll translate the address using
the outdated map file instead leading to conditions such as
out-of-range offsets in symbol__inc_addr_samples() which now has an
assertion for this situation.
Therefore, update the map of the hist_entry dynamically at
lookup/creation time.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
tools/perf/util/hist.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 3dc99a9..b7ab3b9 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -253,6 +253,15 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
if (!cmp) {
he->period += period;
++he->nr_events;
+
+ /* If the map of an existing hist_entry has
+ * become out-of-date due to an exec() or
+ * similar, update it. Otherwise we will
+ * mis-adjust symbol addresses when computing
+ * the history counter to increment.
+ */
+ if (he->ms.map != entry->ms.map)
+ he->ms.map = entry->ms.map;
goto out;
}
--
1.7.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 3/5] perf: Catch and handle out-of-date hist entry maps.
2012-03-25 20:28 [PATCH 3/5] perf: Catch and handle out-of-date hist entry maps David Miller
@ 2012-03-27 7:14 ` David Miller
2012-04-13 18:03 ` [tip:perf/core] perf hists: " tip-bot for David Miller
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2012-03-27 7:14 UTC (permalink / raw)
To: acme; +Cc: linux-kernel
From: David Miller <davem@davemloft.net>
Date: Sun, 25 Mar 2012 16:28:18 -0400 (EDT)
> @@ -253,6 +253,15 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
> if (!cmp) {
> he->period += period;
> ++he->nr_events;
> +
> + /* If the map of an existing hist_entry has
> + * become out-of-date due to an exec() or
> + * similar, update it. Otherwise we will
> + * mis-adjust symbol addresses when computing
> + * the history counter to increment.
> + */
> + if (he->ms.map != entry->ms.map)
> + he->ms.map = entry->ms.map;
> goto out;
This unfortunately has a bug, when we hook up a map to a hist_entry we
must mark it referenced. Otherwise it can get freed up prematurely.
Here's a new version of this patch:
--------------------
perf: Catch and handle out-of-date hist entry maps.
When a process exec()'s, all the maps are retired, but we keep the
hist entries around which hold references to those outdated maps.
If the same library gets mapped in for which we have hist entries, a
new map will be created. But when we take a perf entry hit within
that map, we'll find the existing hist entry with the older map.
This causes symbol translations to be done incorrectly. For example,
the perf entry processing will lookup the correct uptodate map entry
and use that to calculate the symbol and DSO relative address. But
later when we update the histogram we'll translate the address using
the outdated map file instead leading to conditions such as
out-of-range offsets in symbol__inc_addr_samples() which now has an
assertion for this situation.
Therefore, update the map of the hist_entry dynamically at
lookup/creation time.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
tools/perf/util/hist.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 3dc99a9..bf0f259 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -253,6 +253,18 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
if (!cmp) {
he->period += period;
++he->nr_events;
+
+ /* If the map of an existing hist_entry has
+ * become out-of-date due to an exec() or
+ * similar, update it. Otherwise we will
+ * mis-adjust symbol addresses when computing
+ * the history counter to increment.
+ */
+ if (he->ms.map != entry->ms.map) {
+ he->ms.map = entry->ms.map;
+ if (he->ms.map)
+ he->ms.map->referenced = true;
+ }
goto out;
}
--
1.7.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [tip:perf/core] perf hists: Catch and handle out-of-date hist entry maps.
2012-03-27 7:14 ` David Miller
@ 2012-04-13 18:03 ` tip-bot for David Miller
0 siblings, 0 replies; 3+ messages in thread
From: tip-bot for David Miller @ 2012-04-13 18:03 UTC (permalink / raw)
To: linux-tip-commits; +Cc: acme, linux-kernel, hpa, mingo, tglx, davem
Commit-ID: 63fa471dd49e9c9ce029d910d1024330d9b1b145
Gitweb: http://git.kernel.org/tip/63fa471dd49e9c9ce029d910d1024330d9b1b145
Author: David Miller <davem@davemloft.net>
AuthorDate: Tue, 27 Mar 2012 03:14:18 -0400
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 Apr 2012 18:53:47 -0300
perf hists: Catch and handle out-of-date hist entry maps.
When a process exec()'s, all the maps are retired, but we keep the hist
entries around which hold references to those outdated maps.
If the same library gets mapped in for which we have hist entries, a new
map will be created. But when we take a perf entry hit within that map,
we'll find the existing hist entry with the older map.
This causes symbol translations to be done incorrectly. For example,
the perf entry processing will lookup the correct uptodate map entry and
use that to calculate the symbol and DSO relative address. But later
when we update the histogram we'll translate the address using the
outdated map file instead leading to conditions such as out-of-range
offsets in symbol__inc_addr_samples().
Therefore, update the map of the hist_entry dynamically at lookup/
creation time.
Signed-off-by: David S. Miller <davem@davemloft.net>
Cc: stable@kernel.org
Link: http://lkml.kernel.org/r/20120327.031418.1220315351537060808.davem@davemloft.net
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/hist.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 2ec4b60..9f6d630 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -256,6 +256,18 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
if (!cmp) {
he->period += period;
++he->nr_events;
+
+ /* If the map of an existing hist_entry has
+ * become out-of-date due to an exec() or
+ * similar, update it. Otherwise we will
+ * mis-adjust symbol addresses when computing
+ * the history counter to increment.
+ */
+ if (he->ms.map != entry->ms.map) {
+ he->ms.map = entry->ms.map;
+ if (he->ms.map)
+ he->ms.map->referenced = true;
+ }
goto out;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-13 18:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-25 20:28 [PATCH 3/5] perf: Catch and handle out-of-date hist entry maps David Miller
2012-03-27 7:14 ` David Miller
2012-04-13 18:03 ` [tip:perf/core] perf hists: " tip-bot for David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).