[RFC PATCH 0/3] vcpu migration improvements

[RFC PATCH 0/3] vcpu migration improvements

[George Dunlap]

Some compile-tested-only sketches of what I'm talking about.  Let me
know what you think.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[RFC PATCH 1/3] xen: Introduce vcpu_sleep_nosync_locked()

[George Dunlap]

There are a lot of places which release a lock before calling
vcpu_sleep_nosync(), which then just grabs the lock again.  This is
not only a waste of time, but leads to more code duplication (since
you have to copy-and-paste recipes rather than calling a unified
function), which in turn leads to an increased chance of bugs.

Introduce vcpu_sleep_nosync_locked(), which can be called if you
already hold the schedule lock.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Dario Faggioli <dfaggioli@suse.com>
---
 xen/common/schedule.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 343ab6306e..1cb8f042e1 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -455,14 +455,9 @@ void sched_destroy_domain(struct domain *d)
     }
 }
 
-void vcpu_sleep_nosync(struct vcpu *v)
+void vcpu_sleep_nosync_locked(struct vcpu *v)
 {
-    unsigned long flags;
-    spinlock_t *lock;
-
-    TRACE_2D(TRC_SCHED_SLEEP, v->domain->domain_id, v->vcpu_id);
-
-    lock = vcpu_schedule_lock_irqsave(v, &flags);
+    ASSERT(spin_is_locked(per_cpu(schedule_data,v->processor).schedule_lock));
 
     if ( likely(!vcpu_runnable(v)) )
     {
@@ -471,6 +466,18 @@ void vcpu_sleep_nosync(struct vcpu *v)
 
         SCHED_OP(vcpu_scheduler(v), sleep, v);
     }
+}
+
+void vcpu_sleep_nosync(struct vcpu *v)
+{
+    unsigned long flags;
+    spinlock_t *lock;
+
+    TRACE_2D(TRC_SCHED_SLEEP, v->domain->domain_id, v->vcpu_id);
+
+    lock = vcpu_schedule_lock_irqsave(v, &flags);
+
+    vcpu_sleep_nosync_locked(v);
 
     vcpu_schedule_unlock_irqrestore(lock, flags, v);
 }
-- 
2.16.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[RFC PATCH 2/3] xen: Refactor migration

[George Dunlap]

The current sequence to initiate vcpu migration is inefficent and error-prone:

- The initiator sets VPF_migraging with the lock held, then drops the
  lock and calls vcpu_sleep_nosync(), which immediately grabs the lock
  again

- A number of places unnecessarily check for v->pause_flags in between
  those two

- Every call to vcpu_migrate() must be prefaced with
  vcpu_sleep_nosync() or introduce a race condition; this code
  duplication is error-prone

- In the event that v->is_running is true at the beginning of
  vcpu_migrate(), it's almost certain that vcpu_migrate() will end up
  being called in context_switch() as well; we might as well simply
  let it run there and save the duplicated effort (which will be
  non-negligible).

Instead, introduce vcpu_migrate_start() to initiate the process.
vcpu_migrate_start() is called with the scheduling lock held.  It not
only sets VPF_migrating, but also calls vcpu_sleep_nosync_locked()
(which will automatically do nothing if there's nothing to do).

Rename vcpu_migrate() to vcpu_migrate_finish().  Check for v->is_running and
pause_flags & VPF_migrating at the top and return if appropriate.

Then the way to initiate migration is consistently:

* Grab lock
* vcpu_migrate_start()
* Release lock
* vcpu_migrate_finish()

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Dario Faggioli <dfaggioli@suse.com>
---
 xen/common/schedule.c | 59 +++++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 1cb8f042e1..5da2a7d8c8 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -593,13 +593,33 @@ static void vcpu_move_nosched(struct vcpu *v, unsigned int new_cpu)
     sched_move_irqs(v);
 }
 
-static void vcpu_migrate(struct vcpu *v)
+/*
+ * Initiating migration
+ * 
+ * In order to migrate, we need the vcpu in question to have stopped
+ * running and be taken off the runqueues; we also need to hold the lock 
+ */
+void vcpu_migrate_start(struct vcpu *v)
+{
+    set_bit(_VPF_migrating, &v->pause_flags);
+    vcpu_sleep_nosync_locked(v);
+}
+
+static void vcpu_migrate_finish(struct vcpu *v)
 {
     unsigned long flags;
     unsigned int old_cpu, new_cpu;
     spinlock_t *old_lock, *new_lock;
     bool_t pick_called = 0;
 
+    /*
+     * If the vcpu is currently running, this will be handled by
+     * context_saved(); and in any case, if the bit is cleared, then
+     * someone else has already done the work so we don't need to.
+     */
+    if ( v->is_running || !test_bit(_VPF_migrating, &v->pause_flags) )
+        return;
+    
     old_cpu = new_cpu = v->processor;
     for ( ; ; )
     {
@@ -679,14 +699,11 @@ void vcpu_force_reschedule(struct vcpu *v)
     spinlock_t *lock = vcpu_schedule_lock_irq(v);
 
     if ( v->is_running )
-        set_bit(_VPF_migrating, &v->pause_flags);
+        vcpu_migrate_start(v);
+
     vcpu_schedule_unlock_irq(lock, v);
 
-    if ( v->pause_flags & VPF_migrating )
-    {
-        vcpu_sleep_nosync(v);
-        vcpu_migrate(v);
-    }
+    vcpu_migrate_finish(v);
 }
 
 void restore_vcpu_affinity(struct domain *d)
@@ -841,10 +858,10 @@ int cpu_disable_scheduler(unsigned int cpu)
                  *  * the scheduler will always fine a suitable solution, or
                  *    things would have failed before getting in here.
                  */
-                set_bit(_VPF_migrating, &v->pause_flags);
+                vcpu_migrate_start(v);
                 vcpu_schedule_unlock_irqrestore(lock, flags, v);
-                vcpu_sleep_nosync(v);
-                vcpu_migrate(v);
+                
+                vcpu_migrate_finish(v);
 
                 /*
                  * The only caveat, in this case, is that if a vcpu active in
@@ -908,18 +925,14 @@ static int vcpu_set_affinity(
             ASSERT(which == v->cpu_soft_affinity);
             sched_set_affinity(v, NULL, affinity);
         }
-        set_bit(_VPF_migrating, &v->pause_flags);
+        vcpu_migrate_start(v);
     }
 
     vcpu_schedule_unlock_irq(lock, v);
 
     domain_update_node_affinity(v->domain);
 
-    if ( v->pause_flags & VPF_migrating )
-    {
-        vcpu_sleep_nosync(v);
-        vcpu_migrate(v);
-    }
+    vcpu_migrate_finish(v);
 
     return ret;
 }
@@ -1147,7 +1160,6 @@ int vcpu_pin_override(struct vcpu *v, int cpu)
         {
             sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
             v->affinity_broken = 0;
-            set_bit(_VPF_migrating, &v->pause_flags);
             ret = 0;
         }
     }
@@ -1160,20 +1172,18 @@ int vcpu_pin_override(struct vcpu *v, int cpu)
             cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
             v->affinity_broken = 1;
             sched_set_affinity(v, cpumask_of(cpu), NULL);
-            set_bit(_VPF_migrating, &v->pause_flags);
             ret = 0;
         }
     }
 
+    if ( ret == 0 )
+        vcpu_migrate_start(v);
+
     vcpu_schedule_unlock_irq(lock, v);
 
     domain_update_node_affinity(v->domain);
 
-    if ( v->pause_flags & VPF_migrating )
-    {
-        vcpu_sleep_nosync(v);
-        vcpu_migrate(v);
-    }
+    vcpu_migrate_finish(v);
 
     return ret;
 }
@@ -1560,8 +1570,7 @@ void context_saved(struct vcpu *prev)
 
     SCHED_OP(vcpu_scheduler(prev), context_saved, prev);
 
-    if ( unlikely(prev->pause_flags & VPF_migrating) )
-        vcpu_migrate(prev);
+    vcpu_migrate_finish(prev);
 }
 
 /* The scheduler timer: force a run through the scheduler */
-- 
2.16.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[RFC PATCH 3/3] sched/credit: Avoid doing cpu_pick calculation twice when deciding to move

[George Dunlap]

In vcpu_acct(), we call _csched_cpu_pick() in order to decide whether
to consider migrating; but then we throw that result away and do it
again in context_saved() if we decide we do need to move.

Instead, just initiate the migration and let the vcpu_migrate_finish()
in context_saved() determine if it should make any changes.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
I had considered calling vcpu_migrate_start() instead, but that seemed
a bit overkill given that I know it's currently running; same with the other
instance of _VPF_migrating.

CC: Dario Faggioli <dfaggioli@suse.com>
---
 xen/common/sched_credit.c | 26 +++++++++-----------------
 xen/common/schedule.c     |  6 ------
 2 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 9bc638c09c..bf3ed09f5d 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -940,7 +940,6 @@ static void
 csched_vcpu_acct(struct csched_private *prv, unsigned int cpu)
 {
     struct csched_vcpu * const svc = CSCHED_VCPU(current);
-    const struct scheduler *ops = per_cpu(scheduler, cpu);
 
     ASSERT( current->processor == cpu );
     ASSERT( svc->sdom != NULL );
@@ -973,7 +972,6 @@ csched_vcpu_acct(struct csched_private *prv, unsigned int cpu)
     }
     else
     {
-        unsigned int new_cpu;
         unsigned long flags;
         spinlock_t *lock = vcpu_schedule_lock_irqsave(current, &flags);
 
@@ -982,24 +980,18 @@ csched_vcpu_acct(struct csched_private *prv, unsigned int cpu)
          * migrating it to run elsewhere (see multi-core and multi-thread
          * support in csched_cpu_pick()).
          */
-        new_cpu = _csched_cpu_pick(ops, current, 0);
+        set_bit(_VPF_migrating, &current->pause_flags);
+        cpu_raise_softirq(cpu, SCHEDULE_SOFTIRQ);
 
         vcpu_schedule_unlock_irqrestore(lock, flags, current);
 
-        if ( new_cpu != cpu )
-        {
-            SCHED_VCPU_STAT_CRANK(svc, migrate_r);
-            SCHED_STAT_CRANK(migrate_running);
-            set_bit(_VPF_migrating, &current->pause_flags);
-            /*
-             * As we are about to tickle cpu, we should clear its bit in
-             * idlers. But, if we are here, it means there is someone running
-             * on it, and hence the bit must be zero already.
-             */
-            ASSERT(!cpumask_test_cpu(cpu,
-                                     CSCHED_PRIV(per_cpu(scheduler, cpu))->idlers));
-            cpu_raise_softirq(cpu, SCHEDULE_SOFTIRQ);
-        }
+        /*
+         * As we are about to tickle cpu, we should clear its bit in
+         * idlers. But, if we are here, it means there is someone running
+         * on it, and hence the bit must be zero already.
+         */
+        ASSERT(!cpumask_test_cpu(cpu,
+                                 CSCHED_PRIV(per_cpu(scheduler, cpu))->idlers));
     }
 }
 
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 5da2a7d8c8..f6fa0e7e4d 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -593,12 +593,6 @@ static void vcpu_move_nosched(struct vcpu *v, unsigned int new_cpu)
     sched_move_irqs(v);
 }
 
