Avoid stale pointer when moving domain to another cpupool

When a domain is moved to another cpupool the scheduler private data pointers
in vcpu and domain structures must never point to an already freed memory
area.

While at it, simplify sched_init_vcpu() by using DOM2OP instead VCPU2OP.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

This also required commit dbfa7bba0f213b1802e1900b71bc34837c30ee52:

xen, cpupools: Fix cpupool-move to make more consistent

The full order for creating new private data structures when moving
from one pool to another is now:
* Allocate all new structures
 - Allocate a new private domain structure (but don't point there yet)
 - Allocate per-vcpu data structures (but don't point there yet)
* Remove old structures
 - Remove each vcpu, freeing the associated data structure
 - Free the domain data structure
* Switch to the new structures
 - Set the domain to the new cpupool, with the new private domain
 structure
 - Set each vcpu to the respective new structure, and insert

This is in line with a (fairly reasonable) assumption in credit2 that
the private structure of the domain will be the private structure
pointed to by the per-vcpu private structure.

Also fix a bug, in which insert_vcpu was called with the *old* vcpu
ops rather than the new ones.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
master changeset: 482300def7d08e773ccd2a0d978bcb9469fdd810
master date: 2013-02-28 14:56:45 +0000

1 files changed, 22 insertions, 9 deletions

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index be39b20bbd..a116a71dc4 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -227,7 +227,7 @@ int sched_init_vcpu(struct vcpu *v, unsigned int processor)
     if ( v->sched_priv == NULL )
         return 1;
 
-    SCHED_OP(VCPU2OP(v), insert_vcpu, v);
+    SCHED_OP(DOM2OP(d), insert_vcpu, v);
 
     return 0;
 }
@@ -238,6 +238,9 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
     unsigned int new_p;
     void **vcpu_priv;
     void *domdata;
+    void *vcpudata;
+    struct scheduler *old_ops;
+    void *old_domdata;
 
     domdata = SCHED_OP(c->sched, alloc_domdata, d);
     if ( domdata == NULL )
@@ -269,16 +272,26 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
 
     domain_pause(d);
 
+    old_ops = DOM2OP(d);
+    old_domdata = d->sched_priv;
+
+    for_each_vcpu ( d, v )
+    {
+        SCHED_OP(old_ops, remove_vcpu, v);
+    }
+
+    d->cpupool = c;
+    d->sched_priv = domdata;
+
     new_p = first_cpu(c->cpu_valid);
     for_each_vcpu ( d, v )
     {
+        vcpudata = v->sched_priv;
+
         migrate_timer(&v->periodic_timer, new_p);
         migrate_timer(&v->singleshot_timer, new_p);
         migrate_timer(&v->poll_timer, new_p);
 
-        SCHED_OP(VCPU2OP(v), remove_vcpu, v);
-        SCHED_OP(VCPU2OP(v), free_vdata, v->sched_priv);
-
         cpus_setall(v->cpu_affinity);
         v->processor = new_p;
         v->sched_priv = vcpu_priv[v->vcpu_id];
@@ -286,16 +299,16 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
 
         new_p = cycle_cpu(new_p, c->cpu_valid);
 
-        SCHED_OP(VCPU2OP(v), insert_vcpu, v);
+        SCHED_OP(c->sched, insert_vcpu, v);
+
+        SCHED_OP(old_ops, free_vdata, vcpudata);
     }
     domain_update_node_affinity(d);
 
-    d->cpupool = c;
-    SCHED_OP(DOM2OP(d), free_domdata, d->sched_priv);
-    d->sched_priv = domdata;
-
     domain_unpause(d);
 
+    SCHED_OP(old_ops, free_domdata, old_domdata);
+
     xfree(vcpu_priv);
 
     return 0;