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

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2903 bytes --]

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

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

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


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?


cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]