On Thu, 31 Jul 2008 14:00:02 +0200 Sebastien Dugue [off-list ref] wrote:

  Hi Michael,

On Thu, 31 Jul 2008 21:40:56 +1000 Michael Ellerman [off-list ref] wrote:

quoted

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 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.

quoted

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 *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)

quoted

quoted

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.

quoted

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@ozlabs.org
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html