Re: cpupools and locking

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

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

On 05/05/2010 11:00 AM, Keir Fraser wrote:
> On 05/05/2010 06:25, "Juergen Gross"<juergen.gross@ts.fujitsu.com>  wrote:
>
>> cpupool_do_domctl is called always while the domctl lock is being held. Maybe
>> I should have added a comment to document this assumption.
>
> Yes please. Even better would be to explicitly add your own big lock in
> cpupool.c, just for clarity. And why do you need your 'little' lock around
> the lookup operations at all, in that case? It seems unnecessary if you have
> a bigger lock protecting you; yet insufficient if you do not.

That's the best solution, I think. Patch attached.


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

[-- Attachment #2: poollock.patch --]
[-- Type: text/x-patch, Size: 1386 bytes --]

Signed-off-by: juergen.gross@ts.fujitsu.com

diff -r efa1b905d893 xen/common/cpupool.c
--- a/xen/common/cpupool.c	Tue May 04 13:59:55 2010 +0100
+++ b/xen/common/cpupool.c	Wed May 05 13:01:32 2010 +0200
@@ -37,6 +37,7 @@
  *               as it was obtained!
  */
 static DEFINE_SPINLOCK(cpupool_lock);
+static DEFINE_SPINLOCK(cpupool_ctl_lock);
 
 DEFINE_PER_CPU(struct cpupool *, cpupool);
 
@@ -401,6 +402,8 @@
     int ret;
     struct cpupool *c;
 
+    spin_lock(&cpupool_ctl_lock);
+
     switch ( op->op )
     {
 
@@ -426,9 +429,7 @@
 
     case XEN_DOMCTL_CPUPOOL_OP_DESTROY:
     {
-        spin_lock(&cpupool_lock);
         c = cpupool_find_by_id(op->cpupool_id, 1);
-        spin_unlock(&cpupool_lock);
         ret = -ENOENT;
         if ( c == NULL )
             break;
@@ -438,9 +439,7 @@
 
     case XEN_DOMCTL_CPUPOOL_OP_INFO:
     {
-        spin_lock(&cpupool_lock);
         c = cpupool_find_by_id(op->cpupool_id, 0);
-        spin_unlock(&cpupool_lock);
         ret = -ENOENT;
         if ( c == NULL )
             break;
@@ -484,9 +483,7 @@
     {
         unsigned cpu;
 
-        spin_lock(&cpupool_lock);
         c = cpupool_find_by_id(op->cpupool_id, 0);
-        spin_unlock(&cpupool_lock);
         ret = -ENOENT;
         if ( c == NULL )
             break;
@@ -560,6 +557,8 @@
 
     }
 
+    spin_unlock(&cpupool_ctl_lock);
+
     return ret;
 }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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