start_kernel(): bug: interrupts were enabled early
in [Kernel]
|
From: Benjamin Herrenschmidt on 31 Mar 2010 18:40 On Wed, 2010-03-31 at 13:47 -0700, Yinghai Lu wrote: > > perhaps the second one isn't needed? Perhaps no architecture > requires > > that local interrupts be disabled across the above initialisations? > > spin_unlock_irq from arm is different from other archs? No, it's not, it will enable IRQs and thats illegal to do so early during boot. We've been over that one again and again, the problem is that people want to keep using that instead of irqsave/restore because it's a nano-optimisation on x86... oh well... Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 31 Mar 2010 18:50 On 03/31/2010 03:31 PM, Benjamin Herrenschmidt wrote: > On Wed, 2010-03-31 at 13:47 -0700, Yinghai Lu wrote: >>> perhaps the second one isn't needed? Perhaps no architecture >> requires >>> that local interrupts be disabled across the above initialisations? >> >> spin_unlock_irq from arm is different from other archs? > > No, it's not, it will enable IRQs and thats illegal to do so early > during boot. We've been over that one again and again, the problem is > that people want to keep using that instead of irqsave/restore because > it's a nano-optimisation on x86... oh well... > Well, guess what... the particular user in this case *isn't used at all on x86* so it is a non-issue here. So I take it we have your particular vote to use irqsave/irqrestore in lib/rwsem-spinlock.c? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 31 Mar 2010 19:00 On Thu, 01 Apr 2010 09:37:51 +1100 Benjamin Herrenschmidt <benh(a)kernel.crashing.org> wrote: > On Wed, 2010-03-31 at 14:57 -0700, H. Peter Anvin wrote: > > > > The question still remains what the incremental cost is of doing > > irqsave/irqrestore. > > The only other option is to have local_irq_enable() check a global > (system_state ?) before enabling. Almost as gross ... > Add an irq-disable-depth counter to the task_struct, fix all the bugs which that exposes.. But these things are all utterly gross. The bottom line is that radix_tree_init() is manifestly unsuited to being called with local interrupts disabled. 773e3eb7b81e5ba13b5155dfb3bb75b8ce37f8f9 was just a wrong patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: David Howells on 31 Mar 2010 19:10 Russell King <rmk(a)arm.linux.org.uk> wrote: > We use the standard generic kernel implementation. Is x86 different? ;) The optimised fast paths used on x86 rwsems don't disable interrupts. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 31 Mar 2010 21:20
On Wed, 2010-03-31 at 15:49 -0700, Andrew Morton wrote: > > But these things are all utterly gross. The bottom line is that > radix_tree_init() is manifestly unsuited to being called with local > interrupts disabled. 773e3eb7b81e5ba13b5155dfb3bb75b8ce37f8f9 was > just a wrong patch. Except that powerpc (and now it seems x86) both want to use radix trees for interrupt handling... At least on powerpc, we trick and use a linear search until the radix trees are initialized, which we do later during boot, but that somewhat sucks. I believe sherry picking things like not calling radix_tree_init() is going to fix one case today, until we have another one, and another one, and etc... I suspect we're better off fixing the root of the problem in down/up. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |