Hi Michael,

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

wrote:

On Thu, 2008-07-31 at 11:40 +0200, Sebastien Dugue wrote:

quoted

The radix tree used for fast irq reverse mapping by the XICS is initial=

ized

quoted

late in the boot process, after the first interrupt (IPI) gets register=

ed

quoted

and after the first IPI is received.
=20
  This patch moves the initialization of the XICS radix tree earlier in=

to

quoted

the boot process in smp_xics_probe() (the mm is already up but no inter=

rupts

quoted

have been registered at that point) to avoid having to insert a mapping=

 into

quoted

the tree in interrupt context. This will help in simplifying the locking
constraints and move to a lockless radix tree in subsequent patches.
=20
  As a nice side effect, there is no need any longer to check for
(host->revmap_data.tree.gfp_mask !=3D 0) to know if the tree have been
initialized.

=20
Hi Sebastien,
=20
This is a nice cleanup, I think :)

  Thanks.

=20

quoted

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 *ho=

st,

quoted

 }
 EXPORT_SYMBOL_GPL(irq_find_mapping);
=20
+void __init irq_radix_revmap_init(void)
+{
+ 	struct irq_host *h;
+
+	list_for_each_entry(h, &irq_hosts, link) {
+		if (h->revmap_type =3D=3D IRQ_HOST_MAP_TREE)
+			INIT_RADIX_TREE(&h->revmap_data.tree, GFP_ATOMIC);
+	}
+}

=20
Note irq_radix_revmap_init() loops over all irq_hosts ...

  Yep, but there's only one host (xics)

=20

quoted

diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platfo=

rms/pseries/smp.c

quoted

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 m=

sg)

quoted

=20
 static int __init smp_xics_probe(void)
 {
+	irq_radix_revmap_init();
 	xics_request_IPIs();

=20
But now it's only called from the xics setup code.
=20
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.

=20
=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?

  It's hairy, I agree, but as you've mentioned no one has done a request_ir=
q()
at that point. The first one to do it is smp_xics_probe() for the IPI.

  Thanks for your comments.

  Sebastien.