Re: [Xen-devel] [PATCH 05/27] xen, cpu hotplug: Don't call cpu_bringup() in xen_play_dead()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.06.12 at 11:11, "Srivatsa S. Bhat" <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxx>
> Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: x86@xxxxxxxxxx 
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx 
> Cc: virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>
> ---
> 
>  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@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 




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