[PATCH 0/8] patchset focus on MIPS SMP woes

[PATCH 0/8] patchset focus on MIPS SMP woes

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

Since commit 5fbd036b [sched: Cleanup cpu_active madness] and
commit 2baab4e9 [sched: Fix select_fallback_rq() vs cpu_active/cpu_online],
it's more safe to put notify_cpu_starting() and set_cpu_online() with
irq disabled, otherwise we will have a typical race condition which
above two commits try to resolve:

      CPU1                            CPU2
__cpu_up();
   mp_ops->boot_secondary();
                              start_secondary();
                                ->init_secondary();
                                  local_irq_enable();
                              <IRQ>
                              do something;
                                    wake up softirqd;
                                    try_to_wake_up();
                                      select_fallback_rq();
                                      /* select wrong cpu */
   set_cpu_online();


This patchset fix the above issue as well as set_cpu_online is
called on the caller cpu.

BTW, I'm only running it on Cavium board because of limited source,
so if anyone is interested to test it on other board, that's great :)

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>

Yong Zhang (8):
  MIPS: Octeon: delay enable irq to ->smp_finish()
  MIPS: BMIPS: delay irq enable to ->smp_finish()
  MIPS: SMTC: delay irq enable to ->smp_finish()
  MIPS: Yosemite: delay irq enable to ->smp_finish()
  MIPS: call ->smp_finish() a little late
  MIPS: call set_cpu_online() on the uping cpu with irq disabled
  MIPS: smp: Warn on too early irq enable
  MIPS: sync-r4k: remove redundant irq operation

 arch/mips/cavium-octeon/smp.c       |    2 +-
 arch/mips/kernel/smp-bmips.c        |   14 +++++++-------
 arch/mips/kernel/smp.c              |   12 +++++++++---
 arch/mips/kernel/smtc.c             |    3 ++-
 arch/mips/kernel/sync-r4k.c         |    5 -----
 arch/mips/pmc-sierra/yosemite/smp.c |    2 +-
 6 files changed, 20 insertions(+), 18 deletions(-)

[PATCH 1/8] MIPS: Octeon: delay enable irq to ->smp_finish()

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

To prepare for smoothing set_cpu_[active|online]() mess up

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: David Daney <david.daney@cavium.com>
---
 arch/mips/cavium-octeon/smp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 97e7ce9..ef9c34a 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -185,7 +185,6 @@ static void __cpuinit octeon_init_secondary(void)
 	octeon_init_cvmcount();
 
 	octeon_irq_setup_secondary();
-	raw_local_irq_enable();
 }
 
 /**
@@ -233,6 +232,7 @@ static void octeon_smp_finish(void)
 
 	/* to generate the first CPU timer interrupt */
 	write_c0_compare(read_c0_count() + mips_hpt_frequency / HZ);
