Skip to content

Commit 6004eb4

Browse files
mpredfearngregkh
authored andcommitted
MIPS: SMP: Fix deadlock & online race
commit 9e8c399a88f0b87e41a894911475ed2a8f8dff9e upstream. Commit 6f542ebeaee0 ("MIPS: Fix race on setting and getting cpu_online_mask") effectively reverted commit 8f46cca1e6c06 ("MIPS: SMP: Fix possibility of deadlock when bringing CPUs online") and thus has reinstated the possibility of deadlock. The commit was based on testing of kernel v4.4, where the CPU hotplug core code issued a BUG() if the starting CPU is not marked online when the boot CPU returns from __cpu_up. The commit fixes this race (in v4.4), but re-introduces the deadlock situation. As noted in the commit message, upstream differs in this area. Commit 8df3e07e7f21f ("cpu/hotplug: Let upcoming cpu bring itself fully up") adds a completion event in the CPU hotplug core code, making this race impossible. However, people were unhappy with relying on the core code to do the right thing. To address the issues both commits were trying to fix, add a second completion event in the MIPS smp hotplug path. It removes the possibility of a race, since the MIPS smp hotplug code now synchronises both the boot and secondary CPUs before they return to the hotplug core code. It also addresses the deadlock by ensuring that the secondary CPU is not marked online before it's counters are synchronised. This fix should also be backported to fix the race condition introduced by the backport of commit 8f46cca1e6c06 ("MIPS: SMP: Fix possibility of deadlock when bringing CPUs online"), through really that race only existed before commit 8df3e07e7f21f ("cpu/hotplug: Let upcoming cpu bring itself fully up"). Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com> Fixes: 6f542ebeaee0 ("MIPS: Fix race on setting and getting cpu_online_mask") CC: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com> Patchwork: https://patchwork.linux-mips.org/patch/17376/ Signed-off-by: James Hogan <jhogan@kernel.org> [jhogan@kernel.org: Backported 4.1..4.9] Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 857e810 commit 6004eb4

1 file changed

Lines changed: 16 additions & 6 deletions

File tree

arch/mips/kernel/smp.c

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ EXPORT_SYMBOL(cpu_sibling_map);
6464
cpumask_t cpu_core_map[NR_CPUS] __read_mostly;
6565
EXPORT_SYMBOL(cpu_core_map);
6666

67+
static DECLARE_COMPLETION(cpu_starting);
6768
static DECLARE_COMPLETION(cpu_running);
6869

6970
/*
@@ -176,15 +177,24 @@ asmlinkage void start_secondary(void)
176177
cpumask_set_cpu(cpu, &cpu_coherent_mask);
177178
notify_cpu_starting(cpu);
178179

180+
/* Notify boot CPU that we're starting & ready to sync counters */
181+
complete(&cpu_starting);
182+
183+
synchronise_count_slave(cpu);
184+
185+
/* The CPU is running and counters synchronised, now mark it online */
179186
set_cpu_online(cpu, true);
180187

181188
set_cpu_sibling_map(cpu);
182189
set_cpu_core_map(cpu);
183190

184191
calculate_cpu_foreign_map();
185192

193+
/*
194+
* Notify boot CPU that we're up & online and it can safely return
195+
* from __cpu_up
196+
*/
186197
complete(&cpu_running);
187-
synchronise_count_slave(cpu);
188198

189199
/*
190200
* irq will be enabled in ->smp_finish(), enabling it too early
@@ -250,17 +260,17 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
250260
{
251261
mp_ops->boot_secondary(cpu, tidle);
252262

253-
/*
254-
* We must check for timeout here, as the CPU will not be marked
255-
* online until the counters are synchronised.
256-
*/
257-
if (!wait_for_completion_timeout(&cpu_running,
263+
/* Wait for CPU to start and be ready to sync counters */
264+
if (!wait_for_completion_timeout(&cpu_starting,
258265
msecs_to_jiffies(1000))) {
259266
pr_crit("CPU%u: failed to start\n", cpu);
260267
return -EIO;
261268
}
262269

263270
synchronise_count_master(cpu);
271+
272+
/* Wait for CPU to finish startup & mark itself online before return */
273+
wait_for_completion(&cpu_running);
264274
return 0;
265275
}
266276

0 commit comments

Comments
 (0)