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

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

[Paul E. McKenney]

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.

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

 smpboot.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index bcea81e..7a90f42 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -741,8 +741,6 @@ int __cpu_disable(void)
 			return -EBUSY;
 	}
 
-	cpu_clear(cpu, cpu_online_map);
-
 	if (migrate_platform_irqs(cpu)) {
 		cpu_set(cpu, cpu_online_map);
 		return (-EBUSY);
@@ -751,6 +749,7 @@ int __cpu_disable(void)
 	remove_siblinginfo(cpu);
 	cpu_clear(cpu, cpu_online_map);
 	fixup_irqs();
+	cpu_clear(cpu, cpu_online_map);
 	local_flush_tlb_all();
 	cpu_clear(cpu, cpu_callin_map);
 	return 0;

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

[Vegard Nossum]

On Sun, Aug 31, 2008 at 7:28 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> 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.
>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>
>  smpboot.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
> index bcea81e..7a90f42 100644
> --- a/arch/ia64/kernel/smpboot.c
> +++ b/arch/ia64/kernel/smpboot.c
> @@ -741,8 +741,6 @@ int __cpu_disable(void)
>                        return -EBUSY;
>        }
>
> -       cpu_clear(cpu, cpu_online_map);
> -
>        if (migrate_platform_irqs(cpu)) {
>                cpu_set(cpu, cpu_online_map);
>                return (-EBUSY);
> @@ -751,6 +749,7 @@ int __cpu_disable(void)
>        remove_siblinginfo(cpu);
>        cpu_clear(cpu, cpu_online_map);
>        fixup_irqs();
> +       cpu_clear(cpu, cpu_online_map);
>        local_flush_tlb_all();
>        cpu_clear(cpu, cpu_callin_map);
>        return 0;

Hm. Maybe my eyes are going bad. But isn't that now clearing the cpu
both before and after calling fixup_irqs()?


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

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

[Paul E. McKenney]

On Sun, Aug 31, 2008 at 07:46:33PM +0200, Vegard Nossum wrote:
> On Sun, Aug 31, 2008 at 7:28 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > 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.
> >
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >
> >  smpboot.c |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
> > index bcea81e..7a90f42 100644
> > --- a/arch/ia64/kernel/smpboot.c
> > +++ b/arch/ia64/kernel/smpboot.c
> > @@ -741,8 +741,6 @@ int __cpu_disable(void)
> >                        return -EBUSY;
> >        }
> >
> > -       cpu_clear(cpu, cpu_online_map);
> > -
> >        if (migrate_platform_irqs(cpu)) {
> >                cpu_set(cpu, cpu_online_map);
> >                return (-EBUSY);
> > @@ -751,6 +749,7 @@ int __cpu_disable(void)
> >        remove_siblinginfo(cpu);
> >        cpu_clear(cpu, cpu_online_map);
> >        fixup_irqs();
> > +       cpu_clear(cpu, cpu_online_map);
> >        local_flush_tlb_all();
> >        cpu_clear(cpu, cpu_callin_map);
> >        return 0;
> 
> Hm. Maybe my eyes are going bad. But isn't that now clearing the cpu
> both before and after calling fixup_irqs()?

I don't believe so...  I removed the earlier one and added the later
one.  Or is there another one hidden somewhere that I missed?

							Thanx, Paul

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

[Vegard Nossum]

On Sun, Aug 31, 2008 at 7:51 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
>> > @@ -751,6 +749,7 @@ int __cpu_disable(void)
>> >        remove_siblinginfo(cpu);
>> >        cpu_clear(cpu, cpu_online_map);

Here ^^^

>> >        fixup_irqs();
>> > +       cpu_clear(cpu, cpu_online_map);
>> >        local_flush_tlb_all();
>> >        cpu_clear(cpu, cpu_callin_map);
>> >        return 0;
>>
>> Hm. Maybe my eyes are going bad. But isn't that now clearing the cpu
>> both before and after calling fixup_irqs()?
>
> I don't believe so...  I removed the earlier one and added the later
> one.  Or is there another one hidden somewhere that I missed?

See marker above. It's not removed, is it?


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

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

[Paul E. McKenney]

On Sun, Aug 31, 2008 at 07:53:32PM +0200, Vegard Nossum wrote:
> On Sun, Aug 31, 2008 at 7:51 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> >> > @@ -751,6 +749,7 @@ int __cpu_disable(void)
> >> >        remove_siblinginfo(cpu);
> >> >        cpu_clear(cpu, cpu_online_map);
> 
> Here ^^^
> 
> >> >        fixup_irqs();
> >> > +       cpu_clear(cpu, cpu_online_map);
> >> >        local_flush_tlb_all();
> >> >        cpu_clear(cpu, cpu_callin_map);
> >> >        return 0;
> >>
> >> Hm. Maybe my eyes are going bad. But isn't that now clearing the cpu
> >> both before and after calling fixup_irqs()?
> >
> > I don't believe so...  I removed the earlier one and added the later
> > one.  Or is there another one hidden somewhere that I missed?
> 
> See marker above. It's not removed, is it?

Right you are!!!  There were two to begin with, and I moved the first
one only.  The attached updated patch removes the extra one.

						Thanx, Paul

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.  Also remove the existing
redundant cpu_clear(cpu, cpu_online_map).

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

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index bcea81e..d8f05e5 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -741,16 +741,14 @@ int __cpu_disable(void)
 			return -EBUSY;
 	}
 
-	cpu_clear(cpu, cpu_online_map);
-
 	if (migrate_platform_irqs(cpu)) {
 		cpu_set(cpu, cpu_online_map);
 		return (-EBUSY);
 	}
 
 	remove_siblinginfo(cpu);
-	cpu_clear(cpu, cpu_online_map);
 	fixup_irqs();
+	cpu_clear(cpu, cpu_online_map);
 	local_flush_tlb_all();
 	cpu_clear(cpu, cpu_callin_map);
 	return 0;

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

[Vegard Nossum]

On Sun, Aug 31, 2008 at 8:06 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Sun, Aug 31, 2008 at 07:53:32PM +0200, Vegard Nossum wrote:
>> On Sun, Aug 31, 2008 at 7:51 PM, Paul E. McKenney
>> <paulmck@linux.vnet.ibm.com> wrote:
>> >> > @@ -751,6 +749,7 @@ int __cpu_disable(void)
>> >> >        remove_siblinginfo(cpu);
>> >> >        cpu_clear(cpu, cpu_online_map);
>>
>> Here ^^^
>>
>> >> >        fixup_irqs();
>> >> > +       cpu_clear(cpu, cpu_online_map);
>> >> >        local_flush_tlb_all();
>> >> >        cpu_clear(cpu, cpu_callin_map);
>> >> >        return 0;
>> >>
>> >> Hm. Maybe my eyes are going bad. But isn't that now clearing the cpu
>> >> both before and after calling fixup_irqs()?
>> >
>> > I don't believe so...  I removed the earlier one and added the later
>> > one.  Or is there another one hidden somewhere that I missed?
>>
>> See marker above. It's not removed, is it?
>
> Right you are!!!  There were two to begin with, and I moved the first
> one only.  The attached updated patch removes the extra one.

Good, I am not blind yet! (Though I did have to look more than once.)

On the other hand... Paul; human after all!


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

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

[Paul E. McKenney]

On Sun, Aug 31, 2008 at 09:08:34PM +0200, Vegard Nossum wrote:
> On Sun, Aug 31, 2008 at 8:06 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Sun, Aug 31, 2008 at 07:53:32PM +0200, Vegard Nossum wrote:
> >> On Sun, Aug 31, 2008 at 7:51 PM, Paul E. McKenney
> >> <paulmck@linux.vnet.ibm.com> wrote:
> >> >> > @@ -751,6 +749,7 @@ int __cpu_disable(void)
> >> >> >        remove_siblinginfo(cpu);
> >> >> >        cpu_clear(cpu, cpu_online_map);
> >>
> >> Here ^^^
> >>
> >> >> >        fixup_irqs();
> >> >> > +       cpu_clear(cpu, cpu_online_map);
> >> >> >        local_flush_tlb_all();
> >> >> >        cpu_clear(cpu, cpu_callin_map);
> >> >> >        return 0;
> >> >>
> >> >> Hm. Maybe my eyes are going bad. But isn't that now clearing the cpu
> >> >> both before and after calling fixup_irqs()?
> >> >
> >> > I don't believe so...  I removed the earlier one and added the later
> >> > one.  Or is there another one hidden somewhere that I missed?
> >>
> >> See marker above. It's not removed, is it?
> >
> > Right you are!!!  There were two to begin with, and I moved the first
> > one only.  The attached updated patch removes the extra one.
> 
> Good, I am not blind yet! (Though I did have to look more than once.)
> 
> On the other hand... Paul; human after all!

Very much so, I am afraid!  ;-)

							Thanx, Paul