Messages in this thread | | | Date | Tue, 11 Nov 2008 13:48:11 +0100 | From | Ivan Vecera <> | Subject | Re: [PATCH] x86: call machine_shutdown and stop all CPUs in native_machine_halt |
| |
Pavel Machek wrote: > On Thu 2008-11-06 17:10:01, Ivan Vecera wrote: >> Any comments? >> >> Ivan >> >> --- >> Ivan Vecera wrote: >>> Ingo Molnar wrote: >>>> the code in arch/x86/kernel/smp.c::stop_this_cpu() is very similar to >>>> this and could be shared. You could move the stop_this_cpu() function to >>>> arch/x86/kernel/process.c (out of smp.c), so that it can be used by >>>> reboot.c. >>>> >>> Yes, this make sense. Here is the patch. >>> >>> Ivan >>> >>> --- >>> arch/x86/kernel/process.c | 16 ++++++++++++++++ >>> arch/x86/kernel/reboot.c | 5 +++++ >>> arch/x86/kernel/smp.c | 13 ------------- >>> include/asm-x86/system.h | 2 ++ >>> 4 files changed, 23 insertions(+), 13 deletions(-) >>> >>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c >>> index c622772..af6904e 100644 >>> --- a/arch/x86/kernel/process.c >>> +++ b/arch/x86/kernel/process.c >>> @@ -8,6 +8,7 @@ >>> #include <linux/pm.h> >>> #include <linux/clockchips.h> >>> #include <asm/system.h> >>> +#include <asm/apic.h> >>> >>> unsigned long idle_halt; >>> EXPORT_SYMBOL(idle_halt); >>> @@ -122,6 +123,21 @@ void default_idle(void) >>> EXPORT_SYMBOL(default_idle); >>> #endif >>> >>> +void stop_this_cpu(void *dummy) >>> +{ >>> + local_irq_disable(); >>> + /* >>> + * Remove this CPU: >>> + */ >>> + cpu_clear(smp_processor_id(), cpu_online_map); >>> +#ifdef CONFIG_X86_LOCAL_APIC >>> + disable_local_APIC(); >>> +#endif >>> + if (hlt_works(smp_processor_id())) >>> + for (;;) halt(); >>> + for (;;); >>> +} > > Why the new ifdef? And while we are at it, why is it neccessary to > disable APIC for stopping CPU? (comment in code would be nice) Because stop_this_cpu is shared between native_machine_halt and native_smp_send_stop. It was only moved from smp.c to process.c. native_machine_halt doesn't require disable APIC for stopping the current CPU because at this time is already disabled. The ifdef is another thing and I sent already corrected patch. > >>> -static void stop_this_cpu(void *dummy) >>> -{ >>> - local_irq_disable(); >>> - /* >>> - * Remove this CPU: >>> - */ >>> - cpu_clear(smp_processor_id(), cpu_online_map); >>> - disable_local_APIC(); >>> - if (hlt_works(smp_processor_id())) >>> - for (;;) halt(); >>> - for (;;); >>> -} >>> - >>> /* >>> * this function calls the 'stop' function on all other CPUs in the system. >>> */ >
| |