[PATCH 0 of 2] ACPI state change corrections

[PATCH 0 of 2] ACPI state change corrections

[Juergen Gross]

Doing an ACPI state change with not all cpus in cpupool 0 will currently
crash the hypervisor.

Patch 1 relaxes cpupool cpu offlining handling to allow offlining a cpu in
        any cpupool if no lack of ressources is detected
Patch 2 prohibits migrating a vcpu to another cpu if it is paused

6 files changed, 144 insertions(+), 33 deletions(-)
xen/arch/x86/smpboot.c     |    2 -
xen/common/cpupool.c       |   85 +++++++++++++++++++++++++++++++++++++++-----
xen/common/domain.c        |    4 +-
xen/common/schedule.c      |   83 +++++++++++++++++++++++++++++++-----------
xen/include/xen/sched-if.h |    1 
xen/include/xen/sched.h    |    2 +

[PATCH 1 of 2] Allow ACPI state change with active cpupools

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

Changing the ACPI state (e.g. power off) while not all cpus are in cpupool 0
will currently crash the hypervisor during disabling the other cpus.
Up to now only cpus in Pool-0 were allowed to go offline. There is no reason
why a cpu in another cpupool can't be taken offline if there are still other
cpus available in that cpupool.
As disabling the cpus for an ACPI state change is only a temporary measure
(if not poweroff, of course) and all domains are paused in this case, there
is no reason to reject removing the last cpu from a cpupool if only paused
domains are in this cpupool.
The cpus taken offline from a cpupool will be still associated with that
cpupool to enable adding them automatically when they are becoming online
again.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>


4 files changed, 80 insertions(+), 9 deletions(-)
xen/arch/x86/smpboot.c     |    2 -
xen/common/cpupool.c       |   85 +++++++++++++++++++++++++++++++++++++++-----
xen/include/xen/sched-if.h |    1 
xen/include/xen/sched.h    |    1 