-/*
- * Initiating migration
- * 
- * In order to migrate, we need the vcpu in question to have stopped
- * running and be taken off the runqueues; we also need to hold the lock 
- */
 void vcpu_migrate_start(struct vcpu *v)
 {
     set_bit(_VPF_migrating, &v->pause_flags);
-- 
2.16.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [RFC PATCH 1/3] xen: Introduce vcpu_sleep_nosync_locked()

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 879 bytes --]

On Wed, 2018-04-11 at 13:25 +0100, George Dunlap wrote:
> There are a lot of places which release a lock before calling
> vcpu_sleep_nosync(), which then just grabs the lock again.  This is
> not only a waste of time, but leads to more code duplication (since
> you have to copy-and-paste recipes rather than calling a unified
> function), which in turn leads to an increased chance of bugs.
> 
> Introduce vcpu_sleep_nosync_locked(), which can be called if you
> already hold the schedule lock.
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [RFC PATCH 2/3] xen: Refactor migration

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2812 bytes --]

On Wed, 2018-04-11 at 13:25 +0100, George Dunlap wrote:
> The current sequence to initiate vcpu migration is inefficent and
> error-prone:
> 
> - The initiator sets VPF_migraging with the lock held, then drops the
>   lock and calls vcpu_sleep_nosync(), which immediately grabs the
> lock
>   again
> 
> - A number of places unnecessarily check for v->pause_flags in
> between
>   those two
> 
> - Every call to vcpu_migrate() must be prefaced with
>   vcpu_sleep_nosync() or introduce a race condition; this code
>   duplication is error-prone
> 
It's not only error prone, it's the cause of actual bugs! :-P

That's why I would also change the subject of the series into something
like "fix races happening during vcpu migration"

> - In the event that v->is_running is true at the beginning of
>   vcpu_migrate(), it's almost certain that vcpu_migrate() will end up
>   being called in context_switch() as well; we might as well simply
>   let it run there and save the duplicated effort (which will be
>   non-negligible).
> 
> Instead, introduce vcpu_migrate_start() to initiate the process.
> vcpu_migrate_start() is called with the scheduling lock held.  It not
> only sets VPF_migrating, but also calls vcpu_sleep_nosync_locked()
> (which will automatically do nothing if there's nothing to do).
> 
> Rename vcpu_migrate() to vcpu_migrate_finish().  Check for v-
> >is_running and
> pause_flags & VPF_migrating at the top and return if appropriate.
> 
> Then the way to initiate migration is consistently:
> 
> * Grab lock
> * vcpu_migrate_start()
> * Release lock
> * vcpu_migrate_finish()
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -593,13 +593,33 @@ static void vcpu_move_nosched(struct vcpu *v,
> unsigned int new_cpu)
>      sched_move_irqs(v);
>  }
>  
> -static void vcpu_migrate(struct vcpu *v)
> +/*
> + * Initiating migration
> + * 
> + * In order to migrate, we need the vcpu in question to have stopped
> + * running and be taken off the runqueues; we also need to hold the
> lock 
> + */
>
Perhaps "and SCHED_OP(sleep) to have been called on it", instead of
"and be taken off the runqueues", if we don't want to even mention
runqueues in schedule.c.

And there are a couple of instances of whitespace damaging.

Nevertheless,

Reviewed-by: Dario Faggioli <dfaggioli@suse.com>

And I guess we can add a 'Tested-by: Olaf Hering', as he actually did
that, what do you say Olaf?

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [RFC PATCH 3/3] sched/credit: Avoid doing cpu_pick calculation twice when deciding to move

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2023 bytes --]

On Wed, 2018-04-11 at 13:25 +0100, George Dunlap wrote:
> In vcpu_acct(), we call _csched_cpu_pick() in order to decide whether
> to consider migrating; but then we throw that result away and do it
> again in context_saved() if we decide we do need to move.
> 
> Instead, just initiate the migration and let the
> vcpu_migrate_finish()
> in context_saved() determine if it should make any changes.
> 
I am ambivalent about this. In fact, I never liked the duplicated
pick_cpu effort myself.

Still, what happens right now is:
- vcpu_acct() calls _csched_cpu_pick();
- if the returned cpu is equal to where the vcpu is currently running,
  nothing happens;
- if it is different, we initiate a migration, and go through pick 
  again.

So, we have the duplicated call to pick.

Initiating a migration means making the running vcpu not runnable and
hence de-scheduling it (in favour of either idle or some other runnable
vcpu). Then, in vcpu_migrate_finish(), we make it runnable again, put
it back in a runqueue, and raise the SCHEDULE_SOFTIRQ on the pCPU, if
appropriate. It's likely that the pCPU in question is different from
the one where the vcpu was running when vccpu_acct() was invoked.

After this patch, vcpu_acct() initiate a migration unconditionally.
This means we avoid the overhead of the double call to pick, but we
always go through de-scheduling current. This potentially means letting
a runnable vcpu preempt current just for figuring out immediately after
that current should not really migrate, and have it preempting the
other vcpu again.

