Re: [PATCH] kthread: Atomically set completion and perform dequeue in __kthread_parkme

From: Vikram Mulukutla
Date: Wed Jul 05 2017 - 13:23:49 EST

On 7/4/2017 12:49 PM, Thomas Gleixner wrote:
On Mon, 26 Jun 2017, Vikram Mulukutla wrote:
On 6/26/2017 3:18 PM, Vikram Mulukutla wrote:
kthread_park waits for the target kthread to park itself with
__kthread_parkme using a completion variable. __kthread_parkme - which is
invoked by the target kthread - sets the completion variable before
calling schedule() to voluntarily get itself off of the runqueue.

This causes an interesting race in the hotplug path. takedown_cpu()
invoked for CPU_X attempts to park the cpuhp/X hotplug kthread before
running the stopper thread on CPU_X. kthread_unpark doesn't guarantee that
cpuhp/X is off of X's runqueue, only that the thread has executed
__kthread_parkme and set the completion. cpuhp/X may have been preempted
out before calling schedule() to voluntarily sleep. takedown_cpu proceeds
to run the stopper thread on CPU_X which promptly migrates off the
still-on-rq cpuhp/X thread to another cpu CPU_Y, setting its affinity
mask to something other than CPU_X alone.

This is OK - cpuhp/X may finally get itself off of CPU_Y's runqueue at
some later point. But if that doesn't happen (for example, if there's
an RT thread on CPU_Y), the kthread_unpark in a subsequent cpu_up call
for CPU_X will race with the still-on-rq condition. Even now we're
functionally OK because there is a wait_task_inactive in the
kthread_unpark(), BUT the following happens:

[ 12.472745] BUG: scheduling while atomic: swapper/7/0/0x00000002

Thats not the worst problem. We could simply enable preemption there, but
the real issue is that this is the idle task of the upcoming CPU which is
not supposed to schedule in the first place.

So no, your 'fix' is just papering over the underlying issue.

And yes, the moron who did not think about wait_task_inactive() being
called via kthread_unpark() -> kthread_bind() is me.

I'm testing a proper fix for it right now. Will post later.

Thanks, it did totally wrong to have any sort of scheduling in the idle
thread as the subsequent warnings do indicate, but I didn't feel
confident enough to mess around with the hotplug state machine.