WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Cpu pools discussion

To: Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] Cpu pools discussion
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Wed, 29 Jul 2009 10:35:43 +0100
Cc: Tim Deegan <Tim.Deegan@xxxxxxxxxxxxx>, George Dunlap <dunlapg@xxxxxxxxx>, Zhigang Wang <zhigang.x.wang@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 29 Jul 2009 02:36:24 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4A700DB3.9010700@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcoQKdp7piT1aq88SqCPUWQbK6bUigABha/y
Thread-topic: [Xen-devel] Cpu pools discussion
User-agent: Microsoft-Entourage/12.20.0.090605


On 29/07/2009 09:52, "Juergen Gross" <juergen.gross@xxxxxxxxxxxxxx> wrote:

>>> 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.

A couple more comments.

It is not safe to domain_pause() while you hold locks. It can deadlock, as
domain_pause() waits for the domain to be descheduled, but it could be
spinning on a lock you hold. Also it looks like a domain can be moved away
from a pool while the pool is paused, and then you would leak a pause
refcount.

Secondly, I think that the cpupool_borrow/return calls should be embedded
within vcpu_{lock,unlock,locked_change}_affinity(); also I see no need to
have cpupool_return_cpu() return anything as you should be able to make a
decision to move onto another CPU on the next scheduling round anyway (which
can always be forced by setting SCHEDULE_SOFTIRQ).

Really I dislike this patch greatly, as you can tell. ;-) The patchset as a
whole is *ginormous*, the Xen patch by itself is pretty big and complicated
and I believe full of races and deadlocks. I've just picked up on a few
obvious ones from a very brief read.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel