From mboxrd@z Thu Jan  1 00:00:00 1970
From: John Weekes <lists.xen@nuclearfallout.net>
Subject: [PATCH] Fix locking bug in vcpu_migrate
Date: Fri, 22 Apr 2011 03:00:26 -0700
Message-ID: <4DB151BA.9010006@nuclearfallout.net>
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="------------080605000508060702090809"
Return-path: <xen-devel-bounces@lists.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
Cc: George Dunlap <george.dunlap@citrix.com>
List-Id: xen-devel@lists.xenproject.org

This is a multi-part message in MIME format.
--------------080605000508060702090809
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

c/s 22957:c5c4688d5654 (unstable) changed the locking scheme for 
vcpu_migrate by adjusting the order so that the lock with the lowest 
lock address is obtained first. However, the code does not release them 
in the correct reverse order; it removes new_lock first if it differs 
from old_lock, but that is not the last lock obtained when old_lock > 
new_lock.

As a result of this bug, under credit2, domUs would sometimes take a 
long time to start, and there was an occasional panic.

This fix should be applied to both xen-unstable and xen-4.1-testing. I 
have tested and seen the problem with both, and also tested to confirm 
an improvement for both.

Signed-off-by: John Weekes <lists.xen@nuclearfallout.net>

diff -r eb4505f8dd97 xen/common/schedule.c
--- a/xen/common/schedule.c     Wed Apr 20 12:02:51 2011 +0100
+++ b/xen/common/schedule.c     Fri Apr 22 03:46:00 2011 -0500
@@ -455,9 +455,20 @@
              pick_called = 0;
          }

-        if ( old_lock != new_lock )
+        if ( old_lock == new_lock )
+        {
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else if ( old_lock < new_lock )
+        {
              spin_unlock(new_lock);
-        spin_unlock_irqrestore(old_lock, flags);
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else
+        {
+            spin_unlock(old_lock);
+            spin_unlock_irqrestore(new_lock, flags);
+        }
      }

      /*
@@ -468,9 +479,20 @@
      if ( v->is_running ||
           !test_and_clear_bit(_VPF_migrating, &v->pause_flags) )
      {
-        if ( old_lock != new_lock )
+        if ( old_lock == new_lock )
+        {
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else if ( old_lock < new_lock )
+        {
              spin_unlock(new_lock);
-        spin_unlock_irqrestore(old_lock, flags);
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else
+        {
+            spin_unlock(old_lock);
+            spin_unlock_irqrestore(new_lock, flags);
+        }
          return;
      }

@@ -491,9 +513,20 @@
       */
      v->processor = new_cpu;

-    if ( old_lock != new_lock )
+    if ( old_lock == new_lock )
+    {
+        spin_unlock_irqrestore(old_lock, flags);
+    }
+    else if ( old_lock < new_lock )
+    {
          spin_unlock(new_lock);
-    spin_unlock_irqrestore(old_lock, flags);
+        spin_unlock_irqrestore(old_lock, flags);
+    }
+    else
+    {
+        spin_unlock_irqrestore(old_lock, flags);
+        spin_unlock(new_lock);
+    }

      if ( old_cpu != new_cpu )
          evtchn_move_pirqs(v);


--------------080605000508060702090809
Content-Type: text/plain;
 name="schedule.dff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="schedule.dff"

diff -r eb4505f8dd97 xen/common/schedule.c
--- a/xen/common/schedule.c     Wed Apr 20 12:02:51 2011 +0100
+++ b/xen/common/schedule.c     Fri Apr 22 03:46:00 2011 -0500
@@ -455,9 +455,20 @@
             pick_called = 0;
         }

-        if ( old_lock != new_lock )
+        if ( old_lock == new_lock )
+        {
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else if ( old_lock < new_lock )
+        {
             spin_unlock(new_lock);
-        spin_unlock_irqrestore(old_lock, flags);
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else
+        {
+            spin_unlock(old_lock);
+            spin_unlock_irqrestore(new_lock, flags);
+        }
     }

     /*
@@ -468,9 +479,20 @@
     if ( v->is_running ||
          !test_and_clear_bit(_VPF_migrating, &v->pause_flags) )
     {
-        if ( old_lock != new_lock )
+        if ( old_lock == new_lock )
+        {
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else if ( old_lock < new_lock )
+        {
             spin_unlock(new_lock);
-        spin_unlock_irqrestore(old_lock, flags);
+            spin_unlock_irqrestore(old_lock, flags);
+        }
+        else
+        {
+            spin_unlock(old_lock);
+            spin_unlock_irqrestore(new_lock, flags);
+        }
         return;
     }

@@ -491,9 +513,20 @@
      */
     v->processor = new_cpu;

-    if ( old_lock != new_lock )
+    if ( old_lock == new_lock )
+    {
+        spin_unlock_irqrestore(old_lock, flags);
+    }
+    else if ( old_lock < new_lock )
+    {
         spin_unlock(new_lock);
-    spin_unlock_irqrestore(old_lock, flags);
+        spin_unlock_irqrestore(old_lock, flags);
+    }
+    else
+    {
+        spin_unlock_irqrestore(old_lock, flags);
+        spin_unlock(new_lock);
+    }

     if ( old_cpu != new_cpu )
         evtchn_move_pirqs(v);

--------------080605000508060702090809
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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

--------------080605000508060702090809--