+	local_irq_enable();
 }
 
 /**
-- 
1.7.1

[PATCH 2/8] MIPS: BMIPS: delay irq enable to ->smp_finish()

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

To prepare for smoothing set_cpu_[active|online]() mess up

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 arch/mips/kernel/smp-bmips.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c
index 3046e29..298b437 100644
--- a/arch/mips/kernel/smp-bmips.c
+++ b/arch/mips/kernel/smp-bmips.c
@@ -197,13 +197,6 @@ static void bmips_init_secondary(void)
 
 	write_c0_brcm_action(ACTION_CLR_IPI(smp_processor_id(), 0));
 #endif
-
-	/* make sure there won't be a timer interrupt for a little while */
-	write_c0_compare(read_c0_count() + mips_hpt_frequency / HZ);
-
-	irq_enable_hazard();
-	set_c0_status(IE_SW0 | IE_SW1 | IE_IRQ1 | IE_IRQ5 | ST0_IE);
-	irq_enable_hazard();
 }
 
 /*
@@ -212,6 +205,13 @@ static void bmips_init_secondary(void)
 static void bmips_smp_finish(void)
 {
 	pr_info("SMP: CPU%d is running\n", smp_processor_id());
+
+	/* make sure there won't be a timer interrupt for a little while */
+	write_c0_compare(read_c0_count() + mips_hpt_frequency / HZ);
+
+	irq_enable_hazard();
+	set_c0_status(IE_SW0 | IE_SW1 | IE_IRQ1 | IE_IRQ5 | ST0_IE);
+	irq_enable_hazard();
 }
 
 /*
-- 
1.7.1

[PATCH 3/8] MIPS: SMTC: delay irq enable to ->smp_finish()

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

To prepare for smoothing set_cpu_[active|online]() mess up

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 arch/mips/kernel/smtc.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/mips/kernel/smtc.c b/arch/mips/kernel/smtc.c
index f5dd38f..af46fdd 100644
--- a/arch/mips/kernel/smtc.c
+++ b/arch/mips/kernel/smtc.c
@@ -615,7 +615,6 @@ void __cpuinit smtc_boot_secondary(int cpu, struct task_struct *idle)
 
 void smtc_init_secondary(void)
 {
-	local_irq_enable();
 }
 
 void smtc_smp_finish(void)
@@ -631,6 +630,8 @@ void smtc_smp_finish(void)
 	if (cpu > 0 && (cpu_data[cpu].vpe_id != cpu_data[cpu - 1].vpe_id))
 		write_c0_compare(read_c0_count() + mips_hpt_frequency/HZ);
 
+	local_irq_enable();
+
 	printk("TC %d going on-line as CPU %d\n",
 		cpu_data[smp_processor_id()].tc_id, smp_processor_id());
 }
-- 
1.7.1

[PATCH 4/8] MIPS: Yosemite: delay irq enable to ->smp_finish()

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

To prepare for smoothing set_cpu_[active|online]() mess up

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 arch/mips/pmc-sierra/yosemite/smp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/mips/pmc-sierra/yosemite/smp.c b/arch/mips/pmc-sierra/yosemite/smp.c
index b71fae2..5edab2b 100644
--- a/arch/mips/pmc-sierra/yosemite/smp.c
+++ b/arch/mips/pmc-sierra/yosemite/smp.c
@@ -115,11 +115,11 @@ static void yos_send_ipi_mask(const struct cpumask *mask, unsigned int action)
  */
 static void __cpuinit yos_init_secondary(void)
 {
-	set_c0_status(ST0_CO | ST0_IE | ST0_IM);
 }
 
 static void __cpuinit yos_smp_finish(void)
 {
+	set_c0_status(ST0_CO | ST0_IM | ST0_IE);
 }
 
 /* Hook for after all CPUs are online */
-- 
1.7.1

[PATCH 5/8] MIPS: call ->smp_finish() a little late

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

We have move irq enable to ->smp_finish. Place ->smp_finish() a little
late to prepare for move set_cpu_online() into start_secondary.
And it's not necessary to call cpu_set(cpu, cpu_callin_map) and
synchronise_count_slave() with irq enabled.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 arch/mips/kernel/smp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index ba9376b..73a268a 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -122,13 +122,14 @@ asmlinkage __cpuinit void start_secondary(void)
 
 	notify_cpu_starting(cpu);
 
-	mp_ops->smp_finish();
 	set_cpu_sibling_map(cpu);
 
 	cpu_set(cpu, cpu_callin_map);
 
 	synchronise_count_slave();
 
+	mp_ops->smp_finish();
+
 	cpu_idle();
 }
 
-- 
1.7.1

[PATCH 6/8] MIPS: call set_cpu_online() on the uping cpu with irq disabled

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

To prevent a problem as commit 5fbd036b && 2baab4e9 try to resolve,
move set_cpu_online() to the uping CPU and with irq disabled.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 arch/mips/kernel/smp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 73a268a..042145f 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -122,6 +122,8 @@ asmlinkage __cpuinit void start_secondary(void)
 
 	notify_cpu_starting(cpu);
 
+	set_cpu_online(cpu, true);
+
 	set_cpu_sibling_map(cpu);
 
 	cpu_set(cpu, cpu_callin_map);
@@ -249,8 +251,6 @@ int __cpuinit __cpu_up(unsigned int cpu)
 	while (!cpu_isset(cpu, cpu_callin_map))
 		udelay(100);
 
