Re: [PATCH] MIPS: cavium: Don't enable irq in ->init__secondary()
|[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]|
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().
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, YongSigned-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(); } /**
[Linux MIPS Home] [LKML Archive] [Linux ARM] [Linux] [Git] [Photo] [Yosemite News] [Linux SCSI] [Linux Hams]