[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 4/9] xen: sched: add .init_pdata hook to the scheduler interface



On 09/29/2015 06:55 PM, Dario Faggioli wrote:
with the purpose of decoupling the allocation phase and
the initialization one, for per-pCPU data of the schedulers.

This makes it possible to perform the initialization later
in the pCPU bringup/assignement process, when more information
(for instance, the host CPU topology) are available. This,
for now, is important only for Credit2, but it can well be
useful to other schedulers.

This also fixes a latent bug. In fact, when moving a pCPU
from a Credit2 cpupool to another, whose scheduler also
remaps runqueue spin locks (e.g., RTDS), we experience the
following Oops:

(XEN) Initializing RTDS scheduler
(XEN) WARNING: This is experimental software in development.
(XEN) Use at your own risk.
(XEN) Removing cpu 6 from runqueue 0
(XEN) Assertion 'sd->schedule_lock == &rqd->lock' failed at sched_credit2.c:2102
(XEN) ----[ Xen-4.7-unstable  x86_64  debug=y  Not tainted ]----
... ... ...
(XEN) Xen call trace:
(XEN)    [<ffff82d0801256fa>] csched2_free_pdata+0x135/0x180
(XEN)    [<ffff82d08012ccad>] schedule_cpu_switch+0x1ee/0x217
(XEN)    [<ffff82d080101c34>] cpupool_assign_cpu_locked+0x4d/0x152
(XEN)    [<ffff82d0801024ad>] cpupool_do_sysctl+0x20b/0x69d
(XEN)    [<ffff82d08012f693>] do_sysctl+0x665/0x10f8
(XEN)    [<ffff82d08023cb86>] lstar_enter+0x106/0x160

This happens because, right now, the scheduler of the
target pool remaps the runqueue lock during (rt_)alloc_pdata,
which is called at the very beginning of schedule_cpu_switch().
Credit2's csched2_free_pdata(), however, wants to find the spin
lock the same way it was put by csched2_alloc_pdata(), and
explodes, it not being the case.

This patch also fixes this as, now, spin lock remapping
happens in the init_pdata hook of the target scheduler for
the pCPU, which we can easily make sure it is ivoked *after*
the free_pdata hook of the old scheduler (which is exactly
what is done in this patch), without needing to restructure
schedule_cpu_switch().

Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
---
Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
Cc: Juergen Gross <jgross@xxxxxxxx>
---
  xen/common/schedule.c      |   16 +++++++++++++---
  xen/include/xen/sched-if.h |    1 +
  2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 4a89222..83244d7 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1407,6 +1407,9 @@ static int cpu_schedule_callback(

      switch ( action )
      {
+    case CPU_STARTING:
+        SCHED_OP(&ops, init_pdata, cpu);
+        break;
      case CPU_UP_PREPARE:
          rc = cpu_schedule_up(cpu);
          break;
@@ -1484,6 +1487,7 @@ void __init scheduler_init(void)
      if ( ops.alloc_pdata &&
           !(this_cpu(schedule_data).sched_priv = ops.alloc_pdata(&ops, 0)) )
          BUG();
+    SCHED_OP(&ops, init_pdata, 0);

You can't call this without having it set in all schedulers.

I guess using the init_pdata hook has to be introduced after or in
patch 5.


Juergen

  }

  int schedule_cpu_switch(unsigned int cpu, struct cpupool *c)
@@ -1513,12 +1517,18 @@ int schedule_cpu_switch(unsigned int cpu, struct 
cpupool *c)
      per_cpu(scheduler, cpu) = new_ops;
      ppriv_old = per_cpu(schedule_data, cpu).sched_priv;
      per_cpu(schedule_data, cpu).sched_priv = ppriv;
-    SCHED_OP(new_ops, tick_resume, cpu);
-    SCHED_OP(new_ops, insert_vcpu, idle);
-
      SCHED_OP(old_ops, free_vdata, vpriv_old);
      SCHED_OP(old_ops, free_pdata, ppriv_old, cpu);

+    /*
+     * Now that the new pcpu data have been allocated and all pointers
+     * correctly redirected to them, we can perform all the necessary
+     * initializations.
+     */
+    SCHED_OP(new_ops, init_pdata, cpu);
+    SCHED_OP(new_ops, tick_resume, cpu);
+    SCHED_OP(new_ops, insert_vcpu, idle);
+
      return 0;
  }

diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index dbe7cab..14392f3 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -133,6 +133,7 @@ struct scheduler {
                                      void *);
      void         (*free_pdata)     (const struct scheduler *, void *, int);
      void *       (*alloc_pdata)    (const struct scheduler *, int);
+    void         (*init_pdata)     (const struct scheduler *, int);
      void         (*free_domdata)   (const struct scheduler *, void *);
      void *       (*alloc_domdata)  (const struct scheduler *, struct domain 
*);



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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.