Re: Remaining arch problems in cpu_idle

[Paul Mundt <lethal@linux-sh.org>]

[-- Attachment #1: Type: text/plain, Size: 2654 bytes --]

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);

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]