From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755992AbYGaMBP (ORCPT ); Thu, 31 Jul 2008 08:01:15 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751383AbYGaMA6 (ORCPT ); Thu, 31 Jul 2008 08:00:58 -0400 Received: from ecfrec.frec.bull.fr ([129.183.4.8]:56056 "EHLO ecfrec.frec.bull.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751610AbYGaMA5 convert rfc822-to-8bit (ORCPT ); Thu, 31 Jul 2008 08:00:57 -0400 Date: Thu, 31 Jul 2008 14:00:02 +0200 From: Sebastien Dugue To: michael@ellerman.id.au Cc: linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, benh@kernel.crashing.org, paulus@samba.org, jean-pierre.dion@bull.net, gilles.carry@ext.bull.net, tinytim@us.ibm.com, tglx@linutronix.de, rostedt@goodmis.org Subject: Re: [PATCH] powerpc - Initialize the irq radix tree earlier Message-ID: <20080731140002.31bbe4a0@bull.net> In-Reply-To: <1217504456.9817.22.camel@localhost> References: <1217497241-10685-1-git-send-email-sebastien.dugue@bull.net> <1217497241-10685-2-git-send-email-sebastien.dugue@bull.net> <1217504456.9817.22.camel@localhost> X-Mailer: Claws Mail 3.2.0 (GTK+ 2.12.2; i486-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Michael, On Thu, 31 Jul 2008 21:40:56 +1000 Michael Ellerman wrote: > On Thu, 2008-07-31 at 11:40 +0200, Sebastien Dugue wrote: > > The radix tree used for fast irq reverse mapping by the XICS is initialized > > late in the boot process, after the first interrupt (IPI) gets registered > > and after the first IPI is received. > > > > This patch moves the initialization of the XICS radix tree earlier into > > the boot process in smp_xics_probe() (the mm is already up but no interrupts > > have been registered at that point) to avoid having to insert a mapping into > > the tree in interrupt context. This will help in simplifying the locking > > constraints and move to a lockless radix tree in subsequent patches. > > > > As a nice side effect, there is no need any longer to check for > > (host->revmap_data.tree.gfp_mask != 0) to know if the tree have been > > initialized. > > Hi Sebastien, > > This is a nice cleanup, I think :) Thanks. > > > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > > index 6ac8612..0a1445c 100644 > > --- a/arch/powerpc/kernel/irq.c > > +++ b/arch/powerpc/kernel/irq.c > > @@ -893,28 +890,28 @@ unsigned int irq_find_mapping(struct irq_host *host, > > } > > EXPORT_SYMBOL_GPL(irq_find_mapping); > > > > +void __init irq_radix_revmap_init(void) > > +{ > > + struct irq_host *h; > > + > > + list_for_each_entry(h, &irq_hosts, link) { > > + if (h->revmap_type == IRQ_HOST_MAP_TREE) > > + INIT_RADIX_TREE(&h->revmap_data.tree, GFP_ATOMIC); > > + } > > +} > > Note irq_radix_revmap_init() loops over all irq_hosts ... Yep, but there's only one host (xics) > > > diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c > > index 9d8f8c8..b143fe7 100644 > > --- a/arch/powerpc/platforms/pseries/smp.c > > +++ b/arch/powerpc/platforms/pseries/smp.c > > @@ -130,6 +130,7 @@ static void smp_xics_message_pass(int target, int msg) > > > > static int __init smp_xics_probe(void) > > { > > + irq_radix_revmap_init(); > > xics_request_IPIs(); > > But now it's only called from the xics setup code. > > Which seems a bit ugly. In practice it doesn't matter because at the > moment xics is the only user of the radix revmap. But if we're going to > switch to this sort of initialisation I think xics should only be > init'ing the revmap for itself. You're right, that's what I intended to do from the beginning but stumbled upon ... hmm, can't remember what, that made me change my mind. But I agree, I'm not particularly proud of that. Will look again into that. > > > This boot ordering stuff is pretty hairy, so I might have missed > something, but this is how the code is ordered AFAICT: >  > start_kernel() > init_IRQ() > ... > local_irq_enable() > ... > rest_init() > kernel_thread() > kernel_init() > smp_prepare_cpus() > smp_xics_probe() (via smp_ops->probe()) > > > What's stopping us from taking an irq between local_irq_enable() and > smp_xics_probe() ? Is it just that no one's request_irq()'ed them yet? It's hairy, I agree, but as you've mentioned no one has done a request_irq() at that point. The first one to do it is smp_xics_probe() for the IPI. Thanks for your comments. Sebastien.