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

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

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

 schedule_cpu_switch(x, csched2 --> csched):
   //scheduler[x] is csched2
   //schedule_lock[x] is csched2_lock
   csched_alloc_pdata(x)
   csched_init_pdata(x)
   pcpu_schedule_lock(x) ----> takes csched2_lock
   scheduler[X] = csched
   pcpu_schedule_unlock(x) --> unlocks csched2_lock
   [*]
   csched2_free_pdata(x)
     pcpu_schedule_lock(x) --> takes csched2_lock
     schedule_lock[x] = csched_lock
     spin_unlock(csched2_lock)

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

The problem arises because there is a window where the
scheduler is already the new one, but the lock is still the
old one. And, in this specific transition this is due to the
fact that it is only csched2_free_pdata() that (re)sets the
lock in the way that Credit1 needs it, instead of Credit1
itself doing it on its own.

This patch, therefore, introduce a new hook in the scheduler
interface, called switch_sched, meant at being used when
switching scheduler on a CPU, and implements it for Credit1.

This allows to do the various operations in the correct order
and under the protection of the best suited (set of) lock(s).

It is necessary to add the hook (as compare to keep doing
things in generic code), because different schedulers may
have different locking scheme. See, for instance, the different
lock nesting rules in Credit1 and Credit2.

Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
---
Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
---
 xen/common/sched_credit.c  |   38 ++++++++++++++++++++++++++++++++++++++
 xen/include/xen/sched-if.h |    3 +++
 2 files changed, 41 insertions(+)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 4488d7c..929ba9c 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -583,6 +583,43 @@ csched_init_pdata(const struct scheduler *ops, void 
*pdata, int cpu)
     spin_unlock_irqrestore(&prv->lock, flags);
 }
 
+/* Change the scheduler of cpu to us (Credit). */
+static void
+csched_switch_sched(struct scheduler *ops, unsigned int cpu,
+                    void *pdata, void *vdata)
+{
+    struct schedule_data *sd = &per_cpu(schedule_data, cpu);
+    struct csched_private *prv = CSCHED_PRIV(ops);
+    struct csched_vcpu *svc = vdata;
+    spinlock_t * old_lock;
+
+    ASSERT(svc && is_idle_vcpu(svc->vcpu));
+
+    /*
+     * 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_irq(cpu);
+
+    idle_vcpu[cpu]->sched_priv = vdata;
+
+    spin_lock(&prv->lock);
+    init_pdata(prv, pdata, cpu);
+    spin_unlock(&prv->lock);
+
+    per_cpu(scheduler, cpu) = ops;
+    per_cpu(schedule_data, cpu).sched_priv = pdata;
+    /* (Re?)route the lock to the per pCPU lock. */
+    sd->schedule_lock = &sd->_lock;
+
+    /* _Not_ pcpu_schedule_unlock(): schedule_lock may have changed! */
+    spin_unlock_irq(old_lock);
+}
+
 #ifndef NDEBUG
 static inline void
 __csched_vcpu_check(struct vcpu *vc)
@@ -2064,6 +2101,7 @@ static const struct scheduler sched_credit_def = {
     .alloc_pdata    = csched_alloc_pdata,
     .init_pdata     = csched_init_pdata,
     .free_pdata     = csched_free_pdata,
+    .switch_sched   = csched_switch_sched,
     .alloc_domdata  = csched_alloc_domdata,
     .free_domdata   = csched_free_domdata,
 
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 560cba5..0aa00de 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -138,6 +138,9 @@ struct scheduler {
     void         (*free_domdata)   (const struct scheduler *, void *);
     void *       (*alloc_domdata)  (const struct scheduler *, struct domain *);
 
+    void         (*switch_sched)   (struct scheduler *, unsigned int,
+                                    void *, void *);
+
     int          (*init_domain)    (const struct scheduler *, struct domain *);
     void         (*destroy_domain) (const struct scheduler *, struct domain *);
 


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