[-- Attachment #2: xen-staging.hg-2.patch --]
[-- Type: text/x-patch, Size: 6405 bytes --]

# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1332404504 -3600
# Node ID 12d8bb0e02edf68b4a0526a2989a660218535fcb
# Parent  4e1d091d10d83130842170cd61f1194e5459f2aa
Allow ACPI state change with active cpupools

Changing the ACPI state (e.g. power off) while not all cpus are in cpupool 0
will currently crash the hypervisor during disabling the other cpus.
Up to now only cpus in Pool-0 were allowed to go offline. There is no reason
why a cpu in another cpupool can't be taken offline if there are still other
cpus available in that cpupool.
As disabling the cpus for an ACPI state change is only a temporary measure
(if not poweroff, of course) and all domains are paused in this case, there
is no reason to reject removing the last cpu from a cpupool if only paused
domains are in this cpupool.
The cpus taken offline from a cpupool will be still associated with that
cpupool to enable adding them automatically when they are becoming online
again.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

diff -r 4e1d091d10d8 -r 12d8bb0e02ed xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c	Fri Mar 16 15:24:25 2012 +0000
+++ b/xen/arch/x86/smpboot.c	Thu Mar 22 09:21:44 2012 +0100
@@ -863,7 +863,7 @@ void __cpu_disable(void)
     remove_siblinginfo(cpu);
 
     /* It's now safe to remove this processor from the online map */
-    cpumask_clear_cpu(cpu, cpupool0->cpu_valid);
+    cpupool_disable_cpu(cpu);
     cpumask_clear_cpu(cpu, &cpu_online_map);
     fixup_irqs();
 
diff -r 4e1d091d10d8 -r 12d8bb0e02ed xen/common/cpupool.c
--- a/xen/common/cpupool.c	Fri Mar 16 15:24:25 2012 +0000
+++ b/xen/common/cpupool.c	Thu Mar 22 09:21:44 2012 +0100
@@ -24,6 +24,7 @@
 
 struct cpupool *cpupool0;                /* Initial cpupool with Dom0 */
 cpumask_t cpupool_free_cpus;             /* cpus not in any cpupool */
+static cpumask_t cpupool_free_offline_cpus;
 
 static struct cpupool *cpupool_list;     /* linked list, sorted by poolid */
 
@@ -41,16 +42,25 @@ static struct cpupool *alloc_cpupool_str
 {
     struct cpupool *c = xzalloc(struct cpupool);
 
-    if ( c && zalloc_cpumask_var(&c->cpu_valid) )
+    if ( !c )
+        return NULL;
+    if ( !zalloc_cpumask_var(&c->cpu_valid) )
+        goto fail;
+    if ( zalloc_cpumask_var(&c->cpu_offline) )
         return c;
+
+    free_cpumask_var(c->cpu_valid);
+fail:
     xfree(c);
     return NULL;
 }
 
 static void free_cpupool_struct(struct cpupool *c)
 {
-    if ( c )
-        free_cpumask_var(c->cpu_valid);
+    if ( !c )
+        return;
+    free_cpumask_var(c->cpu_valid);
+    free_cpumask_var(c->cpu_offline);
     xfree(c);
 }
 
@@ -413,30 +423,67 @@ void cpupool_rm_domain(struct domain *d)
 
 /*
  * called to add a new cpu to pool admin
+ * a temporarily disabled cpu will be added to its former pool
  * we add a hotplugged cpu to the cpupool0 to be able to add it to dom0
  */
 static void cpupool_cpu_add(unsigned int cpu)
 {
+    struct cpupool **c;
+
     spin_lock(&cpupool_lock);
     cpumask_clear_cpu(cpu, &cpupool_locked_cpus);
     cpumask_set_cpu(cpu, &cpupool_free_cpus);
+    if ( cpumask_test_cpu(cpu, &cpupool_free_offline_cpus) )
+    {
+        cpumask_clear_cpu(cpu, &cpupool_free_offline_cpus);
+        goto out;
+    }
+    for_each_cpupool(c)
+    {
+        if ( cpumask_test_cpu(cpu, (*c)->cpu_offline) )
+        {
+            cpumask_clear_cpu(cpu, (*c)->cpu_offline);
+            cpupool_assign_cpu_locked(*c, cpu);
+            goto out;
+        }
+    }
     cpupool_assign_cpu_locked(cpupool0, cpu);
+out:
     spin_unlock(&cpupool_lock);
 }
 
 /*
  * called to remove a cpu from pool admin
- * the cpu to be removed is locked to avoid removing it from dom0
+ * the cpu to be removed is locked to avoid removing it from any cpupool
+ * (the real remove at end of taking a cpu down must not take the cpupool
+ * lock)
  * returns failure if not in pool0
  */
 static int cpupool_cpu_remove(unsigned int cpu)
 {
     int ret = 0;
-	
+    struct cpupool *c;
+    struct domain *d;
+
     spin_lock(&cpupool_lock);
-    if ( !cpumask_test_cpu(cpu, cpupool0->cpu_valid))
-        ret = -EBUSY;
-    else
+    c = per_cpu(cpupool, cpu);
+    if ( c == NULL )
+        goto out;
+    if ( cpumask_test_cpu(cpu, c->cpu_valid) )
+    {
+        if ( cpumask_weight(c->cpu_valid) > 1 )
+            goto out;
+        rcu_read_lock(&domlist_read_lock);
+        for_each_domain_in_cpupool(d, c)
+        {
+            if ( !d->is_dying && !atomic_read(&d->pause_count) )
+                ret = -EBUSY;
+        }
+        rcu_read_unlock(&domlist_read_lock);
+    }
+
+out:
+    if ( !ret )
         cpumask_set_cpu(cpu, &cpupool_locked_cpus);
     spin_unlock(&cpupool_lock);
 
@@ -597,6 +644,28 @@ int cpupool_do_sysctl(struct xen_sysctl_
     return ret;
 }
 
+/*
+ * remove a cpu from configuration
+ * called at end of cpu offlining
+ * we must not take the cpupool lock
+ */
+void cpupool_disable_cpu(unsigned int cpu)
+{
+    struct cpupool *c;
+
+    c = per_cpu(cpupool, cpu);
+    if ( c == NULL )
+    {
+        cpumask_clear_cpu(cpu, &cpupool_free_cpus);
+        cpumask_set_cpu(cpu, &cpupool_free_offline_cpus);
+    }
+    else if ( cpumask_test_cpu(cpu, c->cpu_valid) )
+    {
+        cpumask_clear_cpu(cpu, c->cpu_valid);
+        cpumask_set_cpu(cpu, c->cpu_offline);
+    }
+}
+
 void dump_runq(unsigned char key)
 {
     unsigned long    flags;
diff -r 4e1d091d10d8 -r 12d8bb0e02ed xen/include/xen/sched-if.h
--- a/xen/include/xen/sched-if.h	Fri Mar 16 15:24:25 2012 +0000
+++ b/xen/include/xen/sched-if.h	Thu Mar 22 09:21:44 2012 +0100
@@ -199,6 +199,7 @@ struct cpupool
 {
     int              cpupool_id;
     cpumask_var_t    cpu_valid;      /* all cpus assigned to pool */
+    cpumask_var_t    cpu_offline;    /* cpus from pool taken offline */
     struct cpupool   *next;
     unsigned int     n_dom;
     struct scheduler *sched;
diff -r 4e1d091d10d8 -r 12d8bb0e02ed xen/include/xen/sched.h
--- a/xen/include/xen/sched.h	Fri Mar 16 15:24:25 2012 +0000
+++ b/xen/include/xen/sched.h	Thu Mar 22 09:21:44 2012 +0100
@@ -715,6 +715,7 @@ int cpupool_do_sysctl(struct xen_sysctl_
 int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op);
 void schedule_dump(struct cpupool *c);
 extern void dump_runq(unsigned char key);
+void cpupool_disable_cpu(unsigned int cpu);
 
 #define num_cpupool_cpus(c) cpumask_weight((c)->cpu_valid)
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

[PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Currently offlining a cpu will migrate all vcpus which were active on that
cpu to another one, possibly breaking existing cpu affinities.

In case of an ACPI state change the cpus are taken offline and online later
(if not poweroff) while all domains are paused. There is no reason to
migrate the vcpus during offlining the cpus, as the cpus will be available
again when the domains are being unpaused.

This patch defers the migration check in case of paused vcpus or domains
by adding vcpu_arouse() to wake up a vcpu and check whether it must be
migrated to another cpu.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>


3 files changed, 64 insertions(+), 24 deletions(-)
xen/common/domain.c     |    4 +-
xen/common/schedule.c   |   83 ++++++++++++++++++++++++++++++++++-------------
xen/include/xen/sched.h |    1 



[-- Attachment #2: xen-staging.hg-2.patch --]
[-- Type: text/x-patch, Size: 5733 bytes --]

# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1332404507 -3600
# Node ID 932dc3987e3dac816d51e49861ab2ff21e8228b5
# Parent  12d8bb0e02edf68b4a0526a2989a660218535fcb
Avoid vcpu migration of paused vcpus

Currently offlining a cpu will migrate all vcpus which were active on that
cpu to another one, possibly breaking existing cpu affinities.

In case of an ACPI state change the cpus are taken offline and online later
(if not poweroff) while all domains are paused. There is no reason to
migrate the vcpus during offlining the cpus, as the cpus will be available
again when the domains are being unpaused.

This patch defers the migration check in case of paused vcpus or domains
by adding vcpu_arouse() to wake up a vcpu and check whether it must be
migrated to another cpu.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

diff -r 12d8bb0e02ed -r 932dc3987e3d xen/common/domain.c
--- a/xen/common/domain.c	Thu Mar 22 09:21:44 2012 +0100
+++ b/xen/common/domain.c	Thu Mar 22 09:21:47 2012 +0100
@@ -734,7 +734,7 @@ void vcpu_unpause(struct vcpu *v)
 void vcpu_unpause(struct vcpu *v)
 {
     if ( atomic_dec_and_test(&v->pause_count) )
-        vcpu_wake(v);
+        vcpu_arouse(v);
 }
 
 void domain_pause(struct domain *d)
@@ -755,7 +755,7 @@ void domain_unpause(struct domain *d)
 
     if ( atomic_dec_and_test(&d->pause_count) )
         for_each_vcpu( d, v )
-            vcpu_wake(v);
+            vcpu_arouse(v);
 }
 
 void domain_pause_by_systemcontroller(struct domain *d)
diff -r 12d8bb0e02ed -r 932dc3987e3d xen/common/schedule.c
--- a/xen/common/schedule.c	Thu Mar 22 09:21:44 2012 +0100
+++ b/xen/common/schedule.c	Thu Mar 22 09:21:47 2012 +0100
@@ -525,6 +525,47 @@ void vcpu_force_reschedule(struct vcpu *
     }
 }
 
+static int vcpu_chk_affinity(struct vcpu *v, unsigned int cpu)
+{
+    cpumask_t online_affinity;
+    struct cpupool *c;
+
+    c = v->domain->cpupool;
+    cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
+    if ( cpumask_empty(&online_affinity) &&
+         cpumask_test_cpu(cpu, v->cpu_affinity) )
+    {
+        printk("Breaking vcpu affinity for domain %d vcpu %d\n",
+                v->domain->domain_id, v->vcpu_id);
+        cpumask_setall(v->cpu_affinity);
+    }
+
+    return ( !cpumask_test_cpu(cpu, c->cpu_valid) && (v->processor == cpu) );
+}
+
+void vcpu_arouse(struct vcpu *v)
+{
+    unsigned long flags;
+
+    if ( atomic_read(&v->pause_count) ||
+         atomic_read(&v->domain->pause_count) )
+        return;
+
+    vcpu_schedule_lock_irqsave(v, flags);
+
+    if ( unlikely(vcpu_chk_affinity(v, v->processor) && (v != current)) )
+    {
+        set_bit(_VPF_migrating, &v->pause_flags);
+        vcpu_schedule_unlock_irqrestore(v, flags);
+        vcpu_migrate(v);
+        return;
+    }
+
+    vcpu_schedule_unlock_irqrestore(v, flags);
+
+    vcpu_wake(v);
+}
+
 /*
  * This function is used by cpu_hotplug code from stop_machine context
  * and from cpupools to switch schedulers on a cpu.
@@ -534,7 +575,6 @@ int cpu_disable_scheduler(unsigned int c
     struct domain *d;
     struct vcpu *v;
     struct cpupool *c;
-    cpumask_t online_affinity;
     int    ret = 0;
 
     c = per_cpu(cpupool, cpu);
@@ -547,34 +587,33 @@ int cpu_disable_scheduler(unsigned int c
         {
             vcpu_schedule_lock_irq(v);
 
-            cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
-            if ( cpumask_empty(&online_affinity) &&
-                 cpumask_test_cpu(cpu, v->cpu_affinity) )
+            if ( likely(!atomic_read(&v->pause_count) &&
+                        !atomic_read(&d->pause_count)) )
             {
-                printk("Breaking vcpu affinity for domain %d vcpu %d\n",
-                        v->domain->domain_id, v->vcpu_id);
-                cpumask_setall(v->cpu_affinity);
-            }
+                if ( vcpu_chk_affinity(v, cpu) )
+                {
+                    set_bit(_VPF_migrating, &v->pause_flags);
+                    vcpu_schedule_unlock_irq(v);
+                    vcpu_sleep_nosync(v);
+                    vcpu_migrate(v);
+                }
+                else
+                {
+                    vcpu_schedule_unlock_irq(v);
+                }
 
-            if ( v->processor == cpu )
-            {
-                set_bit(_VPF_migrating, &v->pause_flags);
-                vcpu_schedule_unlock_irq(v);
-                vcpu_sleep_nosync(v);
-                vcpu_migrate(v);
+                /*
+                 * A vcpu active in the hypervisor will not be migratable.
+                 * The caller should try again after releasing and reaquiring
+                 * all locks.
+                 */
+                if ( v->processor == cpu )
+                    ret = -EAGAIN;
             }
             else
             {
                 vcpu_schedule_unlock_irq(v);
             }
-
-            /*
-             * A vcpu active in the hypervisor will not be migratable.
-             * The caller should try again after releasing and reaquiring
-             * all locks.
-             */
-            if ( v->processor == cpu )
-                ret = -EAGAIN;
         }
 
         domain_update_node_affinity(d);
diff -r 12d8bb0e02ed -r 932dc3987e3d xen/include/xen/sched.h
--- a/xen/include/xen/sched.h	Thu Mar 22 09:21:44 2012 +0100
+++ b/xen/include/xen/sched.h	Thu Mar 22 09:21:47 2012 +0100
@@ -523,6 +523,7 @@ void sched_tick_suspend(void);
 void sched_tick_suspend(void);
 void sched_tick_resume(void);
 void vcpu_wake(struct vcpu *d);
+void vcpu_arouse(struct vcpu *d);    /* like vcpu_wake, check migrate */
 void vcpu_sleep_nosync(struct vcpu *d);
 void vcpu_sleep_sync(struct vcpu *d);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1 of 2] Allow ACPI state change with active cpupools

[Jan Beulich]

>>> On 22.03.12 at 09:22, Juergen Gross <juergen.gross@ts.fujitsu.com> wrote:
> static void cpupool_cpu_add(unsigned int cpu)
> {
>+    struct cpupool **c;
>+
>     spin_lock(&cpupool_lock);
>     cpumask_clear_cpu(cpu, &cpupool_locked_cpus);
>     cpumask_set_cpu(cpu, &cpupool_free_cpus);
>+    if ( cpumask_test_cpu(cpu, &cpupool_free_offline_cpus) )
>+    {
>+        cpumask_clear_cpu(cpu, &cpupool_free_offline_cpus);

cpumask_test_and_clear_cpu()

>+        goto out;
>+    }
>+    for_each_cpupool(c)
>+    {
>+        if ( cpumask_test_cpu(cpu, (*c)->cpu_offline) )
>+        {
>+            cpumask_clear_cpu(cpu, (*c)->cpu_offline);

Here too.

>+            cpupool_assign_cpu_locked(*c, cpu);
>+            goto out;
>+        }
>+    }
>     cpupool_assign_cpu_locked(cpupool0, cpu);
>...
>+void cpupool_disable_cpu(unsigned int cpu)
>+{
>+    struct cpupool *c;
>+
>+    c = per_cpu(cpupool, cpu);
>+    if ( c == NULL )
>+    {
>+        cpumask_clear_cpu(cpu, &cpupool_free_cpus);
>+        cpumask_set_cpu(cpu, &cpupool_free_offline_cpus);
>+    }
>+    else if ( cpumask_test_cpu(cpu, c->cpu_valid) )
>+    {
>+        cpumask_clear_cpu(cpu, c->cpu_valid);

And once more.

>+        cpumask_set_cpu(cpu, c->cpu_offline);
>+    }
>+}

Jan

Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Keir Fraser]

Your original patch didn't touch this code. Was that an omission in the
original version? On reflection I prefer your original patch to this new
approach. I'll apply it if you still believe your original patch is complete
and correct as it stands.

 -- Keir

On 22/03/2012 08:22, "Juergen Gross" <juergen.gross@ts.fujitsu.com> wrote:

> Currently offlining a cpu will migrate all vcpus which were active on that
> cpu to another one, possibly breaking existing cpu affinities.
> 
> In case of an ACPI state change the cpus are taken offline and online later
> (if not poweroff) while all domains are paused. There is no reason to
> migrate the vcpus during offlining the cpus, as the cpus will be available
> again when the domains are being unpaused.
> 
> This patch defers the migration check in case of paused vcpus or domains
> by adding vcpu_arouse() to wake up a vcpu and check whether it must be
> migrated to another cpu.
> 
> Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
> 
> 
> 3 files changed, 64 insertions(+), 24 deletions(-)
> xen/common/domain.c     |    4 +-
> xen/common/schedule.c   |   83 ++++++++++++++++++++++++++++++++++-------------
> xen/include/xen/sched.h |    1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Juergen Gross]

