On Wed, Jun 29, 2005 at 07:09:38PM +1000, Nick Piggin wrote: > Well you only need to disable IRQs if you are about to go to > sleep waiting for the next pending IRQ. So your hlt_counter > case looks OK. > > In the case that you do sleep until the next IRQ, sh64 does indeed > disable irqs over the need_resched check, however it re-enables them > before sleeping. So disabling at all is basically useless because > any pending IRQs will probably all get serviced right as soon as IRQs > are re-eanbled. > Ok, I've switched sh64 to use a similar model as sh.
> Well as you probably know, but just to be clear: architectures
> that handle this without a race have an instruction that basically
> turns on interrupts and go to sleep at the same time. I'm not aware
> of a simple way to do it without that facility.
>
> Unless you can easily raise an NMI from another processor as an IPI.
>
Unfortunately we don't have any such easy facility. The closest I suppose
would to have the watchdog generate an NMI, but that severely limits the
kind of sleep state that we are able to enter.
> As far as spin waiting goes, something like:
>
> while (!need_resched())
> cpu_relax();
>
> Is generally used.
>
> Now this might introduce some power and heat penalty. What's more,
> your race isn't a fatal one: in the worst case, it should just
> stall until the next timer interrupt (aside, that might be fatal
> with a tickless kernel).
>
After incorporating your changes, how about this?
--
diff --git a/arch/sh64/kernel/process.c b/arch/sh64/kernel/process.c
--- a/arch/sh64/kernel/process.c
+++ b/arch/sh64/kernel/process.c
@@ -305,44 +305,29 @@ static int __init hlt_setup(char *__unus
__setup("nohlt", nohlt_setup);
__setup("hlt", hlt_setup);
-static inline void hlt(void)
-{
- if (hlt_counter)
- return;
-
- __asm__ __volatile__ ("sleep" : : : "memory");
-}
-
/*
* The idle loop on a uniprocessor SH..
*/
-void default_idle(void)
+void cpu_idle(void)
{
/* endless idle loop with no priority at all */
while (1) {
if (hlt_counter) {
- while (1)
- if (need_resched())
- break;
+ while (!need_resched())
+ cpu_relax();
} else {
- local_irq_disable();
while (!need_resched()) {
- local_irq_enable();
idle_trace();
- hlt();
- local_irq_disable();
+ cpu_sleep();
}
- local_irq_enable();
}
+
+ preempt_enable_no_resched();
schedule();
+ preempt_disable();
}
}
-void cpu_idle(void)
-{
- default_idle();
-}
-
void machine_restart(char * __unused)
{
extern void phys_stext(void);
pgpQyMPSOCvGE.pgp
Description: PGP signature
