Linux Virtualization - Jun 2012 - [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

xen_play_dead calls cpu_bringup() which looks weird, because xen_play_dead()
is invoked in the cpu down path, whereas cpu_bringup() (as the name suggests)
is useful in the cpu bringup path.

Getting rid of xen_play_dead()'s dependency on cpu_bringup() helps in
hooking
on to the generic SMP booting framework.

Also remove the extra call to preempt_enable() added by commit 41bd956
(xen/smp: Fix CPU online/offline bug triggering a BUG: scheduling while
atomic) because it becomes unnecessary after this change.

Cc: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
Cc: Jeremy Fitzhardinge <jeremy at goop.org>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Ingo Molnar <mingo at redhat.com>
Cc: "H. Peter Anvin" <hpa at zytor.com>
Cc: x86 at kernel.org
Cc: xen-devel at lists.xensource.com
Cc: virtualization at lists.linux-foundation.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat at linux.vnet.ibm.com>
---

 arch/x86/xen/smp.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 09a7199..602d6b7 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -417,14 +417,6 @@ static void __cpuinit xen_play_dead(void) /* used only with
HOTPLUG_CPU */
 {
 	play_dead_common();
 	HYPERVISOR_vcpu_op(VCPUOP_down, smp_processor_id(), NULL);
-	cpu_bringup();
-	/*
-	 * Balance out the preempt calls - as we are running in cpu_idle
-	 * loop which has been called at bootup from cpu_bringup_and_idle.
-	 * The cpucpu_bringup_and_idle called cpu_bringup which made a
-	 * preempt_disable() So this preempt_enable will balance it out.
-	 */
-	preempt_enable();
 }
 
 #else /* !CONFIG_HOTPLUG_CPU */

>>> On 01.06.12 at 11:11, "Srivatsa S. Bhat"
<srivatsa.bhat at linux.vnet.ibm.com>
wrote:
> xen_play_dead calls cpu_bringup() which looks weird, because
xen_play_dead()
> is invoked in the cpu down path, whereas cpu_bringup() (as the name 
> suggests) is useful in the cpu bringup path.
This might not be correct - the code as it is without this change is
safe even when the vCPU gets onlined back later by an external
entity (e.g. the Xen tool stack), and it would in that case resume
at the return point of the VCPUOP_down hypercall. That might
be a heritage from the original XenoLinux tree though, and be
meaningless in pv-ops context - Jeremy, Konrad?

Possibly it was bogus/unused even in that original tree - Keir?

Jan
> Getting rid of xen_play_dead()'s dependency on cpu_bringup() helps in 
> hooking on to the generic SMP booting framework.
> 
> Also remove the extra call to preempt_enable() added by commit 41bd956
> (xen/smp: Fix CPU online/offline bug triggering a BUG: scheduling while
> atomic) because it becomes unnecessary after this change.
> 
> Cc: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
> Cc: Jeremy Fitzhardinge <jeremy at goop.org>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Ingo Molnar <mingo at redhat.com>
> Cc: "H. Peter Anvin" <hpa at zytor.com>
> Cc: x86 at kernel.org 
> Cc: xen-devel at lists.xensource.com 
> Cc: virtualization at lists.linux-foundation.org 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat at linux.vnet.ibm.com>
> ---
> 
>  arch/x86/xen/smp.c |    8 --------
>  1 files changed, 0 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 09a7199..602d6b7 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -417,14 +417,6 @@ static void __cpuinit xen_play_dead(void) /* used only
> with HOTPLUG_CPU */
>  {
>  	play_dead_common();
>  	HYPERVISOR_vcpu_op(VCPUOP_down, smp_processor_id(), NULL);
> -	cpu_bringup();
> -	/*
> -	 * Balance out the preempt calls - as we are running in cpu_idle
> -	 * loop which has been called at bootup from cpu_bringup_and_idle.
> -	 * The cpucpu_bringup_and_idle called cpu_bringup which made a
> -	 * preempt_disable() So this preempt_enable will balance it out.
> -	 */
> -	preempt_enable();
>  }
>  
>  #else /* !CONFIG_HOTPLUG_CPU */
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel at lists.xen.org 
> http://lists.xen.org/xen-devel