Hi Michael,
On Thu, 31 Jul 2008 21:40:56 +1000 Michael Ellerman <michael@xxxxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