-	set_cpu_online(cpu, true);
-
 	return 0;
 }
 
-- 
1.7.1

Re: [PATCH 6/8] MIPS: call set_cpu_online() on the uping cpu with irq disabled

[Sergei Shtylyov]

Hello.

On 17-05-2012 14:10, Yong Zhang wrote:

> From: Yong Zhang<yong.zhang@windriver.com>

> To prevent a problem as commit 5fbd036b&&  2baab4e9 try to resolve,

    Please also specify the summary of those commits in parens.

> move set_cpu_online() to the uping CPU and with irq disabled.

    Uping? Maybe "brought up"?

> Signed-off-by: Yong Zhang<yong.zhang0@gmail.com>

WBR, Sergei

[PATCH 7/8] MIPS: smp: Warn on too early irq enable

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

Just to catch a potential issue.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 arch/mips/kernel/smp.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 042145f..0d48598 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -130,6 +130,11 @@ asmlinkage __cpuinit void start_secondary(void)
 
 	synchronise_count_slave();
 
+	/*
+	 * irq will be enabled in ->smp_finish(), enable it too early
+	 * is dangerous.
+	 */
+	WARN_ON_ONCE(!irqs_disabled());
 	mp_ops->smp_finish();
 
 	cpu_idle();
-- 
1.7.1

Re: [PATCH 7/8] MIPS: smp: Warn on too early irq enable

[Sergei Shtylyov]

Hello.

On 17-05-2012 14:10, Yong Zhang wrote:

> From: Yong Zhang<yong.zhang@windriver.com>

> Just to catch a potential issue.

    Grammar nitpicking ahead. :-)

> Signed-off-by: Yong Zhang<yong.zhang0@gmail.com>
> ---
>   arch/mips/kernel/smp.c |    5 +++++
>   1 files changed, 5 insertions(+), 0 deletions(-)

> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> index 042145f..0d48598 100644
> --- a/arch/mips/kernel/smp.c
> +++ b/arch/mips/kernel/smp.c
> @@ -130,6 +130,11 @@ asmlinkage __cpuinit void start_secondary(void)
>
>   	synchronise_count_slave();
>
> +	/*
> +	 * irq will be enabled in ->smp_finish(), enable it too early

    s/enable/enabling/

WBR, Sergei

[PATCH 8/8] MIPS: sync-r4k: remove redundant irq operation

[Yong Zhang]

From: Yong Zhang <yong.zhang@windriver.com>

Since we have delayed irq enable to ->smp_finish()

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 arch/mips/kernel/sync-r4k.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kernel/sync-r4k.c b/arch/mips/kernel/sync-r4k.c
index 99f913c..842d55e 100644
--- a/arch/mips/kernel/sync-r4k.c
+++ b/arch/mips/kernel/sync-r4k.c
@@ -111,7 +111,6 @@ void __cpuinit synchronise_count_master(void)
 void __cpuinit synchronise_count_slave(void)
 {
 	int i;
-	unsigned long flags;
 	unsigned int initcount;
 	int ncpus;
 
@@ -123,8 +122,6 @@ void __cpuinit synchronise_count_slave(void)
 	return;
 #endif
 
-	local_irq_save(flags);
-
 	/*
 	 * Not every cpu is online at the time this gets called,
 	 * so we first wait for the master to say everyone is ready
@@ -154,7 +151,5 @@ void __cpuinit synchronise_count_slave(void)
 	}
 	/* Arrange for an interrupt in a short while */
 	write_c0_compare(read_c0_count() + COUNTON);
-
-	local_irq_restore(flags);
 }
 #undef NR_LOOPS
-- 
1.7.1

Re: [PATCH 0/8] patchset focus on MIPS SMP woes

[David Daney]

On 05/17/2012 03:10 AM, Yong Zhang wrote:
> From: Yong Zhang<yong.zhang@windriver.com>
>
> Since commit 5fbd036b [sched: Cleanup cpu_active madness] and
> commit 2baab4e9 [sched: Fix select_fallback_rq() vs cpu_active/cpu_online],
> it's more safe to put notify_cpu_starting() and set_cpu_online() with
> irq disabled, otherwise we will have a typical race condition which
> above two commits try to resolve:
>
>        CPU1                            CPU2
> __cpu_up();
>     mp_ops->boot_secondary();
>                                start_secondary();
>                                  ->init_secondary();
>                                    local_irq_enable();
>                                <IRQ>
>                                do something;
>                                      wake up softirqd;
>                                      try_to_wake_up();
>                                        select_fallback_rq();
>                                        /* select wrong cpu */
>     set_cpu_online();
>
>
> This patchset fix the above issue as well as set_cpu_online is
> called on the caller cpu.
>
> BTW, I'm only running it on Cavium board because of limited source,
> so if anyone is interested to test it on other board, that's great :)
>
> Signed-off-by: Yong Zhang<yong.zhang0@gmail.com>
>
> Yong Zhang (8):
>    MIPS: Octeon: delay enable irq to ->smp_finish()
>    MIPS: BMIPS: delay irq enable to ->smp_finish()
>    MIPS: SMTC: delay irq enable to ->smp_finish()
>    MIPS: Yosemite: delay irq enable to ->smp_finish()
>    MIPS: call ->smp_finish() a little late
>    MIPS: call set_cpu_online() on the uping cpu with irq disabled
>    MIPS: smp: Warn on too early irq enable
>    MIPS: sync-r4k: remove redundant irq operation
>

This entire patch set (modulo the change log grammar items noted by Sergei):

Acked-by: David Daney <david.daney@cavium.com>


>   arch/mips/cavium-octeon/smp.c       |    2 +-
>   arch/mips/kernel/smp-bmips.c        |   14 +++++++-------
>   arch/mips/kernel/smp.c              |   12 +++++++++---
>   arch/mips/kernel/smtc.c             |    3 ++-
>   arch/mips/kernel/sync-r4k.c         |    5 -----
>   arch/mips/pmc-sierra/yosemite/smp.c |    2 +-
>   6 files changed, 20 insertions(+), 18 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH 0/8] patchset focus on MIPS SMP woes

[Sergei Shtylyov]

Hello.

On 05/17/2012 08:13 PM, David Daney wrote:

>> From: Yong Zhang<yong.zhang@windriver.com>

>> Since commit 5fbd036b [sched: Cleanup cpu_active madness] and
>> commit 2baab4e9 [sched: Fix select_fallback_rq() vs cpu_active/cpu_online],
>> it's more safe to put notify_cpu_starting() and set_cpu_online() with
>> irq disabled, otherwise we will have a typical race condition which
>> above two commits try to resolve:

>> CPU1 CPU2
>> __cpu_up();
>> mp_ops->boot_secondary();
>> start_secondary();
>> ->init_secondary();
>> local_irq_enable();
>> <IRQ>
>> do something;
>> wake up softirqd;
>> try_to_wake_up();
>> select_fallback_rq();
>> /* select wrong cpu */
>> set_cpu_online();


>> This patchset fix the above issue as well as set_cpu_online is
>> called on the caller cpu.

>> BTW, I'm only running it on Cavium board because of limited source,
>> so if anyone is interested to test it on other board, that's great :)

>> Signed-off-by: Yong Zhang<yong.zhang0@gmail.com>

>> Yong Zhang (8):
>> MIPS: Octeon: delay enable irq to ->smp_finish()
>> MIPS: BMIPS: delay irq enable to ->smp_finish()
>> MIPS: SMTC: delay irq enable to ->smp_finish()
>> MIPS: Yosemite: delay irq enable to ->smp_finish()
>> MIPS: call ->smp_finish() a little late
>> MIPS: call set_cpu_online() on the uping cpu with irq disabled
>> MIPS: smp: Warn on too early irq enable
>> MIPS: sync-r4k: remove redundant irq operation

> This entire patch set (modulo the change log grammar items noted by Sergei):

    I noted not only change log grammar, also comment grammar. And a missing 
summary in commit reference. :-)

> Acked-by: David Daney <david.daney@cavium.com>

WBR, Sergei