Re: [PATCH -v2 6/8] kexec jump: fix for lockdep

2008-08-11 Thread Peter Zijlstra 
On Mon, 2008-08-11 at 08:59 +0800, Huang Ying wrote:
 On Fri, 2008-08-08 at 12:13 +0200, Peter Zijlstra wrote:
  On Fri, 2008-08-08 at 14:52 +0800, Huang Ying wrote:
   Replace local_irq_disable() with raw_local_irq_disable() to prevent
   lockdep complain.
  Uhhm, please provide more information - just using raw_* to silence
  lockdep is generally the wrong thing to do.
 
 In traditional kexec, the new kernel will replace current one, so the
 irq is simply disabled. But now jumping back from kexeced kernel is
 supported, so the irq should be enabled again.
 
 The code sequence of irq during kexec jump is as follow:
 
 local_irq_disable(); /* in kernel_kexec() */
 local_irq_disable(); /* in machine_kexec() */
 local_irq_enable(); /* in kernel_kexec() */
 
 The disable and enable is not match. Maybe another method is to use
 local_irq_save(), local_irq_restore() pair in machine_kexec(), so the
 disable and enable is matched.

And its the machine kernel's lockdep instance that goes complain?

whichever annotation gets used - and I think I can agree that raw_*
might be approriate there, this should be accompanied with a rather
elaborate changelog and preferably a comment in the code too. Without
such we'll be wondering in the years to come WTH happens here.



___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH -v2 6/8] kexec jump: fix for lockdep

2008-08-11 Thread Huang Ying 
On Mon, 2008-08-11 at 08:09 +0200, Peter Zijlstra wrote:
 On Mon, 2008-08-11 at 08:59 +0800, Huang Ying wrote:
  On Fri, 2008-08-08 at 12:13 +0200, Peter Zijlstra wrote:
   On Fri, 2008-08-08 at 14:52 +0800, Huang Ying wrote:
Replace local_irq_disable() with raw_local_irq_disable() to prevent
lockdep complain.
   Uhhm, please provide more information - just using raw_* to silence
   lockdep is generally the wrong thing to do.
  
  In traditional kexec, the new kernel will replace current one, so the
  irq is simply disabled. But now jumping back from kexeced kernel is
  supported, so the irq should be enabled again.
  
  The code sequence of irq during kexec jump is as follow:
  
  local_irq_disable(); /* in kernel_kexec() */
  local_irq_disable(); /* in machine_kexec() */
  local_irq_enable(); /* in kernel_kexec() */
  
  The disable and enable is not match. Maybe another method is to use
  local_irq_save(), local_irq_restore() pair in machine_kexec(), so the
  disable and enable is matched.
 
 And its the machine kernel's lockdep instance that goes complain?
 
 whichever annotation gets used - and I think I can agree that raw_*
 might be approriate there, this should be accompanied with a rather
 elaborate changelog and preferably a comment in the code too. Without
 such we'll be wondering in the years to come WTH happens here.

Sorry, I find there is no complain from lockdep. Un-paired irq
disable/enable has no problem with lockdep, just increase something such
as redundant_hardirqs_off. Please ignore this thread.

Best Regards,
Huang Ying



___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH -v2 6/8] kexec jump: fix for lockdep

2008-08-10 Thread Huang Ying 
On Fri, 2008-08-08 at 12:13 +0200, Peter Zijlstra wrote:
 On Fri, 2008-08-08 at 14:52 +0800, Huang Ying wrote:
  Replace local_irq_disable() with raw_local_irq_disable() to prevent
  lockdep complain.
 Uhhm, please provide more information - just using raw_* to silence
 lockdep is generally the wrong thing to do.

In traditional kexec, the new kernel will replace current one, so the
irq is simply disabled. But now jumping back from kexeced kernel is
supported, so the irq should be enabled again.

The code sequence of irq during kexec jump is as follow:

local_irq_disable(); /* in kernel_kexec() */
local_irq_disable(); /* in machine_kexec() */
local_irq_enable(); /* in kernel_kexec() */

The disable and enable is not match. Maybe another method is to use
local_irq_save(), local_irq_restore() pair in machine_kexec(), so the
disable and enable is matched.

Best Regrards,
Huang Ying



___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH -v2 6/8] kexec jump: fix for lockdep

2008-08-08 Thread Huang Ying 
Replace local_irq_disable() with raw_local_irq_disable() to prevent
lockdep complain.

Signed-off-by: Huang Ying [EMAIL PROTECTED]

---
 arch/x86/kernel/machine_kexec_32.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/x86/kernel/machine_kexec_32.c
+++ b/arch/x86/kernel/machine_kexec_32.c
@@ -123,7 +123,7 @@ void machine_kexec(struct kimage *image)
tracer_disable();
 
/* Interrupts aren't acceptable while we reboot */
-   local_irq_disable();
+   raw_local_irq_disable();
 
if (image-preserve_context) {
 #ifdef CONFIG_X86_IO_APIC



___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec