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@windriver.com>
> >>>
> >>>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@gmail.com>
> >>>Cc: David Daney<ddaney@caviumnetworks.com>
> >>>Cc: Ralf Baechle<ralf@linux-mips.org>
> >>>---
> >>> 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();
> >>> }
> >>>
> >>> /**
> >
|