lkml.org 
[lkml]   [2010]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RESEND PATCH] ARM: fix 'unannotated irqs-on' lockdep warning
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);
}

\
 
 \ /
  Last update: 2010-05-24 12:17    [W:0.040 / U:12.080 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site