Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH 2/6] sched: Simplify migration_cpu_stop() | Date | Wed, 24 Feb 2021 15:34:36 +0000 |
| |
On 24/02/21 13:24, Peter Zijlstra wrote: > @@ -1950,31 +1931,20 @@ static int migration_cpu_stop(void *data > goto out; > > if (pending) { > - p->migration_pending = NULL; > + if (p->migration_pending == pending) > + p->migration_pending = NULL; > complete = true; > } > > - /* migrate_enable() -- we must not race against SCA */ > - if (dest_cpu < 0) { > - /* > - * When this was migrate_enable() but we no longer > - * have a @pending, a concurrent SCA 'fixed' things > - * and we should be valid again. Nothing to do. > - */ > - if (!pending) { > - WARN_ON_ONCE(!cpumask_test_cpu(task_cpu(p), &p->cpus_mask)); > - goto out; > - } > -
This is fixed by 5+6, but at this patch I think you can have double completions - I thought this was an issue, but briefly looking at completion stuff it might not. In any case, consider:
task_cpu(p) == Y
SCA(p, X); SCA(p, Y);
SCA(p, Y) will uninstall SCA(p, X)'s pending and complete.
migration/Y kicked by SCA(p, X) will grab arg->pending, which is still SCA(p, X)'s pending and also complete.
> + if (dest_cpu < 0) > dest_cpu = cpumask_any_distribute(&p->cpus_mask); > - } > > if (task_on_rq_queued(p)) > rq = __migrate_task(rq, &rf, p, dest_cpu); > else > p->wake_cpu = dest_cpu; > > - } else if (dest_cpu < 0 || pending) { > + } else if (pending) { > /* > * This happens when we get migrated between migrate_enable()'s > * preempt_enable() and scheduling the stopper task. At that
| |