smp/hotplug: Move unparking of percpu threads to the control CPU

Vikram reported the following backtrace:

BUG: scheduling while atomic: swapper/7/0/0x00000002
CPU: 7 PID: 0 Comm: swapper/7 Not tainted 4.9.32-perf+ #680
schedule
schedule_hrtimeout_range_clock
schedule_hrtimeout
wait_task_inactive
__kthread_bind_mask
__kthread_bind
__kthread_unpark
kthread_unpark
cpuhp_online_idle
cpu_startup_entry
secondary_start_kernel

He analyzed correctly that a parked cpu hotplug thread of an offlined CPU
was still on the runqueue when the CPU came back online and tried to unpark
it. This causes the thread which invoked kthread_unpark() to call
wait_task_inactive() and subsequently schedule() with preemption disabled.
His proposed workaround was to "make sure" that a parked thread has
scheduled out when the CPU goes offline, so the situation cannot happen.

But that's still wrong because the root cause is not the fact that the
percpu thread is still on the runqueue and neither that preemption is
disabled, which could be simply solved by enabling preemption before
calling kthread_unpark().

The real issue is that the calling thread is the idle task of the upcoming
CPU, which is not supposed to call anything which might sleep. The moron,
who wrote that code, missed completely that kthread_unpark() might end up
in schedule().

The solution is simpler than expected. The thread which controls the
hotplug operation is waiting for the CPU to call complete() on the hotplug
state completion. So the idle task of the upcoming CPU can set its state to
CPUHP_AP_ONLINE_IDLE and invoke complete(). This in turn wakes the control
task on a different CPU, which then can safely do the unpark and kick the
now unparked hotplug thread of the upcoming CPU to complete the bringup to
the final target state.

Control CPU AP

bringup_cpu();
__cpu_up() ------------>
bringup_ap();
bringup_wait_for_ap()
wait_for_completion();
cpuhp_online_idle();
<------------ complete();
unpark(AP->stopper);
unpark(AP->hotplugthread);
while(1)
do_idle();
kick(AP->hotplugthread);
wait_for_completion(); hotplug_thread()
run_online_callbacks();
complete();

Fixes: 8df3e07e7f21 ("cpu/hotplug: Let upcoming cpu bring itself fully up")
Reported-by: Vikram Mulukutla <markivx@codeaurora.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Sewior <bigeasy@linutronix.de>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1707042218020.2131@nanos
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

+19 -18
+19 -18
kernel/cpu.c
··· 271 EXPORT_SYMBOL_GPL(cpu_hotplug_enable); 272 #endif /* CONFIG_HOTPLUG_CPU */ 273 274 static int bringup_wait_for_ap(unsigned int cpu) 275 { 276 struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu); 277 278 wait_for_completion(&st->done); 279 return st->result; 280 } 281 ··· 310 irq_unlock_sparse(); 311 if (ret) 312 return ret; 313 - ret = bringup_wait_for_ap(cpu); 314 - BUG_ON(!cpu_online(cpu)); 315 - return ret; 316 } 317 318 /* ··· 779 } 780 781 /* 782 - * Called from the idle task. We need to set active here, so we can kick off 783 - * the stopper thread and unpark the smpboot threads. If the target state is 784 - * beyond CPUHP_AP_ONLINE_IDLE we kick cpuhp thread and let it bring up the 785 - * cpu further. 786 */ 787 void cpuhp_online_idle(enum cpuhp_state state) 788 { 789 struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); 790 - unsigned int cpu = smp_processor_id(); 791 792 /* Happens for the boot cpu */ 793 if (state != CPUHP_AP_ONLINE_IDLE) 794 return; 795 796 st->state = CPUHP_AP_ONLINE_IDLE; 797 - 798 - /* Unpark the stopper thread and the hotplug thread of this cpu */ 799 - stop_machine_unpark(cpu); 800 - kthread_unpark(st->thread); 801 - 802 - /* Should we go further up ? */ 803 - if (st->target > CPUHP_AP_ONLINE_IDLE) 804 - __cpuhp_kick_ap_work(st); 805 - else 806 - complete(&st->done); 807 } 808 809 /* Requires cpu_add_remove_lock to be held */
··· 271 EXPORT_SYMBOL_GPL(cpu_hotplug_enable); 272 #endif /* CONFIG_HOTPLUG_CPU */ 273 274 + static void __cpuhp_kick_ap_work(struct cpuhp_cpu_state *st); 275 + 276 static int bringup_wait_for_ap(unsigned int cpu) 277 { 278 struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu); 279 280 + /* Wait for the CPU to reach CPUHP_AP_ONLINE_IDLE */ 281 wait_for_completion(&st->done); 282 + BUG_ON(!cpu_online(cpu)); 283 + 284 + /* Unpark the stopper thread and the hotplug thread of the target cpu */ 285 + stop_machine_unpark(cpu); 286 + kthread_unpark(st->thread); 287 + 288 + /* Should we go further up ? */ 289 + if (st->target > CPUHP_AP_ONLINE_IDLE) { 290 + __cpuhp_kick_ap_work(st); 291 + wait_for_completion(&st->done); 292 + } 293 return st->result; 294 } 295 ··· 296 irq_unlock_sparse(); 297 if (ret) 298 return ret; 299 + return bringup_wait_for_ap(cpu); 300 } 301 302 /* ··· 767 } 768 769 /* 770 + * Called from the idle task. Wake up the controlling task which brings the 771 + * stopper and the hotplug thread of the upcoming CPU up and then delegates 772 + * the rest of the online bringup to the hotplug thread. 773 */ 774 void cpuhp_online_idle(enum cpuhp_state state) 775 { 776 struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state); 777 778 /* Happens for the boot cpu */ 779 if (state != CPUHP_AP_ONLINE_IDLE) 780 return; 781 782 st->state = CPUHP_AP_ONLINE_IDLE; 783 + complete(&st->done); 784 } 785 786 /* Requires cpu_add_remove_lock to be held */