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

[Xen-changelog] [xen stable-4.8] xen: credit2: fix shutdown/suspend when playing with cpupools.



commit 4582c2b9597ff4b5be3f6b26449a3b8a0872e46e
Author:     Dario Faggioli <dario.faggioli@xxxxxxxxxx>
AuthorDate: Thu Feb 9 10:25:33 2017 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Feb 9 10:25:33 2017 +0100

    xen: credit2: fix shutdown/suspend when playing with cpupools.
    
    In fact, during shutdown/suspend, we temporarily move all
    the vCPUs to the BSP (i.e., pCPU 0, as of now). For Credit2
    domains, we call csched2_vcpu_migrate(), expects to find the
    target pCPU in the domain's pool
    
    Therefore, if Credit2 is the default scheduler and we have
    removed pCPU 0 from cpupool0, shutdown/suspend fails like
    this:
    
     RIP:    e008:[<ffff82d08012906d>] sched_credit2.c#migrate+0x274/0x2d1
     Xen call trace:
        [<ffff82d08012906d>] sched_credit2.c#migrate+0x274/0x2d1
        [<ffff82d080129138>] sched_credit2.c#csched2_vcpu_migrate+0x6e/0x86
        [<ffff82d08012c468>] schedule.c#vcpu_move_locked+0x69/0x6f
        [<ffff82d08012ec14>] cpu_disable_scheduler+0x3d7/0x430
        [<ffff82d08019669b>] __cpu_disable+0x299/0x2b0
        [<ffff82d0801012f8>] cpu.c#take_cpu_down+0x2f/0x38
        [<ffff82d0801312d8>] stop_machine.c#stopmachine_action+0x7f/0x8d
        [<ffff82d0801330b8>] tasklet.c#do_tasklet_work+0x74/0xab
        [<ffff82d0801333ed>] do_tasklet+0x66/0x8b
        [<ffff82d080166a73>] domain.c#idle_loop+0x3b/0x5e
    
     ****************************************
     Panic on CPU 8:
     Assertion 'svc->vcpu->processor < nr_cpu_ids' failed at 
sched_credit2.c:1729
     ****************************************
    
    On the other hand, if Credit2 is the scheduler of another
    pool, when trying (still during shutdown/suspend) to move
    the vCPUs of the Credit2 domains to pCPU 0, it figures
    out that pCPU 0 is not a Credit2 pCPU, and fails like this:
    
     RIP:    e008:[<ffff82d08012916b>] 
