[PATCH] ia64: prevent irq migration race in __cpu_disable path

[PATCH] ia64: prevent irq migration race in __cpu_disable path

[Alex Chiang]

Commit e7b14036 (prevent ia64 from invoking irq handlers on
offline CPUs) introduced a bug, where we call fixup_irqs before
removing the CPU from the cpu_online_map.

This is wrong because fixup_irqs calls migrate_irqs, and in
migrate_irqs, we use the cpu_online_map to:

	1. look for interrupts on current CPU
	2. if we find one, move it to the first available CPU in
	the cpu_online_map

This means we can potentially migrate an interrupt off ourself
back to... ourself. Uh oh.

We hit a NULL deref later which causes us to oops (output trimmed):

Unable to handle kernel NULL pointer dereference (address 0000000000000040)
ip is at profile_tick+0xd0/0x1c0

Call Trace:
 [<a00000010003c700>] die+0x1a0/0x2e0
                                sp=e0000009c922fbd0 bsp=e0000009c9221438
 [<a0000001006e92f0>] ia64_do_page_fault+0x950/0xa80
                                sp=e0000009c922fbd0 bsp=e0000009c92213d8
 [<a00000010000c7a0>] ia64_native_leave_kernel+0x0/0x270
                                sp=e0000009c922fc60 bsp=e0000009c92213d8
 [<a0000001000ecdb0>] profile_tick+0xd0/0x1c0
                                sp=e0000009c922fe30 bsp=e0000009c9221398
 [<a00000010003bb90>] timer_interrupt+0x170/0x3e0
                                sp=e0000009c922fe30 bsp=e0000009c9221330
 [<a00000010013a800>] handle_IRQ_event+0x80/0x120
                                sp=e0000009c922fe30 bsp=e0000009c92212f8
 [<a00000010013aa00>] __do_IRQ+0x160/0x4a0
                                sp=e0000009c922fe30 bsp=e0000009c9221290
 [<a000000100012290>] ia64_process_pending_intr+0x2b0/0x360
                                sp=e0000009c922fe30 bsp=e0000009c9221208
 [<a0000001000112d0>] fixup_irqs+0xf0/0x2a0
                                sp=e0000009c922fe30 bsp=e0000009c92211a8
 [<a00000010005bd80>] __cpu_disable+0x140/0x240
                                sp=e0000009c922fe30 bsp=e0000009c9221168
 [<a0000001006c5870>] take_cpu_down+0x50/0xa0
                                sp=e0000009c922fe30 bsp=e0000009c9221148
 [<a000000100122610>] stop_cpu+0xd0/0x200
                                sp=e0000009c922fe30 bsp=e0000009c92210f0

Reading through the original thread:

	http://lkml.org/lkml/2008/8/31/116

It looks like Paul fixed his original issue correctly, put in a
new call to cpu_clear() in the wrong spot, and then was convinced
to remove the _correct_ call to cpu_clear().

Cc: stable@kernel.org
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---
In my opinion, this is .29 material.

Sorry for the huge changelog:patch ratio, but this area is tricky
enough that more explanation is better than less, I think.

Also, I'm still a little troubled by Paul's original patch. What
happens if we're trying to offline the CPEI target? The code in
migrate_platform_irqs() uses cpu_online_map to select the new
CPEI target, and it seems like we can end up in the same
situation as the problem I'm trying to fix now.

Paul?

My patch has held up for over 24 hours of stress testing, where
we put the system under a heavy load and then randomly
offline/online CPUs every 2 seconds. Without this patch, the
machine would crash reliably within 15 minutes.

---

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index 1146399..2a17d1c 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -742,8 +742,8 @@ int __cpu_disable(void)
 	}
 
 	remove_siblinginfo(cpu);
-	fixup_irqs();
 	cpu_clear(cpu, cpu_online_map);
+	fixup_irqs();
 	local_flush_tlb_all();
 	cpu_clear(cpu, cpu_callin_map);
 	return 0;

Re: [PATCH] ia64: prevent irq migration race in __cpu_disable path

[Paul E. McKenney]

On Fri, Feb 06, 2009 at 09:22:13AM -0700, Alex Chiang wrote:
> Commit e7b14036 (prevent ia64 from invoking irq handlers on
> offline CPUs) introduced a bug, where we call fixup_irqs before
> removing the CPU from the cpu_online_map.
> 
> This is wrong because fixup_irqs calls migrate_irqs, and in
> migrate_irqs, we use the cpu_online_map to:
> 
> 	1. look for interrupts on current CPU
> 	2. if we find one, move it to the first available CPU in
> 	the cpu_online_map
> 
> This means we can potentially migrate an interrupt off ourself
> back to... ourself. Uh oh.
> 
> We hit a NULL deref later which causes us to oops (output trimmed):
> 
> Unable to handle kernel NULL pointer dereference (address 0000000000000040)
> ip is at profile_tick+0xd0/0x1c0
> 
> Call Trace:
>  [<a00000010003c700>] die+0x1a0/0x2e0
>                                 sp=e0000009c922fbd0 bsp=e0000009c9221438
>  [<a0000001006e92f0>] ia64_do_page_fault+0x950/0xa80
>                                 sp=e0000009c922fbd0 bsp=e0000009c92213d8
>  [<a00000010000c7a0>] ia64_native_leave_kernel+0x0/0x270
>                                 sp=e0000009c922fc60 bsp=e0000009c92213d8
>  [<a0000001000ecdb0>] profile_tick+0xd0/0x1c0
>                                 sp=e0000009c922fe30 bsp=e0000009c9221398
>  [<a00000010003bb90>] timer_interrupt+0x170/0x3e0
>                                 sp=e0000009c922fe30 bsp=e0000009c9221330
>  [<a00000010013a800>] handle_IRQ_event+0x80/0x120
>                                 sp=e0000009c922fe30 bsp=e0000009c92212f8
>  [<a00000010013aa00>] __do_IRQ+0x160/0x4a0
>                                 sp=e0000009c922fe30 bsp=e0000009c9221290
>  [<a000000100012290>] ia64_process_pending_intr+0x2b0/0x360
>                                 sp=e0000009c922fe30 bsp=e0000009c9221208
>  [<a0000001000112d0>] fixup_irqs+0xf0/0x2a0
>                                 sp=e0000009c922fe30 bsp=e0000009c92211a8
>  [<a00000010005bd80>] __cpu_disable+0x140/0x240
>                                 sp=e0000009c922fe30 bsp=e0000009c9221168
>  [<a0000001006c5870>] take_cpu_down+0x50/0xa0
>                                 sp=e0000009c922fe30 bsp=e0000009c9221148
>  [<a000000100122610>] stop_cpu+0xd0/0x200
>                                 sp=e0000009c922fe30 bsp=e0000009c92210f0
> 
> Reading through the original thread:
> 
> 	http://lkml.org/lkml/2008/8/31/116
> 
> It looks like Paul fixed his original issue correctly, put in a
> new call to cpu_clear() in the wrong spot, and then was convinced
> to remove the _correct_ call to cpu_clear().
> 
> Cc: stable@kernel.org
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Alex Chiang <achiang@hp.com>
> ---
> In my opinion, this is .29 material.
> 
> Sorry for the huge changelog:patch ratio, but this area is tricky
> enough that more explanation is better than less, I think.
> 
> Also, I'm still a little troubled by Paul's original patch. What
> happens if we're trying to offline the CPEI target? The code in
> migrate_platform_irqs() uses cpu_online_map to select the new
> CPEI target, and it seems like we can end up in the same
> situation as the problem I'm trying to fix now.
> 
> Paul?
> 
> My patch has held up for over 24 hours of stress testing, where
> we put the system under a heavy load and then randomly
> offline/online CPUs every 2 seconds. Without this patch, the
> machine would crash reliably within 15 minutes.

I don't claim much expertise on IA64 low-level architectural details,
so am reduced to asking the usual question...  Does this patch guarantee
that a given CPU won't be executing irq handlers while marked offline?
If there is no such guarantee, things can break.  (See below.)

In any case, apologies for failing to correctly fix the original
problem!!!

> ---
> 
> diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
> index 1146399..2a17d1c 100644
> --- a/arch/ia64/kernel/smpboot.c
> +++ b/arch/ia64/kernel/smpboot.c
> @@ -742,8 +742,8 @@ int __cpu_disable(void)
>  	}
> 
>  	remove_siblinginfo(cpu);
> -	fixup_irqs();
>  	cpu_clear(cpu, cpu_online_map);
> +	fixup_irqs();

So you argument is that because we are running in the context of
stop_machine(), even though fixup_irqs() does in fact cause irq handlers
to run on the current CPU which is marked offline, the fact that there
is no one running to notice this misbehavior makes it OK?  (Which
perhaps it is, just asking the question.)

							Thanx, Paul

>  	local_flush_tlb_all();
>  	cpu_clear(cpu, cpu_callin_map);
>  	return 0;

Re: [PATCH] ia64: prevent irq migration race in __cpu_disable path

[Alex Chiang]

[removing stable@kernel.org for now while we figure this out]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> On Fri, Feb 06, 2009 at 09:22:13AM -0700, Alex Chiang wrote:
> > ---
> > In my opinion, this is .29 material.
> > 
> > Sorry for the huge changelog:patch ratio, but this area is tricky
> > enough that more explanation is better than less, I think.
> > 
> > Also, I'm still a little troubled by Paul's original patch. What
> > happens if we're trying to offline the CPEI target? The code in
> > migrate_platform_irqs() uses cpu_online_map to select the new
> > CPEI target, and it seems like we can end up in the same
> > situation as the problem I'm trying to fix now.
> > 
> > Paul?
> > 
> > My patch has held up for over 24 hours of stress testing, where
> > we put the system under a heavy load and then randomly
> > offline/online CPUs every 2 seconds. Without this patch, the
> > machine would crash reliably within 15 minutes.
> 
> I don't claim much expertise on IA64 low-level architectural details,

I'm starting to get a bit out of my depth here too... :-/

> so am reduced to asking the usual question...  Does this patch guarantee
> that a given CPU won't be executing irq handlers while marked offline?
> If there is no such guarantee, things can break.  (See below.)

My patch makes no guarantee. What it does do is prevent a NULL
deref while we are, in fact, executing an irq handler while
marked offline.
 
> In any case, apologies for failing to correctly fix the original
> problem!!!

I'm curious, reading through your old change log:

    Make ia64 refrain from clearing a given to-be-offlined CPU's
    bit in the cpu_online_mask until it has processed pending
    irqs.  This change prevents other CPUs from being blindsided
    by an apparently offline CPU nevertheless changing globally
    visible state. 

Was your patch fixing a theoretical problem or a real bug? What
globally visible state were you referencing there?

> > ---
> > 
> > diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
> > index 1146399..2a17d1c 100644
> > --- a/arch/ia64/kernel/smpboot.c
> > +++ b/arch/ia64/kernel/smpboot.c
> > @@ -742,8 +742,8 @@ int __cpu_disable(void)
> >  	}
> > 
> >  	remove_siblinginfo(cpu);
> > -	fixup_irqs();
> >  	cpu_clear(cpu, cpu_online_map);
> > +	fixup_irqs();
> 
> So you argument is that because we are running in the context of
> stop_machine(), even though fixup_irqs() does in fact cause irq handlers
> to run on the current CPU which is marked offline, the fact that there
> is no one running to notice this misbehavior makes it OK?  (Which
> perhaps it is, just asking the question.)

I wouldn't say that I have a solid argument, per se, just fixing
symptoms. ;)

My reading of the cpu_down() path makes it seem like we need to
process pending interrupts on the current CPU, and the original
author certainly thought it was ok to call an irq handler on the
current CPU. We don't disable local irqs until the very last step
of fixup_irqs().

So the actual design of this path assumed it was ok to call an
irq handler on a marked-offline CPU.

Can you educate me on the danger of doing such a thing? That
might help in how I interpret the code.

Thanks.

/ac

Re: [PATCH] ia64: prevent irq migration race in __cpu_disable path

[Paul E. McKenney]

On Fri, Feb 06, 2009 at 11:07:42AM -0700, Alex Chiang wrote:
> 
> [removing stable@kernel.org for now while we figure this out]
> 
> * Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> > On Fri, Feb 06, 2009 at 09:22:13AM -0700, Alex Chiang wrote:
> > > ---
> > > In my opinion, this is .29 material.
> > > 
> > > Sorry for the huge changelog:patch ratio, but this area is tricky
> > > enough that more explanation is better than less, I think.
> > > 
> > > Also, I'm still a little troubled by Paul's original patch. What
> > > happens if we're trying to offline the CPEI target? The code in
> > > migrate_platform_irqs() uses cpu_online_map to select the new
> > > CPEI target, and it seems like we can end up in the same
> > > situation as the problem I'm trying to fix now.
> > > 
> > > Paul?
> > > 
> > > My patch has held up for over 24 hours of stress testing, where
> > > we put the system under a heavy load and then randomly
> > > offline/online CPUs every 2 seconds. Without this patch, the
> > > machine would crash reliably within 15 minutes.
> > 
> > I don't claim much expertise on IA64 low-level architectural details,
> 
> I'm starting to get a bit out of my depth here too... :-/
> 
> > so am reduced to asking the usual question...  Does this patch guarantee
> > that a given CPU won't be executing irq handlers while marked offline?
> > If there is no such guarantee, things can break.  (See below.)
> 
> My patch makes no guarantee. What it does do is prevent a NULL
> deref while we are, in fact, executing an irq handler while
> marked offline.
> 
> > In any case, apologies for failing to correctly fix the original
> > problem!!!
> 
> I'm curious, reading through your old change log:
> 
>     Make ia64 refrain from clearing a given to-be-offlined CPU's
>     bit in the cpu_online_mask until it has processed pending
>     irqs.  This change prevents other CPUs from being blindsided
>     by an apparently offline CPU nevertheless changing globally
>     visible state. 
> 
> Was your patch fixing a theoretical problem or a real bug? What
> globally visible state were you referencing there?
> 
> > > ---
> > > 
> > > diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
> > > index 1146399..2a17d1c 100644
> > > --- a/arch/ia64/kernel/smpboot.c
> > > +++ b/arch/ia64/kernel/smpboot.c
> > > @@ -742,8 +742,8 @@ int __cpu_disable(void)
> > >  	}
> > > 
> > >  	remove_siblinginfo(cpu);
> > > -	fixup_irqs();
> > >  	cpu_clear(cpu, cpu_online_map);
> > > +	fixup_irqs();
> > 
> > So you argument is that because we are running in the context of
> > stop_machine(), even though fixup_irqs() does in fact cause irq handlers
> > to run on the current CPU which is marked offline, the fact that there
> > is no one running to notice this misbehavior makes it OK?  (Which
> > perhaps it is, just asking the question.)
> 
> I wouldn't say that I have a solid argument, per se, just fixing
> symptoms. ;)
> 
> My reading of the cpu_down() path makes it seem like we need to
> process pending interrupts on the current CPU, and the original
> author certainly thought it was ok to call an irq handler on the
> current CPU. We don't disable local irqs until the very last step
> of fixup_irqs().
> 
> So the actual design of this path assumed it was ok to call an
> irq handler on a marked-offline CPU.
> 
> Can you educate me on the danger of doing such a thing? That
> might help in how I interpret the code.

Well, RCU happily ignores CPUs that don't have their bits set in
cpu_online_map, so if there are RCU read-side critical sections in the
irq handlers being run, RCU will ignore them.  If the other CPUs were
running, they might sequence through the RCU state machine, which could
result in data structures being yanked out from under those irq handlers,
which in turn could result in oopses or worse.

							Thanx, Paul

Re: [PATCH] ia64: prevent irq migration race in __cpu_disable path

[Alex Chiang]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> On Fri, Feb 06, 2009 at 11:07:42AM -0700, Alex Chiang wrote:
> > 
> > [removing stable@kernel.org for now while we figure this out]
> > 
> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> > > On Fri, Feb 06, 2009 at 09:22:13AM -0700, Alex Chiang wrote:
> > > > ---
> > > > In my opinion, this is .29 material.
> > > > 
> > > > Sorry for the huge changelog:patch ratio, but this area is tricky
> > > > enough that more explanation is better than less, I think.
> > > > 
> > > > Also, I'm still a little troubled by Paul's original patch. What
> > > > happens if we're trying to offline the CPEI target? The code in
> > > > migrate_platform_irqs() uses cpu_online_map to select the new
> > > > CPEI target, and it seems like we can end up in the same
> > > > situation as the problem I'm trying to fix now.
> > > > 
> > > > Paul?
> > > > 
> > > > My patch has held up for over 24 hours of stress testing, where
> > > > we put the system under a heavy load and then randomly
> > > > offline/online CPUs every 2 seconds. Without this patch, the
> > > > machine would crash reliably within 15 minutes.
> > > 
> > > I don't claim much expertise on IA64 low-level architectural details,
> > 
> > I'm starting to get a bit out of my depth here too... :-/
> > 
> > > so am reduced to asking the usual question...  Does this patch guarantee
> > > that a given CPU won't be executing irq handlers while marked offline?
> > > If there is no such guarantee, things can break.  (See below.)
> > 
> > My patch makes no guarantee. What it does do is prevent a NULL
> > deref while we are, in fact, executing an irq handler while
> > marked offline.
> > 
> > > In any case, apologies for failing to correctly fix the original
> > > problem!!!
> > 
> > I'm curious, reading through your old change log:
> > 
> >     Make ia64 refrain from clearing a given to-be-offlined CPU's
> >     bit in the cpu_online_mask until it has processed pending
> >     irqs.  This change prevents other CPUs from being blindsided
> >     by an apparently offline CPU nevertheless changing globally
> >     visible state. 
> > 
> > Was your patch fixing a theoretical problem or a real bug? What
> > globally visible state were you referencing there?
> > 
> > > > ---
> > > > 
> > > > diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
> > > > index 1146399..2a17d1c 100644
> > > > --- a/arch/ia64/kernel/smpboot.c
> > > > +++ b/arch/ia64/kernel/smpboot.c
> > > > @@ -742,8 +742,8 @@ int __cpu_disable(void)
> > > >  	}
> > > > 
> > > >  	remove_siblinginfo(cpu);
> > > > -	fixup_irqs();
> > > >  	cpu_clear(cpu, cpu_online_map);
> > > > +	fixup_irqs();
> > > 
> > > So you argument is that because we are running in the context of
> > > stop_machine(), even though fixup_irqs() does in fact cause irq handlers
> > > to run on the current CPU which is marked offline, the fact that there
> > > is no one running to notice this misbehavior makes it OK?  (Which
> > > perhaps it is, just asking the question.)
> > 
> > I wouldn't say that I have a solid argument, per se, just fixing
> > symptoms. ;)
> > 
> > My reading of the cpu_down() path makes it seem like we need to
> > process pending interrupts on the current CPU, and the original
> > author certainly thought it was ok to call an irq handler on the
> > current CPU. We don't disable local irqs until the very last step
> > of fixup_irqs().
> > 
> > So the actual design of this path assumed it was ok to call an
> > irq handler on a marked-offline CPU.
> > 
> > Can you educate me on the danger of doing such a thing? That
> > might help in how I interpret the code.
> 
> Well, RCU happily ignores CPUs that don't have their bits set in
> cpu_online_map, so if there are RCU read-side critical sections in the
> irq handlers being run, RCU will ignore them.  If the other CPUs were
> running, they might sequence through the RCU state machine, which could
> result in data structures being yanked out from under those irq handlers,
> which in turn could result in oopses or worse.

Ok, that makes sense.

As I'm continuing to dig, I took a look at the x86 side of the 
house and they have this interesting sequence:

cpu_disable_common()
	remove_cpu_from_maps()  /* remove cpu from online map */
	fixup_irqs()
		[break irq-CPU affinity]

		local_irq_enable();
		mdelay(1);
		local_irq_disable();
                
So in x86, we allow interrupt handlers to run on a CPU that's 
already been removed from the online map.

Does that seem like an analagous situation to what we have in
ia64?

Thanks.

/ac, still digging

RE: [PATCH] ia64: prevent irq migration race in __cpu_disable path

[Luck, Tony]

> This is wrong because fixup_irqs calls migrate_irqs, and in
> migrate_irqs, we use the cpu_online_map to:
>
>       1. look for interrupts on current CPU
>       2. if we find one, move it to the first available CPU in
>       the cpu_online_map
>
> This means we can potentially migrate an interrupt off ourself
> back to... ourself. Uh oh.

Should we make migrate_irqs smarter then ... does any caller really
expect that it would "migrate" the irq to the same cpu?

-Tony

Re: [PATCH] ia64: prevent irq migration race in __cpu_disable path

[Alex Chiang]

* Luck, Tony <tony.luck@intel.com>:
> > This is wrong because fixup_irqs calls migrate_irqs, and in
> > migrate_irqs, we use the cpu_online_map to:
> >
> >       1. look for interrupts on current CPU
> >       2. if we find one, move it to the first available CPU in
> >       the cpu_online_map
> >
> > This means we can potentially migrate an interrupt off ourself
> > back to... ourself. Uh oh.
> 
> Should we make migrate_irqs smarter then ... does any caller really
> expect that it would "migrate" the irq to the same cpu?

The only thing migrate_irqs does is locate irqs that have their
CPU affinity set to the current CPU, and if so, changes the
affinity.

We still have possible pending timer interrupts that we need to
handle, so I'm not seeing how changing the migrate_irqs
implementation (to avoid migrating to ourself) will handle that.

On one hand, I think the only irq handler that can be called at
this point is our timer_interrupt, which doesn't seem to be using
any RCU APIs.

On the other hand, if we really want to make sure that we're not
calling interrupt handlers with our CPU marked as 'offline', then
we need to fix ia64_process_pending_intr() so that we're not
firing our timer_interrupt with a NULL pt_regs.

Add in to the mix that x86 seems to have the same assumption that
we do (that it's ok to call our timer interrupt handler even if
we're already marked as 'offline').

I'm leaning towards reverting Paul's previous commit entirely
because

	- migrate_platform_irqs() doesn't cause any interrupt
	  handlers to be fired by itself

	- it also uses cpu_online)map to find a new CPU to assign
	  CPEI to

Thoughts?

/ac

Re: [PATCH] ia64: prevent irq migration race in __cpu_disable path

[Paul E. McKenney]

On Fri, Feb 06, 2009 at 02:05:50PM -0700, Alex Chiang wrote:
> * Luck, Tony <tony.luck@intel.com>:
> > > This is wrong because fixup_irqs calls migrate_irqs, and in
> > > migrate_irqs, we use the cpu_online_map to:
> > >
> > >       1. look for interrupts on current CPU
> > >       2. if we find one, move it to the first available CPU in
> > >       the cpu_online_map
> > >
> > > This means we can potentially migrate an interrupt off ourself
> > > back to... ourself. Uh oh.
> > 
> > Should we make migrate_irqs smarter then ... does any caller really
> > expect that it would "migrate" the irq to the same cpu?
> 
> The only thing migrate_irqs does is locate irqs that have their
> CPU affinity set to the current CPU, and if so, changes the
> affinity.
> 
> We still have possible pending timer interrupts that we need to
> handle, so I'm not seeing how changing the migrate_irqs
> implementation (to avoid migrating to ourself) will handle that.
> 
> On one hand, I think the only irq handler that can be called at
> this point is our timer_interrupt, which doesn't seem to be using
> any RCU APIs.
> 
> On the other hand, if we really want to make sure that we're not
> calling interrupt handlers with our CPU marked as 'offline', then
> we need to fix ia64_process_pending_intr() so that we're not
> firing our timer_interrupt with a NULL pt_regs.
> 
> Add in to the mix that x86 seems to have the same assumption that
> we do (that it's ok to call our timer interrupt handler even if
> we're already marked as 'offline').
> 
> I'm leaning towards reverting Paul's previous commit entirely
> because
> 
> 	- migrate_platform_irqs() doesn't cause any interrupt
> 	  handlers to be fired by itself
> 
> 	- it also uses cpu_online)map to find a new CPU to assign
> 	  CPEI to
> 
> Thoughts?

I would suggest at least a comment stating why it is safe to take the
interrupts on a CPU marked offline.  As to the eventual solution, you
guys are the experts on your architecture.

							Thanx, Paul