@@ -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,
@@ -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 *);
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@citrix.com> --- Cc: George Dunlap <george.dunlap@eu.citrix.com> --- xen/common/sched_credit.c | 38 ++++++++++++++++++++++++++++++++++++++ xen/include/xen/sched-if.h | 3 +++ 2 files changed, 41 insertions(+)