On Tue, 2008-09-09 at 21:56 +0200, [email protected] wrote:
> From: Alexander van Heukelum <[email protected]>
>
> At this point interrupts are off, so let's inform the tracing
> code of that fact before calling into C.
Sorry but this is an obvious dud, lockdep (and thus the irq state
tracer) aren't nmi safe.
Ingo, please revert, as people are already seeing lockdep warnings due
to this.
> Signed-off-by: Alexander van Heukelum <[email protected]>
> ---
> arch/x86/kernel/entry_32.S | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index a278505..2abdc9a 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -849,6 +849,7 @@ nmi_stack_correct:
> pushl %eax
> CFI_ADJUST_CFA_OFFSET 4
> SAVE_ALL
> + TRACE_IRQS_OFF
> xorl %edx,%edx # zero error code
> movl %esp,%eax # pt_regs pointer
> call do_nmi
> @@ -889,6 +890,7 @@ nmi_espfix_stack:
> pushl %eax
> CFI_ADJUST_CFA_OFFSET 4
> SAVE_ALL
> + TRACE_IRQS_OFF
> FIXUP_ESPFIX_STACK # %eax == %esp
> xorl %edx,%edx # zero error code
> call do_nmi
* Peter Zijlstra <[email protected]> wrote:
> On Tue, 2008-09-09 at 21:56 +0200, [email protected] wrote:
> > From: Alexander van Heukelum <[email protected]>
> >
> > At this point interrupts are off, so let's inform the tracing
> > code of that fact before calling into C.
>
> Sorry but this is an obvious dud, lockdep (and thus the irq state
> tracer) aren't nmi safe.
>
> Ingo, please revert, as people are already seeing lockdep warnings due
> to this.
done - reverted it in tip/x86/urgent, see the commit below. Is that all
that we need, wasnt there a 64-bit side done too?
Ingo
--------------->
>From e8cea892dff8e3ebed42954c46730309b617196f Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Mon, 12 Jan 2009 19:36:59 +0100
Subject: [PATCH] Revert "i386: add TRACE_IRQS_OFF for the nmi"
This reverts commit e0c7317557c8fc8eacf611e30c2a80f4e24e47a3.
This patch was wrong, as lockdep (and thus the irq state tracer)
aren't nmi safe. People are already seeing lockdep warnings due
to this.
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/entry_32.S | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index d6f0490..4646902 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1203,7 +1203,6 @@ nmi_stack_correct:
pushl %eax
CFI_ADJUST_CFA_OFFSET 4
SAVE_ALL
- TRACE_IRQS_OFF
xorl %edx,%edx # zero error code
movl %esp,%eax # pt_regs pointer
call do_nmi
@@ -1244,7 +1243,6 @@ nmi_espfix_stack:
pushl %eax
CFI_ADJUST_CFA_OFFSET 4
SAVE_ALL
- TRACE_IRQS_OFF
FIXUP_ESPFIX_STACK # %eax == %esp
xorl %edx,%edx # zero error code
call do_nmi
On Mon, 2009-01-12 at 19:39 +0100, Ingo Molnar wrote:
> * Peter Zijlstra <[email protected]> wrote:
>
> > On Tue, 2008-09-09 at 21:56 +0200, [email protected] wrote:
> > > From: Alexander van Heukelum <[email protected]>
> > >
> > > At this point interrupts are off, so let's inform the tracing
> > > code of that fact before calling into C.
> >
> > Sorry but this is an obvious dud, lockdep (and thus the irq state
> > tracer) aren't nmi safe.
> >
> > Ingo, please revert, as people are already seeing lockdep warnings due
> > to this.
>
> done - reverted it in tip/x86/urgent, see the commit below. Is that all
> that we need, wasnt there a 64-bit side done too?
I had a _very_ quick peek but couldn't fine one, Alexander, does your
memory go back that far? :-)
On Mon, 12 Jan 2009 19:43:35 +0100, "Peter Zijlstra"
<[email protected]> said:
> On Mon, 2009-01-12 at 19:39 +0100, Ingo Molnar wrote:
> > * Peter Zijlstra <[email protected]> wrote:
> >
> > > On Tue, 2008-09-09 at 21:56 +0200, [email protected] wrote:
> > > > From: Alexander van Heukelum <[email protected]>
> > > >
> > > > At this point interrupts are off, so let's inform the tracing
> > > > code of that fact before calling into C.
> > >
> > > Sorry but this is an obvious dud, lockdep (and thus the irq state
> > > tracer) aren't nmi safe.
> > >
> > > Ingo, please revert, as people are already seeing lockdep warnings due
> > > to this.
> >
> > done - reverted it in tip/x86/urgent, see the commit below. Is that all
> > that we need, wasnt there a 64-bit side done too?
>
> I had a _very_ quick peek but couldn't fine one, Alexander, does your
> memory go back that far? :-)
Git helped a bit, I must admit. The 64-bit version does not include
TRACE_IRQS_ON/TRACE_IRQS_OFF in the nmi handler. The comment above
"ENTRY(paranoid_exit)" in entry_64.S should be updated, though. That
code is not used for NMIs.
The revert is obviously the correct thing to do, but a comment would
not hurt here either.
Greetings,
Alexander
--
Alexander van Heukelum
[email protected]
--
http://www.fastmail.fm - Access your email from home and the web