So, AFAICT, we're saving some overhead, but introducing some other...

Anyway, this patch is not really necessary for fixing the race, so I'd
leave it aside for now.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [RFC PATCH 0/3] vcpu migration improvements

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1754 bytes --]

On Wed, 2018-04-11 at 13:25 +0100, George Dunlap wrote:
> Some compile-tested-only sketches of what I'm talking about.  Let me
> know what you think.
> 
So, patches 1 and 2 of this series solves what I think was one of the
nastiest races I've ever had to chase in the scheduler. :-)

Having figured out what the exact root cause of the race itself is,
this is the _proper_ fix, as it puts setting of VPF_migrate and
SCHED_op(sleep) inside the same critical section, which is what closes
the race window.

I'd like to argue for this series to be considered a bugfix, and
included in 4.11 (and backported as far as possible, which has been
already proved to be feasible, e.g., until 4.7).

The alternative would be to come up with something else which kind of
works around the race, within sched_credit.c... But I don't really see
a reason for doing that. Code-wise, it may probably be a bit more self-
contained, but it's not like this series is that spread/intrusive in
the first place.

And the net effect would be basically the same. I.e., in both cases, we
need to change what happens when vcpu_migrate() is called, and I don't
see much difference between doing that by changing vcpu_migrate()
itself, or by changing how Credit react to vcpu_migrate() being called
(especially considering that Credit is the default scheduler).

And therefore, between a proper fix and a workaround, which have
similar impact and effects, I think we should go for the former. :-)

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [RFC PATCH 2/3] xen: Refactor migration

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 309 bytes --]

Am Tue, 17 Apr 2018 09:20:33 +0200
schrieb Dario Faggioli <dfaggioli@suse.com>:

> And I guess we can add a 'Tested-by: Olaf Hering', as he actually did
> that, what do you say Olaf?

Yes, that is true. I have tested these three patches with staging.

Tested-by: Olaf Hering <olaf@aepfle.de>

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [RFC PATCH 1/3] xen: Introduce vcpu_sleep_nosync_locked()

[Juergen Gross]

On 11/04/18 14:25, George Dunlap wrote:
> There are a lot of places which release a lock before calling
> vcpu_sleep_nosync(), which then just grabs the lock again.  This is
> not only a waste of time, but leads to more code duplication (since
> you have to copy-and-paste recipes rather than calling a unified
> function), which in turn leads to an increased chance of bugs.
> 
> Introduce vcpu_sleep_nosync_locked(), which can be called if you
> already hold the schedule lock.
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [RFC PATCH 2/3] xen: Refactor migration

[Juergen Gross]

On 11/04/18 14:25, George Dunlap wrote:
> The current sequence to initiate vcpu migration is inefficent and error-prone:
> 
> - The initiator sets VPF_migraging with the lock held, then drops the
>   lock and calls vcpu_sleep_nosync(), which immediately grabs the lock
>   again
> 
> - A number of places unnecessarily check for v->pause_flags in between
>   those two
> 
> - Every call to vcpu_migrate() must be prefaced with
>   vcpu_sleep_nosync() or introduce a race condition; this code
>   duplication is error-prone
> 
> - In the event that v->is_running is true at the beginning of
>   vcpu_migrate(), it's almost certain that vcpu_migrate() will end up
>   being called in context_switch() as well; we might as well simply
>   let it run there and save the duplicated effort (which will be
>   non-negligible).
> 
> Instead, introduce vcpu_migrate_start() to initiate the process.
> vcpu_migrate_start() is called with the scheduling lock held.  It not
> only sets VPF_migrating, but also calls vcpu_sleep_nosync_locked()
> (which will automatically do nothing if there's nothing to do).
> 
> Rename vcpu_migrate() to vcpu_migrate_finish().  Check for v->is_running and
> pause_flags & VPF_migrating at the top and return if appropriate.
> 
> Then the way to initiate migration is consistently:
> 
> * Grab lock
> * vcpu_migrate_start()
> * Release lock
> * vcpu_migrate_finish()
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel