Re: [PATCH] powerpc - Initialize the irq radix tree earlier

On Thu, 31 Jul 2008 23:01:39 +1000 Michael Ellerman <michael@ellerman.id.au=

wrote:

On Thu, 2008-07-31 at 22:58 +1000, Michael Ellerman wrote:

quoted

On Thu, 2008-07-31 at 14:00 +0200, Sebastien Dugue wrote:

quoted

On Thu, 31 Jul 2008 21:40:56 +1000 Michael Ellerman <michael@ellerman=

.id.au> wrote:

quoted

quoted

quoted

=20
This boot ordering stuff is pretty hairy, so I might have missed
something, but this is how the code is ordered AFAICT:
=EF=BB=BF
start_kernel()
	init_IRQ()
	...
	local_irq_enable()
	...
	rest_init()
		kernel_thread()
			kernel_init()
				smp_prepare_cpus()
					smp_xics_probe()	(via smp_ops->probe())
=20
=20
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?

quoted

quoted

=20
  It's hairy, I agree, but as you've mentioned no one has done a requ=

est_irq()

quoted

quoted

at that point. The first one to do it is smp_xics_probe() for the IPI.

=20
Hmm, I don't think that's strong enough. I can trivially cause irqs to
fire during a kexec reboot just by mashing the keyboard.
=20
And during a kdump boot all sorts of stuff could be firing. Even during
a clean boot, from firmware, I don't think we can guarantee that
nothing's going to fire.
=20
.. after a bit of testing ..
=20
It seems it actually works (sort of).=20
=20
xics_remap_irq() calls irq_radix_revmap_lookup(), which calls:
=20
ptr =3D radix_tree_lookup(&host->revmap_data.tree, hwirq);
=20
And because =EF=BB=BFhost->revmap_data.tree was zalloc'ed we trip on th=

e first

quoted

check here:

=20
@#$% ctrl-enter =3D=3D send!
=20
Continuing ...
=20
void *radix_tree_lookup(struct radix_tree_root *root, unsigned long index)
{
        unsigned int height, shift;
        struct radix_tree_node *node, **slot;
=20
        node =3D rcu_dereference(root->rnode);
        if (node =3D=3D NULL)
                return NULL;
=20
Which means =EF=BB=BFirq_radix_revmap_lookup() will return NO_IRQ, which =

is cool.

  Which is what I intended so that as long as no IRQ is registered we
return NO_IRQ.

=20
=20
So I think it can fly, as long as we're happy that we can't reverse map
anything until smp_xics_probe() - and I think that's true, as any irq we
take will be invalid.

  That's true as no IRQs are registered before smp_xics_probe() and for any
interrupt we might get before that, irq_radix_revmap_lookup() will return
NO_IRQ.

  Thanks,

  Sebastien.