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

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Mon, May 24, 2010 at 08:19:21AM +0100, Russell King - ARM Linux wrote:
> On Mon, May 24, 2010 at 11:23:55AM +0800, Ming Lei wrote:
> > On Sun, 23 May 2010 20:47:46 +0100
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > > Moreover, I put to you that it's utterly pointless - and a waste of
> > > CPU time - telling lockdep about the IRQ masking when an exception
> > 
> > Yes, the patch still tries to remove the pointless trace of IRQ masking,
> > such as: replace disable_irq with disable_irq_notrace.
> > 
> > > occurs, and it's also pointless telling lockdep about the IRQ
> > > unmasking when we resume userspace.
> > 
> > Even it is pointless, but if lockdep doesn't see the IRQ unmasking, the 
> > warning "unannotated irqs-on" will be triggered and lockdep doe not work
> > any longer, so we have to remove the warning to make lockdep workable on
> > ARM, could you agree on it?  It is the main purpose of the patch.
> 
> I'm sorry, I think we have a communication issue; you're not understanding
> the points that I'm making.  I feel I'm wasting my time trying to explain
> it.
> 
> I'm not merging your patch as-is because I believe it to be wrong.

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.

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index acf5e6f..a5f8fd0 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -351,17 +351,25 @@ EXPORT_SYMBOL(dump_fpu);
 
 /*
  * Shuffle the argument into the correct register before calling the
- * thread function.  r1 is the thread argument, r2 is the pointer to
- * the thread function, and r3 points to the exit function.
+ * thread function.  r4 is the thread argument, r5 is the pointer to
+ * the thread function, and r6 points to the exit function.
  */
 extern void kernel_thread_helper(void);
 asm(	".pushsection .text\n"
 "	.align\n"
 "	.type	kernel_thread_helper, #function\n"
 "kernel_thread_helper:\n"
-"	mov	r0, r1\n"
-"	mov	lr, r3\n"
-"	mov	pc, r2\n"
+#ifdef CONFIG_TRACE_IRQFLAGS
+"	bl	trace_hardirqs_on\n"
+#endif
+#if __LINUX_ARM_ARCH__ >= 6
+"	cpsie	i\n"
+#else
+"	msr	cpsr_c, r7\n"
+#endif
+"	mov	r0, r4\n"
+"	mov	lr, r6\n"
+"	mov	pc, r5\n"
 "	.size	kernel_thread_helper, . - kernel_thread_helper\n"
 "	.popsection");
 
@@ -391,11 +399,12 @@ pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags)
 
 	memset(&regs, 0, sizeof(regs));
 
-	regs.ARM_r1 = (unsigned long)arg;
-	regs.ARM_r2 = (unsigned long)fn;
-	regs.ARM_r3 = (unsigned long)kernel_thread_exit;
+	regs.ARM_r4 = (unsigned long)arg;
+	regs.ARM_r5 = (unsigned long)fn;
+	regs.ARM_r6 = (unsigned long)kernel_thread_exit;
+	regs.ARM_r7 = SVC_MODE | PSR_ISETSTATE;
 	regs.ARM_pc = (unsigned long)kernel_thread_helper;
-	regs.ARM_cpsr = SVC_MODE | PSR_ENDSTATE | PSR_ISETSTATE;
+	regs.ARM_cpsr = SVC_MODE | PSR_ENDSTATE | PSR_ISETSTATE | PSR_I_BIT;
 
 	return do_fork(flags|CLONE_VM|CLONE_UNTRACED, 0, &regs, 0, NULL, NULL);
 }