[Xen-devel] [PATCH 07/16] xen: sched: prepare a .switch_sched hook for Credit2

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

In fact, right now, if we switch cpu X from, say,
Credit to Credit2, we do:

 schedule_cpu_switch(X, csched --> csched2):
   //scheduler[x] is csched
   //schedule_lock[x] is csched_lock
   csched2_alloc_pdata(x)
   csched2_init_pdata(x)
     pcpu_schedule_lock(x) --> takes csched_lock
     schedule_lock[x] = csched2_lock
     spin_unlock(csched_lock)
   [1]
   pcpu_schedule_lock(x) ----> takes csched2_lock
   scheduler[X] = csched2
   pcpu_schedule_unlock(x) --> unlocks csched2_lock
   csched_free_pdata(x)

So, if anything scheduling related and involving CPU X
happens, at time [1], we will:
 - take csched2_lock,
 - operate on Credit1 functions and data structures,
which is no good!

Furthermore, if we switch cpu X from RTDS to Credit2,
we do:

 schedule_cpu_switch(X, RTDS --> csched2):
   //scheduler[x] is rtds
   //schedule_lock[x] is rtds_lock
   csched2_alloc_pdata(x)
   csched2_init_pdata(x)
     pcpu_schedule_lock(x) --> takes rtds_lock
     schedule_lock[x] = csched2_lock
     spin_unlock(rtds_lock)
   pcpu_schedule_lock(x) ----> takes csched2_lock
   scheduler[x] = csched2
   pcpu_schedule_unlock(x) --> unlocks csched2_lock
   rtds_free_pdata(x)
     spin_lock(rtds_lock)
     ASSERT(schedule_lock[x] == rtds_lock) [2]
     schedule_lock[x] = DEFAULT_SCHEDULE_LOCK [3]
     spin_unlock(rtds_lock)

Which means:
 1) the ASSERT at [2] triggers!
 2) At [3], we screw up the lock remapping we've done for
    ourself in csched2_init_pdata()!

The former problem arises because there is a window during
which the lock is already the new one, but the scheduler is
still the old one. The latter problem, becase we let other's
scheduler mess with lock (re)mapping during their freeing
path, instead of doing it ourself.

This patch, therefore, introduces the new switch_sched hook,
for Credit2, as done already (in "xen: sched: prepare
.switch_sched for Credit1") for Credit1.

Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
---
Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
---
 xen/common/sched_credit2.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 919ca13..25d8e85 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1968,7 +1968,8 @@ static void deactivate_runqueue(struct csched2_private 
*prv, int rqi)
     cpumask_clear_cpu(rqi, &prv->active_queues);
 }
 
-static void
+/* Returns the ID of the runqueue the cpu is assigned to. */
+static unsigned
 init_pdata(struct csched2_private *prv, unsigned int cpu)
 {
     unsigned rqi;
@@ -2021,7 +2022,7 @@ init_pdata(struct csched2_private *prv, unsigned int cpu)
 
     cpumask_set_cpu(cpu, &prv->initialized);
 
-    return;
+    return rqi;
 }
 
 static void
@@ -2035,6 +2036,43 @@ csched2_init_pdata(const struct scheduler *ops, void 
*pdata, int cpu)
     spin_unlock_irqrestore(&prv->lock, flags);
 }
 
+/* Change the scheduler of cpu to us (Credit2). */
+static void
+csched2_switch_sched(struct scheduler *new_ops, unsigned int cpu,
+                     void *pdata, void *vdata)
+{
+    struct csched2_private *prv = CSCHED2_PRIV(new_ops);
+    struct csched2_vcpu *svc = vdata;
+    spinlock_t *old_lock;
+    unsigned rqi;
+
+    ASSERT(!pdata && svc && is_idle_vcpu(svc->vcpu));
+
+    spin_lock_irq(&prv->lock);
+    /*
+     * We may be acquiring the lock of another scheduler here (the one cpu
+     * still belongs to when calling this function). That is ok as, anyone
+     * trying to schedule on this cpu will block until when we release that
+     * lock (bottom of this function). When unblocked --because of the loop
+     * implemented by schedule_lock() functions-- he will notice the lock
+     * changed, and acquire ours before being able to proceed.
+     */
+    old_lock = pcpu_schedule_lock(cpu);
+
+    idle_vcpu[cpu]->sched_priv = vdata;
+
+    rqi = init_pdata(prv, cpu);
+
+    per_cpu(scheduler, cpu) = new_ops;
+    per_cpu(schedule_data, cpu).sched_priv = NULL; /* no pdata */
+    /* (Re?)route the lock to the per pCPU lock. */
+    per_cpu(schedule_data, cpu).schedule_lock = &prv->rqd[rqi].lock;
+
+    /* _Not_ pcpu_schedule_unlock(): schedule_lock may have changed! */
+    spin_unlock(old_lock);
+    spin_unlock_irq(&prv->lock);
+}
+
 static void
 csched2_free_pdata(const struct scheduler *ops, void *pcpu, int cpu)
 {
@@ -2167,6 +2205,7 @@ static const struct scheduler sched_credit2_def = {
     .free_vdata     = csched2_free_vdata,
     .init_pdata     = csched2_init_pdata,
     .free_pdata     = csched2_free_pdata,
+    .switch_sched   = csched2_switch_sched,
     .alloc_domdata  = csched2_alloc_domdata,
     .free_domdata   = csched2_free_domdata,
 };


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel