On Tue, Apr 17, 2012 at 01:45:50PM -0700, David Daney wrote: > On 04/16/2012 08:08 PM, Yong Zhang wrote: > >On Mon, Apr 16, 2012 at 09:48:14AM -0700, David Daney wrote: > >>On 04/16/2012 12:25 AM, Yong Zhang wrote: > >>>From: Yong Zhang<yong.zhang@xxxxxxxxxxxxx> > >>> > >>>Too early to enable irq will break some following action, > >>>such as notify_cpu_starting(). > >> > >>Can you be more specific about what breaks? > > > >For example: > > > > CPU1 CPU2 > >__cpu_up(); > > mp_ops->boot_secondary(); > > start_secondary(); > > octeon_init_secondary(); > > raw_local_irq_enable(); > > <IRQ> > > do something; > > wake up softirqd; > > try_to_wake_up(); > > select_fallback_rq(); > > /* select wrong cpu */ > > set_cpu_online(); > > > > Yeah, that looks broken to me too. > > >> > >>> > >>>I don't get side effect with this patch. > >> > >>Without this, where do irqs get enabled on the secondary CPUs? > > > >cpu_idle() will handle it. But in fact we should not depend on > >cpu_idle(). > > It is not done in cpu_idle() itself. If irqs are disabled upon > entry to cpu_idle() *and* need_resched(), then we call into > schedule(). The irqs are then enabled by the > raw_spin_unlock_irq(&rq->lock) at the end of kernel/sched/core.c: > __schedule(). > > > > >But it seems there is not suitable place to put local_irq_enable(), > >though ->smp_finish() looks like a more suitable place. > > It would be better, but it seems like really it should be done in > cpu_idle() immediately after rcu_idle_enter(). Hmm... seems we should put it before enter while (1) in cpu_idle(), otherwise tick_nohz_idle_enter() will be unhappy if NO_HZ enabled. But anyway, we still cann't survive from set_cpu_online() called on other cpu. I'll recheck smp support for MIPS to see if things could be improved ;-) Thanks, Yong > > > > >When looking more at smp support on MIPS, there is more things I find. > >Such as set_cpu_online() is called on CPU1, so there will be another race > >window like above scenario. Please take a look at what commit 2baab4e9 > >intend to resolve. > > > >Thanks, > >Yong > > > > > > > > > >> > >>> > >>>Signed-off-by: Yong Zhang<yong.zhang0@xxxxxxxxx> > >>>Cc: David Daney<ddaney@xxxxxxxxxxxxxxxxxx> > >>>Cc: Ralf Baechle<ralf@xxxxxxxxxxxxxx> > >>>--- > >>> arch/mips/cavium-octeon/smp.c | 1 - > >>> 1 files changed, 0 insertions(+), 1 deletions(-) > >>> > >>>diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c > >>>index 97e7ce9..7e65c88 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(); > >>> } > >>> > >>> /** > >
