On 5/2/2018 1:50 PM, Peter Zijlstra wrote:
On Wed, May 02, 2018 at 10:45:52AM +0530, Kohli, Gaurav wrote:
On 5/1/2018 6:49 PM, Peter Zijlstra wrote:

   - complete(&kthread->parked), which we can do inside schedule(); this
     solves the problem because then kthread_park() will not return early
     and the task really is blocked.

I think complete will not help, as problem is like below :

Control Thread                                CPUHP thread
                                        
                                              cpuhp_thread_fun
                                              Wake control thread
                                              complete(&st->done);

takedown_cpu
kthread_park
set_bit(KTHREAD_SHOULD_PARK

                                             Here cpuhp is looping,
                                        //success case
                                             Generally when issue is not
                                             coming
                                             it schedule out by below :
                                            ht->thread_should_run(td->cpu
                                              scheduler
                                        //failure case
                                        before schedule
                                        loop check
                                        (kthread_should_park()
                                         enter here as PARKED set

wake_up_process(k)

If k has TASK_PARKED, then wake_up_process() which uses TASK_NORMAL will
no-op, because:

        TASK_PARKED & TASK_NORMAL == 0

                                        __kthread_parkme
                                         complete(&self->parked);
SETS RUNNING
                                         schedule                       

But suppose, you do get that store, and we get to schedule with
TASK_RUNNING, then schedule will no-op and we'll go around the loop and
not complete.

See also: 
lkml.kernel.org/r/20180430111744.ge4...@hirez.programming.kicks-ass.net

Either TASK_RUNNING gets set before we do schedule() and we go around
again, re-set TASK_PARKED, resched the condition and re-call schedule(),
or we schedule() first and ttwu() will not issue the TASK_RUNNING store.

In either case, we'll eventually hit schedule() with TASK_PARKED. Then,
and only then will the complete() happen.

wait_for_completion(&kthread->parked);

The point is, we'll only ever complete ^ that completion when we've
scheduled out the task in TASK_PARKED state. If the task didn't get
parked, no completion.

Thanks for the detailed explanation, yes in all cases unpark will observe parked state only.


And that is the reason I like this approach above the others. It
guarantees the task really is parked when we ask for it. We don't have
to deal with the task still running and getting migrated to another CPU
nonsense.


--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Reply via email to