Re: Cpu pools discussion

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

Keir Fraser wrote:
> On 28/07/2009 14:41, "George Dunlap" <dunlapg@umich.edu> wrote:
> 
>> As Juergen says, for people who don't use the feature, it shouldn't
>> have any real effect.  The patch is pretty straightforward, except for
>> the "continue_hypercall_on_cpu()" bit.
> 
> 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.
I can add an explicit check not to unassign borrowed cpus, if you like.

> 
> Another thing I noted is that sched_tick_suspend/resume are pointlessly
> changed to take a cpu parameter, which is smp_processor_id(). I swear at the
> screen whenever I see people trying to slip that kind of nonsense in. It

Sorry, this seems to be an artefact of an earlier version of my changes.
I'll remove this one...

> makes it look like the functions can operate on an arbitrary cpu when in
> fact I'll wager they cannot (and I doubt the author of such changes has
> checked). It's a nasty nasty interface change.

I'm pretty sure they could indeed work on any cpu. At least I tried to use
them on other cpus, but I ran into other problems leading to the current
solution not requiring the cpu parameter any more.


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