mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Paul Mackerras <paulus@samba.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	"K. Prasad" <prasad@linux.vnet.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: [PATCH 8/9] x86: Fixup wrong debug exception frame link in stacktraces
Date: Sun,  6 Dec 2009 08:34:57 +0100	[thread overview]
Message-ID: <1260084898-11686-9-git-send-regression-fweisbec@gmail.com> (raw)
In-Reply-To: <1260084898-11686-1-git-send-regression-fweisbec@gmail.com>

While dumping a stacktrace, the end of the exception stack won't link
the frame pointer to the previous stack.

The interrupted stack will then be considered as unreliable and ignored
by perf, as the frame pointer is unreliable itself.

This happens because we overwrite the frame pointer that links to the
interrupted frame with the address of the exception stack. This is
done in order to reserve space inside.
But rbp has been chosen here only because it is not a scratch register,
so that the address of the exception stack remains in rbp after calling
do_debug(), we can then release the exception stack space without the
need to retrieve its address again.

But we can pick another non-scratch register to do that, so that we
preserve the link to the interrupted stack frame in the stacktraces.

Just randomly choose r12. Every registers are saved just before and
restored just after calling do_debug(). And r12 is not used in the
middle, which makes it a perfect candidate.

Example: perf record -g -a -c 1 -f -e mem:$(tasklist_lock_addr):rw

Before:
    44.18%  [k] _raw_read_lock
            |
            |
            ---  |--6.31%-- waitid
                 |
                 |--4.26%-- writev
                 |
                 |--3.63%-- __select
                 |
                 |--3.15%-- __waitpid
                 |          |
                 |          |--28.57%-- 0x8b52e00000139f
                 |          |
                 |          |--28.57%-- 0x8b52e0000013c6
                 |          |
                 |          |--14.29%-- 0x7fde786dc000
                 |          |
                 |          |--14.29%-- 0x62696c2f7273752f
                 |          |
                 |           --14.29%-- 0x1ea9df800000000
                 |
                 |--3.00%-- __poll

After:

    43.94%  [k] _raw_read_lock
            |
            --- _read_lock
               |
               |--60.53%-- send_sigio
               |          __kill_fasync
               |          kill_fasync
               |          evdev_pass_event
               |          evdev_event
               |          input_pass_event
               |          input_handle_event
               |          input_event
               |          synaptics_process_byte
               |          psmouse_handle_byte
               |          psmouse_interrupt
               |          serio_interrupt
               |          i8042_interrupt
               |          handle_IRQ_event
               |          handle_edge_irq
               |          handle_irq
               |          __irqentry_text_start
               |          ret_from_intr
               |          |
               |          |--30.43%-- __select
               |          |
               |          |--17.39%-- 0x454f15
               |          |
               |          |--13.04%-- __read
               |          |
               |          |--13.04%-- vread_hpet
               |          |
               |          |--13.04%-- _xcb_lock_io
               |          |
               |           --13.04%-- 0x7f630878ce87

Note: it does not only affect perf events but also other stacktraces in
x86-64. They were considered as unreliable once we quit the debug
stack frame.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: "K. Prasad" <prasad@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/kernel/entry_64.S |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 722df1b..0f08a0c 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1076,10 +1076,10 @@ ENTRY(\sym)
 	TRACE_IRQS_OFF
 	movq %rsp,%rdi		/* pt_regs pointer */
 	xorl %esi,%esi		/* no error code */
-	PER_CPU(init_tss, %rbp)
-	subq $EXCEPTION_STKSZ, TSS_ist + (\ist - 1) * 8(%rbp)
+	PER_CPU(init_tss, %r12)
+	subq $EXCEPTION_STKSZ, TSS_ist + (\ist - 1) * 8(%r12)
 	call \do_sym
-	addq $EXCEPTION_STKSZ, TSS_ist + (\ist - 1) * 8(%rbp)
+	addq $EXCEPTION_STKSZ, TSS_ist + (\ist - 1) * 8(%r12)
 	jmp paranoid_exit	/* %ebx: no swapgs flag */
 	CFI_ENDPROC
 END(\sym)
-- 
1.6.2.3


  parent reply	other threads:[~2009-12-06  7:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-06  7:34 [GIT PULL] perf/hw-breakpoints: Various updates Frederic Weisbecker
2009-12-06  7:34 ` [PATCH 1/9] hw-breakpoints: Add two reserved fields for future extensions Frederic Weisbecker
2009-12-06  7:34 ` [PATCH 2/9] perf: Remove pointless union that wraps the hw breakpoint fields Frederic Weisbecker
2009-12-06 10:38   ` Peter Zijlstra
2009-12-07  1:46     ` Frederic Weisbecker
2009-12-06  7:34 ` [PATCH 3/9] perf: Remove unused struct perf_event::event_callback Frederic Weisbecker
2009-12-06  7:34 ` [PATCH 4/9] hw-breakpoints: Drop callback and task parameters from modify helper Frederic Weisbecker
2009-12-06  7:34 ` [PATCH 5/9] hw-breakpoints: Use overflow handler instead of the event callback Frederic Weisbecker
2009-12-06  7:34 ` [PATCH 6/9] perf: Remove the "event" callback from perf events Frederic Weisbecker
2009-12-06 10:40   ` Peter Zijlstra
2009-12-06  7:34 ` [PATCH 7/9] x86/perf: Exclude the debug stack from the callchains Frederic Weisbecker
2009-12-06  7:34 ` Frederic Weisbecker [this message]
2009-12-06  7:34 ` [PATCH 9/9] x86: Fixup wrong irq frame link in stacktraces Frederic Weisbecker
2009-12-06  7:58 ` [GIT PULL] perf/hw-breakpoints: Various updates Ingo Molnar

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=1260084898-11686-9-git-send-regression-fweisbec@gmail.com \
    --to=fweisbec@gmail.com \
    --cc=acme@redhat.com \
    --cc=benh@kernel.crashing.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=prasad@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome