[PATCH] prevent powerpc from invoking irq handlers on offline CPUs

[PATCH] prevent powerpc from invoking irq handlers on offline CPUs

[Paul E. McKenney]

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

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---

 smp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5337ca7..1fedd7d 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -250,11 +250,11 @@ int generic_cpu_disable(void)
 	if (cpu == boot_cpuid)
 		return -EBUSY;
 
-	cpu_clear(cpu, cpu_online_map);
 #ifdef CONFIG_PPC64
 	vdso_data->processorCount--;
 	fixup_irqs(cpu_online_map);
 #endif
+	cpu_clear(cpu, cpu_online_map);
 	return 0;
 }
 

Re: [PATCH] prevent powerpc from invoking irq handlers on offline CPUs

[Benjamin Herrenschmidt]

On Sun, 2008-08-31 at 10:31 -0700, Paul E. McKenney wrote:
> Make powerpc 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.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---

Sounds reasonable... the only possible worry here is if somebody tries
an IPI ... The IPI code will and the target CPU mask with the online
map, so it may try to send to the to-be-offlined CPU and timeout, no ?

Cheers,
Ben.

>  smp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 5337ca7..1fedd7d 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -250,11 +250,11 @@ int generic_cpu_disable(void)
>  	if (cpu == boot_cpuid)
>  		return -EBUSY;
>  
> -	cpu_clear(cpu, cpu_online_map);
>  #ifdef CONFIG_PPC64
>  	vdso_data->processorCount--;
>  	fixup_irqs(cpu_online_map);
>  #endif
> +	cpu_clear(cpu, cpu_online_map);
>  	return 0;
>  }
>  

Re: [PATCH] prevent powerpc from invoking irq handlers on offline CPUs

[Paul E. McKenney]

On Mon, Sep 01, 2008 at 10:34:44AM +1000, Benjamin Herrenschmidt wrote:
> On Sun, 2008-08-31 at 10:31 -0700, Paul E. McKenney wrote:
> > Make powerpc 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.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> 
> Sounds reasonable... the only possible worry here is if somebody tries
> an IPI ... The IPI code will and the target CPU mask with the online
> map, so it may try to send to the to-be-offlined CPU and timeout, no ?

OK.  Do we need separate IPI and online masks?

							Thanx, Paul

> Cheers,
> Ben.
> 
> >  smp.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 5337ca7..1fedd7d 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -250,11 +250,11 @@ int generic_cpu_disable(void)
> >  	if (cpu == boot_cpuid)
> >  		return -EBUSY;
> >  
> > -	cpu_clear(cpu, cpu_online_map);
> >  #ifdef CONFIG_PPC64
> >  	vdso_data->processorCount--;
> >  	fixup_irqs(cpu_online_map);
> >  #endif
> > +	cpu_clear(cpu, cpu_online_map);
> >  	return 0;
> >  }
> >  
> 

Re: [PATCH] prevent powerpc from invoking irq handlers on offline CPUs

[Benjamin Herrenschmidt]

On Sun, 2008-08-31 at 19:06 -0700, Paul E. McKenney wrote:
> On Mon, Sep 01, 2008 at 10:34:44AM +1000, Benjamin Herrenschmidt wrote:
> > On Sun, 2008-08-31 at 10:31 -0700, Paul E. McKenney wrote:
> > > Make powerpc 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.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > ---
> > 
> > Sounds reasonable... the only possible worry here is if somebody tries
> > an IPI ... The IPI code will and the target CPU mask with the online
> > map, so it may try to send to the to-be-offlined CPU and timeout, no ?
> 
> OK.  Do we need separate IPI and online masks?

Shouldn't we already have routed all interrupts to other CPUs anyway ?

IE. The affinity of all interrupts should have been updated. So the
only thing we're going to get here are possibly IPIs and decrementer, 
I don't see it being a big deal making sure we test we are online when
receiving it.

Ben.

Re: [PATCH] prevent powerpc from invoking irq handlers on offline CPUs

[Paul E. McKenney]

On Mon, Sep 01, 2008 at 01:14:40PM +1000, Benjamin Herrenschmidt wrote:
> On Sun, 2008-08-31 at 19:06 -0700, Paul E. McKenney wrote:
> > On Mon, Sep 01, 2008 at 10:34:44AM +1000, Benjamin Herrenschmidt wrote:
> > > On Sun, 2008-08-31 at 10:31 -0700, Paul E. McKenney wrote:
> > > > Make powerpc 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.
> > > > 
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > ---
> > > 
> > > Sounds reasonable... the only possible worry here is if somebody tries
> > > an IPI ... The IPI code will and the target CPU mask with the online
> > > map, so it may try to send to the to-be-offlined CPU and timeout, no ?
> > 
> > OK.  Do we need separate IPI and online masks?
> 
> Shouldn't we already have routed all interrupts to other CPUs anyway ?
> 
> IE. The affinity of all interrupts should have been updated. So the
> only thing we're going to get here are possibly IPIs and decrementer, 
> I don't see it being a big deal making sure we test we are online when
> receiving it.

It did look to me that the CPU removed itself from the interrupt queue
before re-enabling interrupts, so makes sense to me...

							Thanx, Paul