On 06/01/2012 06:29 PM, Jan Beulich wrote:
quoted
quoted
quoted
On 01.06.12 at 11:11, "Srivatsa S. Bhat" [off-list ref]
wrote:
quoted
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?
Thanks for your comments Jan!
In case this change is wrong, the other method I had in mind was to call
cpu_bringup_and_idle() in xen_play_dead(). (Even ARM does something similar,
in the sense that it runs the cpu bringup code including cpu_idle(), in the
cpu offline path, namely the cpu_die() function). Would that approach work
for xen as well? If yes, then we wouldn't have any issues to convert xen to
generic code.
Regards,
Srivatsa S. Bhat
quoted
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 <redacted>
Cc: Jeremy Fitzhardinge <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: <redacted>
Cc: <redacted>
Signed-off-by: Srivatsa S. Bhat <redacted>
---
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 */