On 03/22/2012 11:12 AM, Keir Fraser wrote:
> Your original patch didn't touch this code. Was that an omission in the
> original version? On reflection I prefer your original patch to this new
> approach. I'll apply it if you still believe your original patch is complete
> and correct as it stands.

I like my second patch more :-)

It covers more cases, not just poweroff. In hibernate case no vcpu pinnings
will be lost. Today all vcpus pinned to a cpu other than 0 will lose their
pinnings at cpu offlining. At reactivation those pinnings will not be
restored automatically. My patch will cover that by checking availability
of the cpus after reactivation.

Poweroff (which was my primary concern) works with both versions. I did not
test other ACPI state changes with either version, but would expect better
results in hibernate case with my second approach.


Juergen

>   -- Keir
>
> On 22/03/2012 08:22, "Juergen Gross"<juergen.gross@ts.fujitsu.com>  wrote:
>
>> Currently offlining a cpu will migrate all vcpus which were active on that
>> cpu to another one, possibly breaking existing cpu affinities.
>>
>> In case of an ACPI state change the cpus are taken offline and online later
>> (if not poweroff) while all domains are paused. There is no reason to
>> migrate the vcpus during offlining the cpus, as the cpus will be available
>> again when the domains are being unpaused.
>>
>> This patch defers the migration check in case of paused vcpus or domains
>> by adding vcpu_arouse() to wake up a vcpu and check whether it must be
>> migrated to another cpu.
>>
>> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
>>
>>
>> 3 files changed, 64 insertions(+), 24 deletions(-)
>> xen/common/domain.c     |    4 +-
>> xen/common/schedule.c   |   83 ++++++++++++++++++++++++++++++++++-------------
>> xen/include/xen/sched.h |    1
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
>


