linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* perf backtraces off-by-1
@ 2012-08-24 22:13 Arun Sharma
  2012-08-26 16:10 ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Arun Sharma @ 2012-08-24 22:13 UTC (permalink / raw)
  To: LKML
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Benjamin Redelings,
	Corey Ashford, Cyrill Gorcunov, Frank Ch. Eigler,
	Frederic Weisbecker, Ingo Molnar, Masami Hiramatsu,
	Paul Mackerras, Peter Zijlstra, Robert Richter, Stephane Eranian,
	Tom Zanussi, Ulrich Drepper

Some of our language runtimes like to map IP addresses in perf backtrace 
to specific byte codes. The way things stand now, the addresses on the 
backtrace are return addresses, rather than the caller. I think this 
issue may be present for other unusual call/return sequences where the 
user may be more interested in the calling instruction rather than the 
instruction control flow would return to.

A simple hack such as the one below makes our JIT guys happy. But the
code is not right if there was an asynchronous transfer of control (eg:
signal handler or interrupt).

libunwind contains similar code, but has the additional info in the 
unwind information to recognize async control transfer.

Wondering if this has been discussed before. One option is to support 
this for user mode only, with code to detect signal frames. Any other ideas?

        -Arun

--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -296,6 +296,7 @@ int machine__resolve_callchain(struct machine *self, 
struct perf_evsel *evsel,
	u8 cpumode = PERF_RECORD_MISC_USER;
	unsigned int i;
	int err;
+	int async;

	callchain_cursor_reset(&evsel->hists.callchain_cursor);

@@ -322,6 +323,11 @@ int machine__resolve_callchain(struct machine 
*self, struct perf_evsel *evsel,
			continue;
		}

+		/* XXX: check if this was an async control transfer */
+		async = 0;
+                if (!async) {
+			ip--;
+		}
		al.filtered = false;
		thread__find_addr_location(thread, self, cpumode,
					   MAP__FUNCTION, ip, &al, NULL);



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: perf backtraces off-by-1
  2012-08-24 22:13 perf backtraces off-by-1 Arun Sharma
@ 2012-08-26 16:10 ` Peter Zijlstra
  2012-08-26 17:52   ` Arun Sharma
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2012-08-26 16:10 UTC (permalink / raw)
  To: Arun Sharma
  Cc: LKML, Jiri Olsa, Arnaldo Carvalho de Melo, Benjamin Redelings,
	Corey Ashford, Cyrill Gorcunov, Frank Ch. Eigler,
	Frederic Weisbecker, Ingo Molnar, Masami Hiramatsu,
	Paul Mackerras, Robert Richter, Stephane Eranian, Tom Zanussi,
	Ulrich Drepper

On Fri, 2012-08-24 at 15:13 -0700, Arun Sharma wrote:
> 
> Wondering if this has been discussed before.

Not that I can recall.

>  One option is to support 
> this for user mode only, with code to detect signal frames. Any other
> ideas?
> 
I guess we'd need to see what that patch would look like... :-)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: perf backtraces off-by-1
  2012-08-26 16:10 ` Peter Zijlstra
@ 2012-08-26 17:52   ` Arun Sharma
  2012-08-28 16:34     ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Arun Sharma @ 2012-08-26 17:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: LKML, Jiri Olsa, Arnaldo Carvalho de Melo, Benjamin Redelings,
	Corey Ashford, Cyrill Gorcunov, Frank Ch. Eigler,
	Frederic Weisbecker, Ingo Molnar, Masami Hiramatsu,
	Paul Mackerras, Robert Richter, Stephane Eranian, Tom Zanussi,
	Ulrich Drepper


On 8/26/12 9:10 AM, Peter Zijlstra wrote:
> On Fri, 2012-08-24 at 15:13 -0700, Arun Sharma wrote:
>
>>   One option is to support
>> this for user mode only, with code to detect signal frames. Any other
>> ideas?
>>
> I guess we'd need to see what that patch would look like... :-)
>

It used to look like this:

http://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=commitdiff;h=92cc7fd78a5a79c4bb5f85bfb7d7fb025df9cd5a

These days we just look at dwarf augmentation string:

http://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=blob;f=src/dwarf/Gfde.c;h=8659624b0320c514057861a259b6efe1b605bbf3;hb=HEAD#l189

  -Arun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: perf backtraces off-by-1
  2012-08-26 17:52   ` Arun Sharma
@ 2012-08-28 16:34     ` Peter Zijlstra
  2012-08-28 17:33       ` Arun Sharma
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2012-08-28 16:34 UTC (permalink / raw)
  To: Arun Sharma
  Cc: LKML, Jiri Olsa, Arnaldo Carvalho de Melo, Benjamin Redelings,
	Corey Ashford, Cyrill Gorcunov, Frank Ch. Eigler,
	Frederic Weisbecker, Ingo Molnar, Masami Hiramatsu,
	Paul Mackerras, Robert Richter, Stephane Eranian, Tom Zanussi,
	Ulrich Drepper

On Sun, 2012-08-26 at 10:52 -0700, Arun Sharma wrote:
> On 8/26/12 9:10 AM, Peter Zijlstra wrote:
> > On Fri, 2012-08-24 at 15:13 -0700, Arun Sharma wrote:
> >
> >>   One option is to support
> >> this for user mode only, with code to detect signal frames. Any other
> >> ideas?
> >>
> > I guess we'd need to see what that patch would look like... :-)
> >
> 
> It used to look like this:
> 
> http://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=commitdiff;h=92cc7fd78a5a79c4bb5f85bfb7d7fb025df9cd5a

Hmm, that's not too bad, but a long stretch from pretty ;-)

How would you 'encode' this in the perf callchain data? 

> These days we just look at dwarf augmentation string:
> 
> http://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=blob;f=src/dwarf/Gfde.c;h=8659624b0320c514057861a259b6efe1b605bbf3;hb=HEAD#l189

Right, except of course we don't have that in kernel..

BTW, are you in San Diego for LinuxCon/LPC etc?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: perf backtraces off-by-1
  2012-08-28 16:34     ` Peter Zijlstra
@ 2012-08-28 17:33       ` Arun Sharma
  0 siblings, 0 replies; 5+ messages in thread
From: Arun Sharma @ 2012-08-28 17:33 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: LKML, Jiri Olsa, Arnaldo Carvalho de Melo, Benjamin Redelings,
	Corey Ashford, Cyrill Gorcunov, Frank Ch. Eigler,
	Frederic Weisbecker, Ingo Molnar, Masami Hiramatsu,
	Paul Mackerras, Robert Richter, Stephane Eranian, Tom Zanussi,
	Ulrich Drepper

On 8/28/12 9:34 AM, Peter Zijlstra wrote:
>> It used to look like this:
>>
>> http://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=commitdiff;h=92cc7fd78a5a79c4bb5f85bfb7d7fb025df9cd5a
>
> Hmm, that's not too bad, but a long stretch from pretty ;-)
>
> How would you 'encode' this in the perf callchain data?
>
>> These days we just look at dwarf augmentation string:
>>
>> http://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=blob;f=src/dwarf/Gfde.c;h=8659624b0320c514057861a259b6efe1b605bbf3;hb=HEAD#l189
>
> Right, except of course we don't have that in kernel..
>

The ip-- transformation could happen in user space. The kernel doesn't 
have to know any of this :)

  -Arun


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-08-28 17:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-24 22:13 perf backtraces off-by-1 Arun Sharma
2012-08-26 16:10 ` Peter Zijlstra
2012-08-26 17:52   ` Arun Sharma
2012-08-28 16:34     ` Peter Zijlstra
2012-08-28 17:33       ` Arun Sharma

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).