Re: [PATCH] idle using PNI monitor/mwait (take 2)

[Jamie Lokier <jamie@shareable.org>]

Nakajima, Jun wrote:
> > The result is a halted CPU even though need_resched is set, and the
> > idle loop isn't broken until the next interrupt, often a timer tick.
> 
> I'm aware of the window, but did not realize that it could cause high
> latency spikes. Is that still the case with 2.6 where we have higher HZ
> (1000)? Anyway, I think it's a cheap way of removing such spikes.

HZ is irrelevant.  If you receive an interrupt, and typical wakeup
latency is 0.05ms, then waiting until the next timer tick in 1ms is a spike.
Some applications tolerate that, some don't.

> > So you can remove it from your loop.
> Okay we'll remove local_irq_enable() at entry. So in that case we can
> remove the local_irq_enable() below as well?
> 
> static void poll_idle (void)
> {
> 	int oldval;
> 
> =>	local_irq_enable();

I misunderstood you and now you misunderstood me :)

It's ok to _disable_ irqs for the reasons I gave.  I thought of that
because you pointed to the _disable_ in your quote of default_idle.

The _enable_ is there for a different reason.

Scheduling is not allowed with interrupts disabled.  So we know that
when schedule() returns, local irqs are enabled.  So poll_idle() doesn't
need to enable them.  I suggest this change:

        - remove the local_irq_enable() from poll_idle().

        - add local_irq_enable() at the start of cpu_idle(), before the loop.

-- Jamie

diff -urN --exclude-from=dontdiff orig-2.6.0-test4/arch/i386/kernel/process.c idle_irqs-2.6.0-test4/arch/i386/kernel/process.c
--- orig-2.6.0-test4/arch/i386/kernel/process.c	2003-09-02 23:05:06.000000000 +0100
+++ idle_irqs-2.6.0-test4/arch/i386/kernel/process.c	2003-09-06 10:50:59.000000000 +0100
@@ -105,8 +105,6 @@
 {
 	int oldval;
 
-	local_irq_enable();
-
 	/*
 	 * Deal with another CPU just having chosen a thread to
 	 * run here:
@@ -136,6 +134,8 @@
  */
 void cpu_idle (void)
 {
+	local_irq_enable();
+
 	/* endless idle loop with no priority at all */
 	while (1) {
 		while (!need_resched()) {