-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Jan Beulich]

>>> On 22.03.12 at 09:22, Juergen Gross <juergen.gross@ts.fujitsu.com> wrote:
>--- a/xen/common/schedule.c	Thu Mar 22 09:21:44 2012 +0100
>+++ b/xen/common/schedule.c	Thu Mar 22 09:21:47 2012 +0100
>@@ -525,6 +525,47 @@ void vcpu_force_reschedule(struct vcpu *
>     }
> }
> 
>+static int vcpu_chk_affinity(struct vcpu *v, unsigned int cpu)
>+{
>+    cpumask_t online_affinity;
>+    struct cpupool *c;
>+
>+    c = v->domain->cpupool;
>+    cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
>+    if ( cpumask_empty(&online_affinity) &&

There's no need for a local cpumask_t variable here - please use
!cpumask_intersects(v->cpu_affinity, c->cpu_valid) instead.

>+         cpumask_test_cpu(cpu, v->cpu_affinity) )
>+    {
>+        printk("Breaking vcpu affinity for domain %d vcpu %d\n",
>+                v->domain->domain_id, v->vcpu_id);
>+        cpumask_setall(v->cpu_affinity);
>+    }
>+
>+    return ( !cpumask_test_cpu(cpu, c->cpu_valid) && (v->processor == cpu) );

Please drop the extra outermost parentheses.

>+}
>+
>+void vcpu_arouse(struct vcpu *v)
>+{
>+    unsigned long flags;
>+
>+    if ( atomic_read(&v->pause_count) ||
>+         atomic_read(&v->domain->pause_count) )

Is it not possible (or even more correct) to use vcpu_runnable() here?

>+        return;
>+
>+    vcpu_schedule_lock_irqsave(v, flags);
>+
>+    if ( unlikely(vcpu_chk_affinity(v, v->processor) && (v != current)) )

unlikely() is generally useful only around individual conditions (i.e.
not around && or || expressions).

>+    {
>+        set_bit(_VPF_migrating, &v->pause_flags);
>+        vcpu_schedule_unlock_irqrestore(v, flags);
>+        vcpu_migrate(v);
>+        return;
>+    }
>+
>+    vcpu_schedule_unlock_irqrestore(v, flags);
>+
>+    vcpu_wake(v);
>+}
>+
> /*
>  * This function is used by cpu_hotplug code from stop_machine context
>  * and from cpupools to switch schedulers on a cpu.
>@@ -534,7 +575,6 @@ int cpu_disable_scheduler(unsigned int c
>     struct domain *d;
>     struct vcpu *v;
>     struct cpupool *c;
>-    cpumask_t online_affinity;
>     int    ret = 0;
> 
>     c = per_cpu(cpupool, cpu);
>@@ -547,34 +587,33 @@ int cpu_disable_scheduler(unsigned int c
>         {
>             vcpu_schedule_lock_irq(v);
> 
>-            cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
>-            if ( cpumask_empty(&online_affinity) &&
>-                 cpumask_test_cpu(cpu, v->cpu_affinity) )
>+            if ( likely(!atomic_read(&v->pause_count) &&
>+                        !atomic_read(&d->pause_count)) )

Same question as above regarding vcpu_runnable().

>             {
>-                printk("Breaking vcpu affinity for domain %d vcpu %d\n",
>-                        v->domain->domain_id, v->vcpu_id);
>-                cpumask_setall(v->cpu_affinity);
>-            }
>+                if ( vcpu_chk_affinity(v, cpu) )
>+                {
>+                    set_bit(_VPF_migrating, &v->pause_flags);
>+                    vcpu_schedule_unlock_irq(v);
>+                    vcpu_sleep_nosync(v);
>+                    vcpu_migrate(v);
>+                }
>+                else
>+                {
>+                    vcpu_schedule_unlock_irq(v);
>+                }

Please drop the unnecessary braces here, as per the recently posted
coding style draft.

Jan

> 
>-            if ( v->processor == cpu )
>-            {
>-                set_bit(_VPF_migrating, &v->pause_flags);
>-                vcpu_schedule_unlock_irq(v);
>-                vcpu_sleep_nosync(v);
>-                vcpu_migrate(v);
>+                /*
>+                 * A vcpu active in the hypervisor will not be migratable.
>+                 * The caller should try again after releasing and reaquiring
>+                 * all locks.
>+                 */
>+                if ( v->processor == cpu )
>+                    ret = -EAGAIN;
>             }
>             else
>             {
>                 vcpu_schedule_unlock_irq(v);
>             }
>-
>-            /*
>-             * A vcpu active in the hypervisor will not be migratable.
>-             * The caller should try again after releasing and reaquiring
>-             * all locks.
>-             */
>-            if ( v->processor == cpu )
>-                ret = -EAGAIN;
>         }
> 
>         domain_update_node_affinity(d);

Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Juergen Gross]

