* Oleg Nesterov ([email protected]) wrote:
> On 07/14, Mathieu Desnoyers wrote:
> >
> > @@ -4891,10 +4948,42 @@ static int migration_thread(void *data)
> > list_del_init(head->next);
> >
> > spin_unlock(&rq->lock);
> > - __migrate_task(req->task, cpu, req->dest_cpu);
> > + migrated = __migrate_task(req->task, cpu, req->dest_cpu);
> > local_irq_enable();
> > -
> > - complete(&req->done);
> > + if (!migrated) {
> > + /*
> > + * If the process has not been migrated, let it run
> > + * until it reaches a migration_check() so it can
> > + * wake us up.
> > + */
> > + spin_lock_irq(&rq->lock);
> > + head = &rq->migration_queue;
> > + list_add(&req->list, head);
> > + if (req->task->se.on_rq
> > + || !task_migrate_count(req->task)) {
> > + /*
> > + * The process is on the runqueue, it could
> > + * exit its critical section at any moment,
> > + * don't race with it and retry actively.
> > + * Also, if the thread is not on the runqueue
> > + * and has a zero migration count
> > + * (__migrate_task failed because cpus allowed
> > + * changed), just retry.
> > + */
> > + spin_unlock_irq(&rq->lock);
> > + continue;
>
> Again, this can deadlock. migration_thread() is SCHED_FIFO, and it shares the
> same CPU with req->task. We are doing a busy-wait loop, req->task may have no
> chance to finish its critical section.
>
If we share the CPU with the other thread, it means that it won't be on
the runqueue while we are holding the rq lock. This first case will then
end up in the else branch.
The other case, !task_migrate_count(req->task), is meant to check if the
process has been scheduled between the unlock/lock of the rq. In this
case, it has already decremented its migrate_count to 0.
> > + }
> > + set_tsk_thread_flag(req->task, TIF_NEED_MIGRATE);
> > + set_current_state(TASK_INTERRUPTIBLE);
> > + spin_unlock_irq(&rq->lock);
> > + /*
> > + * Wait for the process currently in its critical
> > + * section.
> > + */
> > + wake_up_process(req->task);
> > + schedule();
>
> As Peter pointed out, we hold up all other migration requests.
>
> And worse, this can hang forever (until another req comes). do_check_migrate()
> can wake_up() the wrong rq.
True.
>
> set_current_state(TASK_INTERRUPTIBLE) is bogus, we can miss a wakeup from
> (say) sched_migrate_task().
>
Yep, this is why it's an RFC. We will have to work on this a little
more.
Regards,
Mathieu
> Oleg.
>
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
