On 12.06.19 10:05, Andrew Cooper wrote:
On 28/05/2019 11:32, Juergen Gross wrote:
@@ -1870,8 +1871,19 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool
*c)
old_lock = pcpu_schedule_lock_irq(cpu);
vpriv_old = idle->sched_priv;
- ppriv_old = per_cpu(schedule_data, cpu).sched_priv;
- sched_switch_sched(new_ops, cpu, ppriv, vpriv);
+ ppriv_old = sd->sched_priv;
+ new_lock = sched_switch_sched(new_ops, cpu, ppriv, vpriv);
+
+ per_cpu(scheduler, cpu) = new_ops;
+ sd->sched_priv = ppriv;
+
+ /*
+ * (Re?)route the lock to the per pCPU lock as /last/ thing. In fact,
+ * if it is free (and it can be) we want that anyone that manages
+ * taking it, finds all the initializations we've done above in place.
+ */
+ smp_mb();
I realise you're just moving existing code, but this barrier sticks out
like a sore thumb.
A full memory barrier is a massive overhead for what should be
smp_wmb(). The matching barrier is actually hidden in the implicit
semantics of managing to lock sd->schedule_lock (which is trial an error
anyway), but the only thing that matters here is that all other written
data is in place first.
Beyond that, local causality will cause all reads to be in order (not
that the are important) due to logic dependencies. Any that miss out on
this are a optimisation-waiting-to-happen as the compiler could elide
them fully.
Not that it would really matter for performance (switching cpus between
cpupools is a _very_ rare operation), I'm fine transforming the barrier
into smp_wmb().
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