On 03/22/2012 11:26 AM, Jan Beulich wrote:
>>>> On 22.03.12 at 09:22, Juergen Gross<juergen.gross@ts.fujitsu.com>  wrote:
>> --- a/xen/common/schedule.c	Thu Mar 22 09:21:44 2012 +0100
>> +++ b/xen/common/schedule.c	Thu Mar 22 09:21:47 2012 +0100
>> @@ -525,6 +525,47 @@ void vcpu_force_reschedule(struct vcpu *
>>      }
>> }
>>
>> +static int vcpu_chk_affinity(struct vcpu *v, unsigned int cpu)
>> +{
>> +    cpumask_t online_affinity;
>> +    struct cpupool *c;
>> +
>> +    c = v->domain->cpupool;
>> +    cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
>> +    if ( cpumask_empty(&online_affinity)&&
> There's no need for a local cpumask_t variable here - please use
> !cpumask_intersects(v->cpu_affinity, c->cpu_valid) instead.

Okay.

>> +         cpumask_test_cpu(cpu, v->cpu_affinity) )
>> +    {
>> +        printk("Breaking vcpu affinity for domain %d vcpu %d\n",
>> +                v->domain->domain_id, v->vcpu_id);
>> +        cpumask_setall(v->cpu_affinity);
>> +    }
>> +
>> +    return ( !cpumask_test_cpu(cpu, c->cpu_valid)&&  (v->processor == cpu) );
> Please drop the extra outermost parentheses.

Okay.

>> +}
>> +
>> +void vcpu_arouse(struct vcpu *v)
>> +{
>> +    unsigned long flags;
>> +
>> +    if ( atomic_read(&v->pause_count) ||
>> +         atomic_read(&v->domain->pause_count) )
> Is it not possible (or even more correct) to use vcpu_runnable() here?

No. There might be flags set in v->pause_flags which I don't want to test here.
I'm only interested in "real" paused state. An old "migrating" case should be
reconsidered here, e.g.

>> +        return;
>> +
>> +    vcpu_schedule_lock_irqsave(v, flags);
>> +
>> +    if ( unlikely(vcpu_chk_affinity(v, v->processor)&&  (v != current)) )
> unlikely() is generally useful only around individual conditions (i.e.
> not around&&  or || expressions).

Ah, I didn't know that. Will change it.

>> +    {
>> +        set_bit(_VPF_migrating,&v->pause_flags);
>> +        vcpu_schedule_unlock_irqrestore(v, flags);
>> +        vcpu_migrate(v);
>> +        return;
>> +    }
>> +
>> +    vcpu_schedule_unlock_irqrestore(v, flags);
>> +
>> +    vcpu_wake(v);
>> +}
>> +
>> /*
>>   * This function is used by cpu_hotplug code from stop_machine context
>>   * and from cpupools to switch schedulers on a cpu.
>> @@ -534,7 +575,6 @@ int cpu_disable_scheduler(unsigned int c
>>      struct domain *d;
>>      struct vcpu *v;
>>      struct cpupool *c;
>> -    cpumask_t online_affinity;
>>      int    ret = 0;
>>
>>      c = per_cpu(cpupool, cpu);
>> @@ -547,34 +587,33 @@ int cpu_disable_scheduler(unsigned int c
>>          {
>>              vcpu_schedule_lock_irq(v);
>>
>> -            cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
>> -            if ( cpumask_empty(&online_affinity)&&
>> -                 cpumask_test_cpu(cpu, v->cpu_affinity) )
>> +            if ( likely(!atomic_read(&v->pause_count)&&
>> +                        !atomic_read(&d->pause_count)) )
> Same question as above regarding vcpu_runnable().

Same answer :-)

>>              {
>> -                printk("Breaking vcpu affinity for domain %d vcpu %d\n",
>> -                        v->domain->domain_id, v->vcpu_id);
>> -                cpumask_setall(v->cpu_affinity);
>> -            }
>> +                if ( vcpu_chk_affinity(v, cpu) )
>> +                {
>> +                    set_bit(_VPF_migrating,&v->pause_flags);
>> +                    vcpu_schedule_unlock_irq(v);
>> +                    vcpu_sleep_nosync(v);
>> +                    vcpu_migrate(v);
>> +                }
>> +                else
>> +                {
>> +                    vcpu_schedule_unlock_irq(v);
>> +                }
> Please drop the unnecessary braces here, as per the recently posted
> coding style draft.

Okay.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Keir Fraser]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

On 22/03/2012 10:22, "Juergen Gross" <juergen.gross@ts.fujitsu.com> wrote:

> On 03/22/2012 11:12 AM, Keir Fraser wrote:
>> Your original patch didn't touch this code. Was that an omission in the
>> original version? On reflection I prefer your original patch to this new
>> approach. I'll apply it if you still believe your original patch is complete
>> and correct as it stands.
> 
> I like my second patch more :-)
> 
> It covers more cases, not just poweroff. In hibernate case no vcpu pinnings
> will be lost. Today all vcpus pinned to a cpu other than 0 will lose their
> pinnings at cpu offlining. At reactivation those pinnings will not be
> restored automatically. My patch will cover that by checking availability
> of the cpus after reactivation.
> 
> Poweroff (which was my primary concern) works with both versions. I did not
> test other ACPI state changes with either version, but would expect better
> results in hibernate case with my second approach.

How about the attached patch? Which is similar to your original patch except
I added the global state variable, and I added a check for it to
cpu_disable_scheduler(). It's nice and small. :-)

 -- Keir

> 
> Juergen
> 
>>   -- Keir
>> 
>> On 22/03/2012 08:22, "Juergen Gross"<juergen.gross@ts.fujitsu.com>  wrote:
>> 
>>> Currently offlining a cpu will migrate all vcpus which were active on that
>>> cpu to another one, possibly breaking existing cpu affinities.
>>> 
>>> In case of an ACPI state change the cpus are taken offline and online later
>>> (if not poweroff) while all domains are paused. There is no reason to
>>> migrate the vcpus during offlining the cpus, as the cpus will be available
>>> again when the domains are being unpaused.
>>> 
>>> This patch defers the migration check in case of paused vcpus or domains
>>> by adding vcpu_arouse() to wake up a vcpu and check whether it must be
>>> migrated to another cpu.
>>> 
>>> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
>>> 
>>> 
>>> 3 files changed, 64 insertions(+), 24 deletions(-)
>>> xen/common/domain.c     |    4 +-
>>> xen/common/schedule.c   |   83
>>> ++++++++++++++++++++++++++++++++++-------------
>>> xen/include/xen/sched.h |    1
>>> 
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>> 
>> 
> 


[-- Attachment #2: 00-suspend-affinities --]
[-- Type: application/octet-stream, Size: 6209 bytes --]

diff -r 6bf50858c3c5 xen/arch/arm/setup.c
--- a/xen/arch/arm/setup.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/arch/arm/setup.c	Thu Mar 22 11:11:39 2012 +0000
@@ -253,6 +253,8 @@ void __init start_xen(unsigned long boot
     /* Hide UART from DOM0 if we're using it */
     serial_endboot();
 
+    system_state = SYS_STATE_active;
+
     domain_unpause_by_systemcontroller(dom0);
 
     /* Switch on to the dynamically allocated stack for the idle vcpu
diff -r 6bf50858c3c5 xen/arch/x86/acpi/power.c
--- a/xen/arch/x86/acpi/power.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/arch/x86/acpi/power.c	Thu Mar 22 11:11:39 2012 +0000
@@ -135,6 +135,9 @@ static int enter_state(u32 state)
     if ( !spin_trylock(&pm_lock) )
         return -EBUSY;
 
+    BUG_ON(system_state != SYS_STATE_active);
+    system_state = SYS_STATE_suspend;
+
     printk(XENLOG_INFO "Preparing system for ACPI S%d state.\n", state);
 
     freeze_domains();
@@ -142,7 +145,10 @@ static int enter_state(u32 state)
     acpi_dmar_reinstate();
 
     if ( (error = disable_nonboot_cpus()) )
+    {
+        system_state = SYS_STATE_resume;
         goto enable_cpu;
+    }
 
     cpufreq_del_cpu(0);
 
@@ -159,6 +165,7 @@ static int enter_state(u32 state)
     if ( (error = device_power_down()) )
     {
         printk(XENLOG_ERR "Some devices failed to power down.");
+        system_state = SYS_STATE_resume;
         goto done;
     }
 
@@ -179,6 +186,8 @@ static int enter_state(u32 state)
         break;
     }
 
+    system_state = SYS_STATE_resume;
+
     /* Restore CR4 and EFER from cached values. */
     cr4 = read_cr4();
     write_cr4(cr4 & ~X86_CR4_MCE);
