diff mbox

xen: credit2: non Credit2 pCPUs are ok during shutdown/suspend.

Message ID 148556774217.15340.9983189953428043418.stgit@Solace.fritz.box (mailing list archive)
State New, archived
Headers show

Commit Message

Dario Faggioli Jan. 28, 2017, 1:42 a.m. UTC
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@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Anshul Makkar <anshul.makkar@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
---
If 7478ebe1602e is backported, this one should be as well.

Sorry for this. I'm sure I tested, so I don't know how it could happened... I
must have forgotten debug=n when testing this case. :-(
---
 xen/common/sched_credit2.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

George Dunlap Feb. 1, 2017, 2:44 p.m. UTC | #1
On 28/01/17 01:42, Dario Faggioli wrote:
> 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@citrix.com>
> ---
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Anshul Makkar <anshul.makkar@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
> If 7478ebe1602e is backported, this one should be as well.
> 
> Sorry for this. I'm sure I tested, so I don't know how it could happened... I
> must have forgotten debug=n when testing this case. :-(

Indeed, and sorry I missed this on review as well.

Reviewed-by: George Dunlap <george.dunlap@citrix.com>
diff mbox

Patch

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index b2f2b17..85f36bc 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1953,10 +1953,6 @@  csched2_vcpu_migrate(
     struct csched2_runqueue_data *trqd;
     s_time_t now = NOW();
 
-    /* Check if new_cpu is valid */
-    ASSERT(cpumask_test_cpu(new_cpu, &CSCHED2_PRIV(ops)->initialized));
-    ASSERT(cpumask_test_cpu(new_cpu, vc->cpu_hard_affinity));
-
     /*
      * 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
@@ -1985,6 +1981,10 @@  csched2_vcpu_migrate(
         return;
     }
 
+    /* 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));
+
     trqd = RQD(ops, new_cpu);
 
     /*