sched_credit2.c#csched2_vcpu_migrate+0xa1/0x107
     Xen call trace:
        [<ffff82d08012916b>] sched_credit2.c#csched2_vcpu_migrate+0xa1/0x107
        [<ffff82d08012c4e9>] schedule.c#vcpu_move_locked+0x69/0x6f
        [<ffff82d08012edfc>] cpu_disable_scheduler+0x3d7/0x430
        [<ffff82d08019687b>] __cpu_disable+0x299/0x2b0
        [<ffff82d0801012f8>] cpu.c#take_cpu_down+0x2f/0x38
        [<ffff82d0801314c0>] stop_machine.c#stopmachine_action+0x7f/0x8d
        [<ffff82d0801332a0>] tasklet.c#do_tasklet_work+0x74/0xab
        [<ffff82d0801335d5>] do_tasklet+0x66/0x8b
        [<ffff82d080166c53>] domain.c#idle_loop+0x3b/0x5e
    
    The solution is to recognise the specific situation, inside
    csched2_vcpu_migrate() and, considering it is something temporary,
    which only happens during shutdown/suspend, quickly deal with it.
    
    Then, in the resume path, in restore_vcpu_affinity(), things
    are set back to normal, and a new v->processor is chosen, for
    each vCPU, from the proper set of pCPUs (i.e., the ones of
    the proper cpupool).
    
    Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
    Acked-by: George Dunlap <george.dunlap@xxxxxxxxxx>
    
    xen: credit2: non Credit2 pCPUs are ok during shutdown/suspend.
    
    Commit 7478ebe1602e6 ("xen: credit2: fix shutdown/suspend
    when playing with cpupools"), while doing the right thing
    for actual code, forgot to update the ASSERT()s accordingly,
    in csched2_vcpu_migrate().
    
    In fact, as stated there already, during shutdown/suspend,
    we must allow a Credit2 vCPU to temporarily migrate to a
    non Credit2 BSP, without any ASSERT() triggering.
    
    Move them down, after the check for whether or not we are
    shutting down, where the assumption that the pCPU must be
    valid Credit2 ones, is valid.
    
    Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
    master commit: 7478ebe1602e6bb8242a18840b15757a1d5ad18a
    master date: 2017-01-24 17:02:07 +0000
    master commit: ad5808d9057248e7879cf375662f0a449fff7005
    master date: 2017-02-01 14:44:51 +0000
---
 xen/common/sched_credit2.c | 34 ++++++++++++++++++++++++++++++++--
 xen/common/schedule.c      | 25 ++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index ce0e146..c669c38 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1946,10 +1946,40 @@ static void
 csched2_vcpu_migrate(
     const struct scheduler *ops, struct vcpu *vc, unsigned int new_cpu)
 {
+    struct domain *d = vc->domain;
     struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
     struct csched2_runqueue_data *trqd;
+    s_time_t now = NOW();
+
+    /*
+     * Being passed a target pCPU which is outside of our cpupool is only
+     * valid if we are shutting down (or doing ACPI suspend), and we are
+     * moving everyone to BSP, no matter whether or not BSP is inside our
+     * cpupool.
+     *
+     * And since there indeed is the chance that it is not part of it, all
+     * we must do is remove _and_ unassign the vCPU from any runqueue, as
+     * well as updating v->processor with the target, so that the suspend
+     * process can continue.
+     *
+     * It will then be during resume that a new, meaningful, value for
+     * v->processor will be chosen, and during actual domain unpause that
+     * the vCPU will be assigned to and added to the proper runqueue.
+     */
+    if ( unlikely(!cpumask_test_cpu(new_cpu, cpupool_domain_cpumask(d))) )
+    {
+        ASSERT(system_state == SYS_STATE_suspend);
+        if ( __vcpu_on_runq(svc) )
+        {
+            __runq_remove(svc);
+            update_load(ops, svc->rqd, NULL, -1, now);
+        }
+        __runq_deassign(svc);
+        vc->processor = new_cpu;
+        return;
+    }
 
-    /* Check if new_cpu is valid */
+    /* If here, new_cpu must be a valid Credit2 pCPU, and in our affinity. */
     ASSERT(cpumask_test_cpu(new_cpu, &CSCHED2_PRIV(ops)->initialized));
     ASSERT(cpumask_test_cpu(new_cpu, vc->cpu_hard_affinity));
 
@@ -1964,7 +1994,7 @@ csched2_vcpu_migrate(
      * pointing to a pcpu where we can't run any longer.
      */
     if ( trqd != svc->rqd )
-        migrate(ops, svc, trqd, NOW());
+        migrate(ops, svc, trqd, now);
     else
         vc->processor = new_cpu;
 }
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 5b444c4..36ff2e9 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -633,8 +633,11 @@ void vcpu_force_reschedule(struct vcpu *v)
 
 void restore_vcpu_affinity(struct domain *d)
 {
+    unsigned int cpu = smp_processor_id();
     struct vcpu *v;
 
+    ASSERT(system_state == SYS_STATE_resume);
+
     for_each_vcpu ( d, v )
     {
         spinlock_t *lock = vcpu_schedule_lock_irq(v);
@@ -643,18 +646,34 @@ void restore_vcpu_affinity(struct domain *d)
         {
             cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved);
             v->affinity_broken = 0;
+
         }
 
-        if ( v->processor == smp_processor_id() )
+        /*
+         * During suspend (in cpu_disable_scheduler()), we moved every vCPU
+         * to BSP (which, as of now, is pCPU 0), as a temporary measure to
+         * allow the nonboot processors to have their data structure freed
+         * and go to sleep. But nothing guardantees that the BSP is a valid
+         * pCPU for a particular domain.
+         *
+         * Therefore, here, before actually unpausing the domains, we should
+         * set v->processor of each of their vCPUs to something that will
+         * make sense for the scheduler of the cpupool in which they are in.
+         */
+        cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity,
+                    cpupool_domain_cpumask(v->domain));
+        v->processor = cpumask_any(cpumask_scratch_cpu(cpu));
+
+        if ( v->processor == cpu )
         {
             set_bit(_VPF_migrating, &v->pause_flags);
-            vcpu_schedule_unlock_irq(lock, v);
+            spin_unlock_irq(lock);;
             vcpu_sleep_nosync(v);
             vcpu_migrate(v);
         }
         else
         {
-            vcpu_schedule_unlock_irq(lock, v);
+            spin_unlock_irq(lock);
         }
     }
 
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.8

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog

 


Rackspace

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