On Thu, 31 Jul 2008 14:00:02 +0200 Sebastien Dugue <sebastien.dugue@xxxxxxxx> wrote:
>
> 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.
Ah yes, I wanted to do it from xics_init_host() but backed off
because at that point the mm is not up. But it does not make a difference
as the first request_irq() happens after the mm is up. A bit shaky I
concede.
> 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.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@xxxxxxxxxx
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
--
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
