From 55c00ad3d979ecf5d11530eaf15abfc4dcfce73f Mon Sep 17 00:00:00 2001 From: Masayuki Ishikawa Date: Fri, 20 Nov 2020 13:16:45 +0900 Subject: [PATCH] arch: cxd56xx: Fix the pause handler for SMP Summary: - I noticed that sched_add_readytorun() runs on multiple CPUs simultaneously - Finally, I found the root cause which was described in TODO - Actually, the task newly scheduled on remote CPU did not acquire g_cpu_irqlock - This commit fixes this issue by adding a critical section to the pause handler - Which will acquire g_cpu_irqlock on the remote CPU explicitly Impact: - SMP only Testing: - Tested with spresense:wifi_smp (NCPUS=2 and 4) - Run smp, ostest, nxplayer Signed-off-by: Masayuki Ishikawa --- arch/arm/src/cxd56xx/cxd56_cpupause.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/arch/arm/src/cxd56xx/cxd56_cpupause.c b/arch/arm/src/cxd56xx/cxd56_cpupause.c index c8bd4108b45..1f5e6b45eff 100644 --- a/arch/arm/src/cxd56xx/cxd56_cpupause.c +++ b/arch/arm/src/cxd56xx/cxd56_cpupause.c @@ -283,6 +283,7 @@ int up_cpu_paused(int cpu) int arm_pause_handler(int irq, void *c, FAR void *arg) { int cpu = up_cpu_index(); + int ret = OK; DPRINTF("cpu%d will be paused \n", cpu); @@ -297,10 +298,21 @@ int arm_pause_handler(int irq, void *c, FAR void *arg) if (spin_islocked(&g_cpu_paused[cpu])) { - return up_cpu_paused(cpu); + /* NOTE: up_cpu_paused() needs to be executed in a critical section + * to ensure that this CPU holds g_cpu_irqlock. However, adding + * a critical section in up_cpu_paused() is not a good idea, + * because it is also called in enter_critical_section() to break + * a deadlock + */ + + irqstate_t flags = enter_critical_section(); + + ret = up_cpu_paused(cpu); + + leave_critical_section(flags); } - return OK; + return ret; } /****************************************************************************