Messages in this thread Patch in this message | | | Date | Fri, 13 Nov 2020 11:27:34 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v4 10/19] sched: Fix migrate_disable() vs set_cpus_allowed_ptr() |
| |
On Thu, Nov 12, 2020 at 07:31:12PM +0000, Valentin Schneider wrote:
> I think what is happening here is: > > affine_move_task() > // task_running() case > stop_one_cpu() > wait_for_completion(&pending->done); > > and this is !PREEMPT, so the stopper can very well hit: > > migration_cpu_stop() > // Task moved between unlocks and scheduling the stopper > task_rq(p) != rq && > // task_running() case > dest_cpu >= 0 > > => no complete_all(), ever :(
Damn...
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 02076e6d3792..fad0a8e62aca 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -1923,7 +1923,7 @@ static int migration_cpu_stop(void *data) > else > p->wake_cpu = dest_cpu; > > - } else if (dest_cpu < 0) { > + } else if (dest_cpu < 0 || pending) { > /* > * This happens when we get migrated between migrate_enable()'s > * preempt_enable() and scheduling the stopper task. At that > @@ -1933,6 +1933,17 @@ static int migration_cpu_stop(void *data) > * more likely. > */ > > + /* > + * The task moved before the stopper got to run. We're holding > + * ->pi_lock, so the allowed mask is stable - if it got > + * somewhere allowed, we're done. > + */ > + if (pending && cpumask_test_cpu(task_cpu(p), p->cpus_ptr)) { > + p->migration_pending = NULL; > + complete = true; > + goto out; > + } > + > /* > * When this was migrate_enable() but we no longer have an > * @pending, a concurrent SCA 'fixed' things and we should be
Agreed, this is very clearly a missing case and the proposed solution seems straight forward enough; but I'm struggling to convince my sleep deprived brain we're actually complete now.
I'll continue staring at it a little more. Could you make it into a proper patch?
| |