Re: [RESEND PATCH] ARM: fix 'unannotated irqs-on' lockdep warning

[Russell King - ARM Linux]

On Mon, May 24, 2010 at 10:20:33PM +0800, Ming Lei wrote:
> On Mon, 24 May 2010 11:14:16 +0100
> Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> wrote:
> 
> > Right, I see what the problem is now - it's all to do with threads
> > created with kernel_thread() confusing lockdep.
> > 
> > I'm of the opinion that all your changes in entry*.S are the wrong
> > way to fix this - not only does it add additional overhead where
> > none is really necessary, it adds additional complexity.
> > 
> > So, here's a patch to solve the warning you quoted.
> 
> No, your patch does not fix the issue, I still can see the lockdep
> warning after applying it against Linux 2.6.34-next-20100524.

No, my patch fixes the issue you raised in the dump.  This is a new
lockdep issue which is unrelated to the first dump.

What's wrong is:

commit 0d928b0b616d1c5c5fe76019a87cba171ca91633
Author: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
Date:   Thu Aug 13 20:38:17 2009 +0200

    Complete irq tracing support for ARM

    Before this patch enabling and disabling irqs in assembler code and by
    the hardware wasn't tracked completly.

    I had to transpose two instructions in arch/arm/lib/bitops.h because
    restore_irqs doesn't preserve the flags with CONFIG_TRACE_IRQFLAGS=y

    Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
    Cc: Russell King <linux@xxxxxxxxxxxxxxxx>
    Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
    Cc: Ingo Molnar <mingo@xxxxxxxxxx>

    Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>

which needs to be redone with a little more understanding, like the
following patch.  Note: you will need the previous patch as well,
which is separate because it addresses a *different* issue.

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 7ee48e7..3fd7861 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -162,8 +162,6 @@ ENDPROC(__und_invalid)
 	@  r4 - orig_r0 (see pt_regs definition in ptrace.h)
 	@
 	stmia	r5, {r0 - r4}
-
-	asm_trace_hardirqs_off
 	.endm
 
 	.align	5
@@ -204,7 +202,7 @@ __dabt_svc:
 	@
 	@ IRQs off again before pulling preserved data off the stack
 	@
-	disable_irq
+	disable_irq_notrace
 
 	@
 	@ restore SPSR and restart the instruction
@@ -218,6 +216,9 @@ ENDPROC(__dabt_svc)
 __irq_svc:
 	svc_entry
 
+#ifdef CONFIG_TRACE_IRQFLAGS
+	bl	trace_hardirqs_off
+#endif
 #ifdef CONFIG_PREEMPT
 	get_thread_info tsk
 	ldr	r8, [tsk, #TI_PREEMPT]		@ get preempt count
@@ -291,7 +292,7 @@ __und_svc:
 	@
 	@ IRQs off again before pulling preserved data off the stack
 	@
-1:	disable_irq
+1:	disable_irq_notrace
 
 	@
 	@ restore SPSR and restart the instruction
@@ -327,7 +328,7 @@ __pabt_svc:
 	@
 	@ IRQs off again before pulling preserved data off the stack
 	@
-	disable_irq
+	disable_irq_notrace
 
 	@
 	@ restore SPSR and restart the instruction
@@ -393,8 +394,6 @@ ENDPROC(__pabt_svc)
 	@ Clear FP to mark the first stack frame
 	@
 	zero_fp
-
-	asm_trace_hardirqs_off
 	.endm
 
 	.macro	kuser_cmpxchg_check
@@ -465,9 +464,6 @@ __irq_usr:
  THUMB(	movne	r0, #0		)
  THUMB(	strne	r0, [r0]	)
 #endif
-#ifdef CONFIG_TRACE_IRQFLAGS
-	bl	trace_hardirqs_on
-#endif
 
 	mov	why, #0
 	b	ret_to_user
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/