@@ -212,6 +221,7 @@ static int enter_state(u32 state)
     mtrr_aps_sync_end();
     acpi_dmar_zap();
     thaw_domains();
+    system_state = SYS_STATE_active;
     spin_unlock(&pm_lock);
     return error;
 }
diff -r 6bf50858c3c5 xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/arch/x86/mm.c	Thu Mar 22 11:11:39 2012 +0000
@@ -5316,7 +5316,7 @@ int ptwr_do_page_fault(struct vcpu *v, u
 
 void free_xen_pagetable(void *v)
 {
-    if ( early_boot )
+    if ( system_state == SYS_STATE_early_boot )
         return;
 
     if ( is_xen_heap_page(virt_to_page(v)) )
diff -r 6bf50858c3c5 xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/arch/x86/setup.c	Thu Mar 22 11:11:39 2012 +0000
@@ -81,8 +81,6 @@ boolean_param("noapic", skip_ioapic_setu
 s8 __read_mostly xen_cpuidle = -1;
 boolean_param("cpuidle", xen_cpuidle);
 
-bool_t __read_mostly early_boot = 1;
-
 cpumask_t __read_mostly cpu_present_map;
 
 unsigned long __read_mostly xen_phys_start;
@@ -271,7 +269,7 @@ static void *__init bootstrap_map(const 
     void *ret;
 
 #ifdef __x86_64__
-    if ( !early_boot )
+    if ( system_state != SYS_STATE_early_boot )
         return mod ? mfn_to_virt(mod->mod_start) : NULL;
 #endif
 
@@ -1168,7 +1166,7 @@ void __init __start_xen(unsigned long mb
 #endif
 
     end_boot_allocator();
-    early_boot = 0;
+    system_state = SYS_STATE_boot;
 
 #if defined(CONFIG_X86_64)
     vesa_init();
@@ -1391,6 +1389,8 @@ void __init __start_xen(unsigned long mb
 
     dmi_end_boot();
 
+    system_state = SYS_STATE_active;
+
     domain_unpause_by_systemcontroller(dom0);
 
     reset_stack_and_jump(init_done);
diff -r 6bf50858c3c5 xen/arch/x86/x86_32/mm.c
--- a/xen/arch/x86/x86_32/mm.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/arch/x86/x86_32/mm.c	Thu Mar 22 11:11:39 2012 +0000
@@ -43,7 +43,7 @@ void *alloc_xen_pagetable(void)
 {
     unsigned long mfn;
 
-    if ( !early_boot )
+    if ( system_state != SYS_STATE_early_boot )
     {
         void *v = alloc_xenheap_page();
 
diff -r 6bf50858c3c5 xen/arch/x86/x86_64/mm.c
--- a/xen/arch/x86/x86_64/mm.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/arch/x86/x86_64/mm.c	Thu Mar 22 11:11:39 2012 +0000
@@ -85,7 +85,7 @@ void *alloc_xen_pagetable(void)
 {
     unsigned long mfn;
 
-    if ( !early_boot )
+    if ( system_state != SYS_STATE_early_boot )
     {
         struct page_info *pg = alloc_domheap_page(NULL, 0);
 
diff -r 6bf50858c3c5 xen/common/cpupool.c
--- a/xen/common/cpupool.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/common/cpupool.c	Thu Mar 22 11:11:39 2012 +0000
@@ -629,6 +629,10 @@ static int cpu_callback(
     unsigned int cpu = (unsigned long)hcpu;
     int rc = 0;
 
+    if ( (system_state == SYS_STATE_suspend) ||
+         (system_state == SYS_STATE_resume) )
+        goto out;
+
     switch ( action )
     {
     case CPU_DOWN_FAILED:
@@ -642,6 +646,7 @@ static int cpu_callback(
         break;
     }
 
+out:
     return !rc ? NOTIFY_DONE : notifier_from_errno(rc);
 }
 
diff -r 6bf50858c3c5 xen/common/kernel.c
--- a/xen/common/kernel.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/common/kernel.c	Thu Mar 22 11:11:39 2012 +0000
@@ -20,6 +20,8 @@
 
 #ifndef COMPAT
 
+enum system_state system_state = SYS_STATE_early_boot;
+
 int tainted;
 
 xen_commandline_t saved_cmdline;
diff -r 6bf50858c3c5 xen/common/schedule.c
--- a/xen/common/schedule.c	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/common/schedule.c	Thu Mar 22 11:11:39 2012 +0000
@@ -538,7 +538,7 @@ int cpu_disable_scheduler(unsigned int c
     int    ret = 0;
 
     c = per_cpu(cpupool, cpu);
-    if ( c == NULL )
+    if ( (c == NULL) || (system_state == SYS_STATE_suspend) )
         return ret;
 
     for_each_domain_in_cpupool ( d, c )
diff -r 6bf50858c3c5 xen/include/asm-x86/setup.h
--- a/xen/include/asm-x86/setup.h	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/include/asm-x86/setup.h	Thu Mar 22 11:11:39 2012 +0000
@@ -3,7 +3,6 @@
 
 #include <xen/multiboot.h>
 
-extern bool_t early_boot;
 extern unsigned long xenheap_initial_phys_start;
 
 void early_cpu_init(void);
diff -r 6bf50858c3c5 xen/include/xen/kernel.h
--- a/xen/include/xen/kernel.h	Thu Mar 22 10:26:03 2012 +0000
+++ b/xen/include/xen/kernel.h	Thu Mar 22 11:11:39 2012 +0000
@@ -87,5 +87,13 @@ extern char _sinittext[], _einittext[];
     (__p >= _sinittext) && (__p < _einittext);  \
 })
 
+extern enum system_state {
+    SYS_STATE_early_boot,
+    SYS_STATE_boot,
+    SYS_STATE_active,
+    SYS_STATE_suspend,
+    SYS_STATE_resume
+} system_state;
+
 #endif /* _LINUX_KERNEL_H */
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 3227 bytes --]

On 03/22/2012 12:20 PM, Keir Fraser wrote:
> On 22/03/2012 10:22, "Juergen Gross"<juergen.gross@ts.fujitsu.com>  wrote:
>
>> On 03/22/2012 11:12 AM, Keir Fraser wrote:
>>> Your original patch didn't touch this code. Was that an omission in the
>>> original version? On reflection I prefer your original patch to this new
>>> approach. I'll apply it if you still believe your original patch is complete
>>> and correct as it stands.
>> I like my second patch more :-)
>>
>> It covers more cases, not just poweroff. In hibernate case no vcpu pinnings
>> will be lost. Today all vcpus pinned to a cpu other than 0 will lose their
>> pinnings at cpu offlining. At reactivation those pinnings will not be
>> restored automatically. My patch will cover that by checking availability
>> of the cpus after reactivation.
>>
>> Poweroff (which was my primary concern) works with both versions. I did not
>> test other ACPI state changes with either version, but would expect better
>> results in hibernate case with my second approach.
> How about the attached patch? Which is similar to your original patch except
> I added the global state variable, and I added a check for it to
> cpu_disable_scheduler(). It's nice and small. :-)

And it works in all of my test cases for poweroff. :-)

Acked-by: Juergen Gross <juergen.gross@ts.fujitsu.com>


Thanks, Juergen

>   -- Keir
>
>> Juergen
>>
>>>    -- Keir
>>>
>>> On 22/03/2012 08:22, "Juergen Gross"<juergen.gross@ts.fujitsu.com>   wrote:
>>>
>>>> Currently offlining a cpu will migrate all vcpus which were active on that
>>>> cpu to another one, possibly breaking existing cpu affinities.
>>>>
>>>> In case of an ACPI state change the cpus are taken offline and online later
>>>> (if not poweroff) while all domains are paused. There is no reason to
>>>> migrate the vcpus during offlining the cpus, as the cpus will be available
>>>> again when the domains are being unpaused.
>>>>
>>>> This patch defers the migration check in case of paused vcpus or domains
>>>> by adding vcpu_arouse() to wake up a vcpu and check whether it must be
>>>> migrated to another cpu.
>>>>
>>>> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
>>>>
>>>>
>>>> 3 files changed, 64 insertions(+), 24 deletions(-)
>>>> xen/common/domain.c     |    4 +-
>>>> xen/common/schedule.c   |   83
>>>> ++++++++++++++++++++++++++++++++++-------------
>>>> xen/include/xen/sched.h |    1
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xen.org
>>>> http://lists.xen.org/xen-devel
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>>>
>>>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html


[-- Attachment #1.2: Type: text/html, Size: 5107 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Juergen Gross]

On 03/22/2012 12:20 PM, Keir Fraser wrote:
> On 22/03/2012 10:22, "Juergen Gross"<juergen.gross@ts.fujitsu.com>  wrote:
>
>> On 03/22/2012 11:12 AM, Keir Fraser wrote:
>>> Your original patch didn't touch this code. Was that an omission in the
>>> original version? On reflection I prefer your original patch to this new
>>> approach. I'll apply it if you still believe your original patch is complete
>>> and correct as it stands.
>> I like my second patch more :-)
>>
>> It covers more cases, not just poweroff. In hibernate case no vcpu pinnings
>> will be lost. Today all vcpus pinned to a cpu other than 0 will lose their
>> pinnings at cpu offlining. At reactivation those pinnings will not be
>> restored automatically. My patch will cover that by checking availability
>> of the cpus after reactivation.
>>
>> Poweroff (which was my primary concern) works with both versions. I did not
>> test other ACPI state changes with either version, but would expect better
>> results in hibernate case with my second approach.
> How about the attached patch? Which is similar to your original patch except
> I added the global state variable, and I added a check for it to
> cpu_disable_scheduler(). It's nice and small. :-)

Would you mind putting it in 4.1, too?


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html