Re: Cpu pools discussion

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

Keir Fraser wrote:
> On 29/07/2009 07:14, "Juergen Gross" <juergen.gross@ts.fujitsu.com> wrote:
> 
>>> Just pulled up the patch. Actually cpupool_borrow_cpu() does not seem to
>>> lock down the cpu-pool-vcpu relationship while continue_hypercall_on_cpu()
>>> is running. In particular, it is clear that it does nothing if the vcpu is
>>> already part of the pool that the domain is running in. But then what if the
>>> cpu is removed from the pool during the borrow_cpu()/return_cpu() critical
>>> region? It hardly inspires confidence.
>> I checked the use cases.
>> All calls leading to cpupool_borrow_cpu() are done under the domctl lock.
>> The same applies to all cpupool operations.
> 
> Uhhh... How did you figure that one out? I don't think one single caller of
> continue_hypercall_on_cpu() holds the domctl_lock. The callers are all
> sysctls and platform_ops.

Sigh. I just recalled it from memory. Seems I was wrong.

> 
>> I can add an explicit check not to unassign borrowed cpus, if you like.
> 
> Your new interface ought to be responsible for its own synchronisation
> needs. And if it's not you should implement the appropriate assertions
> regarding e.g., spin_is_locked(), plus a code comment. It's simple
> negligence to do neither.

You are right.
I will add a check to ensure borrowed cpus are not allowed to change the pool.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 636 47950
Fujitsu Technolgy Solutions               e-mail: juergen.gross@ts.fujitsu.com
Otto-Hahn-Ring 6                        Internet: ts.fujitsu.com
D-81739 Muenchen                 Company details: ts.fujitsu.com/imprint.html