Re: [PATCH] Avoid endless loop for vcpu migration

[Juergen Gross <juergen.gross@ts.fujitsu.com>]

On 03/15/11 10:58, Keir Fraser wrote:
> On 15/03/2011 09:21, "Juergen Gross"<juergen.gross@ts.fujitsu.com>  wrote:
>
>>> That candidate-is-valid check seems absolutely independent of the
>>> particular scheduler used, and hence could be done in the (sole)
>>> caller, thus not requiring any change to the scheduler interface.
>>>
>>> Which at once would eliminate unnecessary calls into pick_cpu (i.e.
>>> you'd call it a second time only if the previously selected CPU really
>>> is no longer valid to be used for that vCPU).
>>
>> True.
>>
>> The patch seems to become smaller :-)
>
> By the way, why is the cpu_isset(new_cpu, v->domain->vcpupool->cpu_valid)
> check required (after calling pick_cpu, in the cuirrently checked-in code)?

Good question. It shouldn't be required, as pick_cpu should check this and
should return only cpus in the current cpupool. With the latest patches from
Jan this seems to be true. :-)
I think I'll send a separate patch to remove the check.

> You already check that pick_cpu was called holding the correct pair of
> locks, if it has returned a cpu that is not in the pool's cpu_valid mask,
> what would make pick_cpu return anything different on the next invocation
> thus avoiding an endless loop?

Nothing. If pick_cpu is returning a cpu outside of it's cpupool, the loop
could be infinite. Inserting a BUG_ON would be a good idea, but this would
require a bit more logic, as a cpu might be removed from the cpupool during
a running vcpu_migrate (this case is handled by the call of
cpu_disable_scheduler() during removing a cpu from a cpupool).


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html