* [PATCH][GIT PULL] powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
@ 2010-12-24 5:46 Steven Rostedt
2010-12-24 21:11 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2010-12-24 5:46 UTC (permalink / raw)
To: LKML; +Cc: Andrew Morton, Joerg Sommer, linuxppc-dev
Ben,
Joerg reported a crash with the irqsoff tracer with PPC32.
Unfortunately, I don't have a ppc32 that I can work with (my kids took
it from me). I posted a test patch on the BZ that was opened for it:
https://bugzilla.kernel.org/show_bug.cgi?id=16573
Anyway, I was also able to reproduce the crash on PPC64. This patch
handles that case.
The following patch is in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
branch: ppc/ftrace
Steven Rostedt (1):
powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
----
arch/powerpc/include/asm/irqflags.h | 40 ++++++++++++++++++++++++++--------
1 files changed, 30 insertions(+), 10 deletions(-)
---------------------------
commit 5025019505da6731f8be13940bb978617599c935
Author: Steven Rostedt <srostedt@redhat.com>
Date: Thu Dec 23 21:07:39 2010 -0800
powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
When an interrupt occurs in userspace, we can call trace_hardirqs_on/off()
With one level stack. But if we have irqsoff tracing enabled,
it checks both CALLER_ADDR0 and CALLER_ADDR1. The second call
goes two stack frames up. If this is from user space, then there may
not exist a second stack.
Add a second stack when calling trace_hardirqs_on/off() otherwise
the following oops might occur:
Oops: Kernel access of bad area, sig: 11 [#1]
PREEMPT SMP NR_CPUS=2 PA Semi PWRficient
last sysfs file: /sys/block/sda/size
Modules linked in: ohci_hcd ehci_hcd usbcore
NIP: c0000000000e1c00 LR: c0000000000034d4 CTR: 000000011012c440
REGS: c00000003e2f3af0 TRAP: 0300 Not tainted (2.6.37-rc6+)
MSR: 9000000000001032 <ME,IR,DR> CR: 48044444 XER: 20000000
DAR: 00000001ffb9db50, DSISR: 0000000040000000
TASK = c00000003e1a00a0[2088] 'emacs' THREAD: c00000003e2f0000 CPU: 1
GPR00: 0000000000000001 c00000003e2f3d70 c00000000084e0d0 c0000000008816e8
GPR04: 000000001034c678 000000001032e8f9 0000000010336540 0000000040020000
GPR08: 0000000040020000 00000001ffb9db40 c00000003e2f3e30 0000000060000000
GPR12: 100000000000f032 c00000000fff0280 000000001032e8c9 0000000000000008
GPR16: 00000000105be9c0 00000000105be950 00000000105be9b0 00000000105be950
GPR20: 00000000ffb9dc50 00000000ffb9dbf0 00000000102f0000 00000000102f0000
GPR24: 00000000102e0000 00000000102f0000 0000000010336540 c0000000009ded38
GPR28: 00000000102e0000 c0000000000034d4 c0000000007ccb10 c00000003e2f3d70
NIP [c0000000000e1c00] .trace_hardirqs_off+0xb0/0x1d0
LR [c0000000000034d4] decrementer_common+0xd4/0x100
Call Trace:
[c00000003e2f3d70] [c00000003e2f3e30] 0xc00000003e2f3e30 (unreliable)
[c00000003e2f3e30] [c0000000000034d4] decrementer_common+0xd4/0x100
Instruction dump:
81690000 7f8b0000 419e0018 f84a0028 60000000 60000000 60000000 e95f0000
80030000 e92a0000 eb6301f8 2f800000 <eb890010> 41fe00dc a06d000a eb1e8050
---[ end trace 4ec7fd2be9240928 ]---
Reported-by: Joerg Sommer <joerg@alea.gnuu.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
diff --git a/arch/powerpc/include/asm/irqflags.h b/arch/powerpc/include/asm/irqflags.h
index b85d8dd..b0b06d8 100644
--- a/arch/powerpc/include/asm/irqflags.h
+++ b/arch/powerpc/include/asm/irqflags.h
@@ -12,24 +12,44 @@
#else
#ifdef CONFIG_TRACE_IRQFLAGS
+#ifdef CONFIG_IRQSOFF_TRACER
+/*
+ * Since the ftrace irqsoff latency trace checks CALLER_ADDR1,
+ * which is the stack frame here, we need to force a stack frame
+ * in case we came from user space.
+ */
+#define TRACE_WITH_FRAME_BUFFER(func) \
+ mflr r0; \
+ stdu r1, -32(r1); \
+ std r0, 16(r1); \
+ stdu r1, -32(r1); \
+ bl func; \
+ ld r1, 0(r1); \
+ ld r1, 0(r1);
+#else
+#define TRACE_WITH_FRAME_BUFFER(func) \
+ bl func;
+#endif
+
/*
* Most of the CPU's IRQ-state tracing is done from assembly code; we
* have to call a C function so call a wrapper that saves all the
* C-clobbered registers.
*/
-#define TRACE_ENABLE_INTS bl .trace_hardirqs_on
-#define TRACE_DISABLE_INTS bl .trace_hardirqs_off
-#define TRACE_AND_RESTORE_IRQ_PARTIAL(en,skip) \
- cmpdi en,0; \
- bne 95f; \
- stb en,PACASOFTIRQEN(r13); \
- bl .trace_hardirqs_off; \
- b skip; \
-95: bl .trace_hardirqs_on; \
+#define TRACE_ENABLE_INTS TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_on)
+#define TRACE_DISABLE_INTS TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_off)
+
+#define TRACE_AND_RESTORE_IRQ_PARTIAL(en,skip) \
+ cmpdi en,0; \
+ bne 95f; \
+ stb en,PACASOFTIRQEN(r13); \
+ TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_off) \
+ b skip; \
+95: TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_on) \
li en,1;
#define TRACE_AND_RESTORE_IRQ(en) \
TRACE_AND_RESTORE_IRQ_PARTIAL(en,96f); \
- stb en,PACASOFTIRQEN(r13); \
+ stb en,PACASOFTIRQEN(r13); \
96:
#else
#define TRACE_ENABLE_INTS
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH][GIT PULL] powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
2010-12-24 5:46 [PATCH][GIT PULL] powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off Steven Rostedt
@ 2010-12-24 21:11 ` Benjamin Herrenschmidt
2010-12-25 21:04 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2010-12-24 21:11 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linuxppc-dev, Andrew Morton, Joerg Sommer, LKML
On Fri, 2010-12-24 at 00:46 -0500, Steven Rostedt wrote:
> arch/powerpc/include/asm/irqflags.h | 40 ++++++++++++++++++++++++++--------
> 1 files changed, 30 insertions(+), 10 deletions(-)
> ---------------------------
> commit 5025019505da6731f8be13940bb978617599c935
> Author: Steven Rostedt <srostedt@redhat.com>
> Date: Thu Dec 23 21:07:39 2010 -0800
>
> powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
>
> When an interrupt occurs in userspace, we can call trace_hardirqs_on/off()
> With one level stack. But if we have irqsoff tracing enabled,
> it checks both CALLER_ADDR0 and CALLER_ADDR1. The second call
> goes two stack frames up. If this is from user space, then there may
> not exist a second stack.
>
> Add a second stack when calling trace_hardirqs_on/off() otherwise
> the following oops might occur:
Hrm... this is really gross :-) So we add gratuituous overhead because
the code below is dumb :-) What about making the code less stupid
instead when poking at the stack and detect it's coming from userspace
instead ?
Cheers,
Ben.
> Oops: Kernel access of bad area, sig: 11 [#1]
> PREEMPT SMP NR_CPUS=2 PA Semi PWRficient
> last sysfs file: /sys/block/sda/size
> Modules linked in: ohci_hcd ehci_hcd usbcore
> NIP: c0000000000e1c00 LR: c0000000000034d4 CTR: 000000011012c440
> REGS: c00000003e2f3af0 TRAP: 0300 Not tainted (2.6.37-rc6+)
> MSR: 9000000000001032 <ME,IR,DR> CR: 48044444 XER: 20000000
> DAR: 00000001ffb9db50, DSISR: 0000000040000000
> TASK = c00000003e1a00a0[2088] 'emacs' THREAD: c00000003e2f0000 CPU: 1
> GPR00: 0000000000000001 c00000003e2f3d70 c00000000084e0d0 c0000000008816e8
> GPR04: 000000001034c678 000000001032e8f9 0000000010336540 0000000040020000
> GPR08: 0000000040020000 00000001ffb9db40 c00000003e2f3e30 0000000060000000
> GPR12: 100000000000f032 c00000000fff0280 000000001032e8c9 0000000000000008
> GPR16: 00000000105be9c0 00000000105be950 00000000105be9b0 00000000105be950
> GPR20: 00000000ffb9dc50 00000000ffb9dbf0 00000000102f0000 00000000102f0000
> GPR24: 00000000102e0000 00000000102f0000 0000000010336540 c0000000009ded38
> GPR28: 00000000102e0000 c0000000000034d4 c0000000007ccb10 c00000003e2f3d70
> NIP [c0000000000e1c00] .trace_hardirqs_off+0xb0/0x1d0
> LR [c0000000000034d4] decrementer_common+0xd4/0x100
> Call Trace:
> [c00000003e2f3d70] [c00000003e2f3e30] 0xc00000003e2f3e30 (unreliable)
> [c00000003e2f3e30] [c0000000000034d4] decrementer_common+0xd4/0x100
> Instruction dump:
> 81690000 7f8b0000 419e0018 f84a0028 60000000 60000000 60000000 e95f0000
> 80030000 e92a0000 eb6301f8 2f800000 <eb890010> 41fe00dc a06d000a eb1e8050
> ---[ end trace 4ec7fd2be9240928 ]---
>
> Reported-by: Joerg Sommer <joerg@alea.gnuu.de>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
>
> diff --git a/arch/powerpc/include/asm/irqflags.h b/arch/powerpc/include/asm/irqflags.h
> index b85d8dd..b0b06d8 100644
> --- a/arch/powerpc/include/asm/irqflags.h
> +++ b/arch/powerpc/include/asm/irqflags.h
> @@ -12,24 +12,44 @@
>
> #else
> #ifdef CONFIG_TRACE_IRQFLAGS
> +#ifdef CONFIG_IRQSOFF_TRACER
> +/*
> + * Since the ftrace irqsoff latency trace checks CALLER_ADDR1,
> + * which is the stack frame here, we need to force a stack frame
> + * in case we came from user space.
> + */
> +#define TRACE_WITH_FRAME_BUFFER(func) \
> + mflr r0; \
> + stdu r1, -32(r1); \
> + std r0, 16(r1); \
> + stdu r1, -32(r1); \
> + bl func; \
> + ld r1, 0(r1); \
> + ld r1, 0(r1);
> +#else
> +#define TRACE_WITH_FRAME_BUFFER(func) \
> + bl func;
> +#endif
> +
> /*
> * Most of the CPU's IRQ-state tracing is done from assembly code; we
> * have to call a C function so call a wrapper that saves all the
> * C-clobbered registers.
> */
> -#define TRACE_ENABLE_INTS bl .trace_hardirqs_on
> -#define TRACE_DISABLE_INTS bl .trace_hardirqs_off
> -#define TRACE_AND_RESTORE_IRQ_PARTIAL(en,skip) \
> - cmpdi en,0; \
> - bne 95f; \
> - stb en,PACASOFTIRQEN(r13); \
> - bl .trace_hardirqs_off; \
> - b skip; \
> -95: bl .trace_hardirqs_on; \
> +#define TRACE_ENABLE_INTS TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_on)
> +#define TRACE_DISABLE_INTS TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_off)
> +
> +#define TRACE_AND_RESTORE_IRQ_PARTIAL(en,skip) \
> + cmpdi en,0; \
> + bne 95f; \
> + stb en,PACASOFTIRQEN(r13); \
> + TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_off) \
> + b skip; \
> +95: TRACE_WITH_FRAME_BUFFER(.trace_hardirqs_on) \
> li en,1;
> #define TRACE_AND_RESTORE_IRQ(en) \
> TRACE_AND_RESTORE_IRQ_PARTIAL(en,96f); \
> - stb en,PACASOFTIRQEN(r13); \
> + stb en,PACASOFTIRQEN(r13); \
> 96:
> #else
> #define TRACE_ENABLE_INTS
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][GIT PULL] powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
2010-12-24 21:11 ` Benjamin Herrenschmidt
@ 2010-12-25 21:04 ` Steven Rostedt
2010-12-27 21:38 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2010-12-25 21:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Andrew Morton, Joerg Sommer, LKML
On Sat, 2010-12-25 at 08:11 +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2010-12-24 at 00:46 -0500, Steven Rostedt wrote:
>
> > arch/powerpc/include/asm/irqflags.h | 40 ++++++++++++++++++++++++++--------
> > 1 files changed, 30 insertions(+), 10 deletions(-)
> > ---------------------------
> > commit 5025019505da6731f8be13940bb978617599c935
> > Author: Steven Rostedt <srostedt@redhat.com>
> > Date: Thu Dec 23 21:07:39 2010 -0800
> >
> > powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
> >
> > When an interrupt occurs in userspace, we can call trace_hardirqs_on/off()
> > With one level stack. But if we have irqsoff tracing enabled,
> > it checks both CALLER_ADDR0 and CALLER_ADDR1. The second call
> > goes two stack frames up. If this is from user space, then there may
> > not exist a second stack.
> >
> > Add a second stack when calling trace_hardirqs_on/off() otherwise
> > the following oops might occur:
>
> Hrm... this is really gross :-) So we add gratuituous overhead because
> the code below is dumb :-) What about making the code less stupid
> instead when poking at the stack and detect it's coming from userspace
> instead ?
Note, when CONFIG_IRQSOFF_TRACE is set, there's already a bit of
overhead :-)
Anyway, I'll have to take a look at how the frame pointer is set up. Or
we could also set up all stacks coming into the kernel to have a "dummy"
frame pointer that wont hurt anything if we index into it.
Anyway, I'm off till the new year, so I'll worry about it then ;-)
-- Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][GIT PULL] powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off
2010-12-25 21:04 ` Steven Rostedt
@ 2010-12-27 21:38 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2010-12-27 21:38 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linuxppc-dev, Andrew Morton, Joerg Sommer, LKML
On Sat, 2010-12-25 at 16:04 -0500, Steven Rostedt wrote:
>
> Note, when CONFIG_IRQSOFF_TRACE is set, there's already a bit of
> overhead :-)
>
> Anyway, I'll have to take a look at how the frame pointer is set up.
> Or
> we could also set up all stacks coming into the kernel to have a
> "dummy"
> frame pointer that wont hurt anything if we index into it.
>
> Anyway, I'm off till the new year, so I'll worry about it then ;-)
Right, might be simpler to just make them loop back onto themselves or
something like that. I can have a look too if I get a chance.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-12-27 21:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-24 5:46 [PATCH][GIT PULL] powerpc64/tracing: Add frame buffer to calls of trace_hardirqs_on/off Steven Rostedt
2010-12-24 21:11 ` Benjamin Herrenschmidt
2010-12-25 21:04 ` Steven Rostedt
2010-12-27 21:38 ` Benjamin Herrenschmidt
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).