On Fri, Oct 17, 2008 at 07:57:12PM -0700, Anirban Sinha wrote:
> This function (stop_this_cpu) in /arch/mips/kernel/smp.c does a
> local_irq_enable() and the adjacent comment says that it's because it
> may need to service _machine_restart IPI. Unfortunately,
> smp_call_function only sends IPIs to cores that are still online ( it
> uses the cpu_online_map U all_but_myself_map in
> smp_call_function_map()).
Usually a system would be restarted through some hardware mechanism -
probably a reset - anyway.
> So the bottom-line is, should we still keep the local irqs enabled or is
> this code totally redundant? I have seen other similar functions in
> other archs where they actually disable the local irqs.
You're right. The code is ancient old and once uppon a time it made sense
to do things this way but the MIPS version was never updates. Stop_this_cpu
also should try to minimize the power consumption by using the WAIT
instruction or whatever else a particular process has to offer.
I didn't try to optimize this for the 34K where a TC could try to halt
itself - there isn't really a point, I think.
A few other architectures are explicitly disabling interrupts but that's
also redundant because smp_call_function() invokes the function on other
processors with interrupts disabled.
Thanks for posting this,
Ralf
Signed-off-by: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 7b59cfb..b79ea70 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -163,8 +163,10 @@ static void stop_this_cpu(void *dummy)
* Remove this CPU:
*/
cpu_clear(smp_processor_id(), cpu_online_map);
- local_irq_enable(); /* May need to service _machine_restart IPI */
- for (;;); /* Wait if available. */
+ for (;;) {
+ if (cpu_wait)
+ (*cpu_wait)(); /* Wait if available. */
+ }
}
void smp_send_stop(void)
