[PATCH] make poll_idle behave more like the other idle methods

[PATCH] make poll_idle behave more like the other idle methods

[Joe Korty]

Make poll_idle() behave more like the other idle methods.

Currently, poll_idle() returns immediately.  The other
idle methods all wait indefinately for some condition
to come true before returning.  poll_idle should emulate
these other methods and also wait for a return condition,
in this case, for need_resched() to become 'true'.

Without this delay the idle loop spends all of its time
in the outer loop that calls poll_idle.  This outer loop,
these days, does real work, some of it under rcu locks.
That work should only be done when idle is entered and
when idle exits, not continuously while idle is spinning.

Signed-off-by: Joe Korty <joe.korty@ccur.com>

Index: 2.6.27-rc4-git4/arch/x86/kernel/process.c
===================================================================
--- 2.6.27-rc4-git4.orig/arch/x86/kernel/process.c	2008-08-26 17:44:34.000000000 -0400
+++ 2.6.27-rc4-git4/arch/x86/kernel/process.c	2008-08-26 18:22:19.000000000 -0400
@@ -185,7 +185,8 @@
 static void poll_idle(void)
 {
 	local_irq_enable();
-	cpu_relax();
+	while(!need_resched())
+		cpu_relax();
 }
 
 /*

Re: [PATCH] make poll_idle behave more like the other idle methods

[Ingo Molnar]

* Joe Korty <joe.korty@ccur.com> wrote:

> Make poll_idle() behave more like the other idle methods.
> 
> Currently, poll_idle() returns immediately.  The other
> idle methods all wait indefinately for some condition
> to come true before returning.  poll_idle should emulate
> these other methods and also wait for a return condition,
> in this case, for need_resched() to become 'true'.
> 
> Without this delay the idle loop spends all of its time
> in the outer loop that calls poll_idle.  This outer loop,
> these days, does real work, some of it under rcu locks.
> That work should only be done when idle is entered and
> when idle exits, not continuously while idle is spinning.

i'm wondering, what's the motivation, have you actually seen
anything bad/undesired happen due to that?

	Ingo

Re: [PATCH] make poll_idle behave more like the other idle methods

[Thomas Gleixner]

On Thu, 28 Aug 2008, Ingo Molnar wrote:
> * Joe Korty <joe.korty@ccur.com> wrote:
> 
> > Make poll_idle() behave more like the other idle methods.
> > 
> > Currently, poll_idle() returns immediately.  The other
> > idle methods all wait indefinately for some condition
> > to come true before returning.  poll_idle should emulate
> > these other methods and also wait for a return condition,
> > in this case, for need_resched() to become 'true'.
> > 
> > Without this delay the idle loop spends all of its time
> > in the outer loop that calls poll_idle.  This outer loop,
> > these days, does real work, some of it under rcu locks.
> > That work should only be done when idle is entered and
> > when idle exits, not continuously while idle is spinning.
> 
> i'm wondering, what's the motivation, have you actually seen
> anything bad/undesired happen due to that?

Hmm, two observations:

1) I think Joe is right that idle_poll should behave like any other
idle function.

2) I wonder whether the work you observe is something we should
investigate. The only code which does work in the idle loop is
rcu_check_callbacks(). What kind of work load scenario do you have
which makes the rcu_check_callbacks() called ?

Thanks,

	tglx

Re: [PATCH] make poll_idle behave more like the other idle methods

[Joe Korty]

On Thu, Aug 28, 2008 at 05:00:36AM -0400, Ingo Molnar wrote:
> 
> * Joe Korty <joe.korty@ccur.com> wrote:
> 
> > Make poll_idle() behave more like the other idle methods.
> >
> > Currently, poll_idle() returns immediately.  The other
> > idle methods all wait indefinately for some condition
> > to come true before returning.  poll_idle should emulate
> > these other methods and also wait for a return condition,
> > in this case, for need_resched() to become 'true'.
> >
> > Without this delay the idle loop spends all of its time
> > in the outer loop that calls poll_idle.  This outer loop,
> > these days, does real work, some of it under rcu locks.
> > That work should only be done when idle is entered and
> > when idle exits, not continuously while idle is spinning.
> 
> i'm wondering, what's the motivation, have you actually seen
> anything bad/undesired happen due to that?

I saw the outer loop running continuously, from the old
trace patch which I had applied.

Nowdays the outer loop runs the NO_HZ stuff, which touches
quite a few memory locations.  Having say two cpus in idle
and there is potential for cache thrashing to suck up a good
part of the local memory bus bandwidth.

And I suspect as time goes on cpu_idle will end up with
more work to do.

Joe

Re: [PATCH] make poll_idle behave more like the other idle methods

[Ingo Molnar]

* Joe Korty <joe.korty@ccur.com> wrote:

> On Thu, Aug 28, 2008 at 05:00:36AM -0400, Ingo Molnar wrote:
> > 
> > * Joe Korty <joe.korty@ccur.com> wrote:
> > 
> > > Make poll_idle() behave more like the other idle methods.
> > >
> > > Currently, poll_idle() returns immediately.  The other
> > > idle methods all wait indefinately for some condition
> > > to come true before returning.  poll_idle should emulate
> > > these other methods and also wait for a return condition,
> > > in this case, for need_resched() to become 'true'.
> > >
> > > Without this delay the idle loop spends all of its time
> > > in the outer loop that calls poll_idle.  This outer loop,
> > > these days, does real work, some of it under rcu locks.
> > > That work should only be done when idle is entered and
> > > when idle exits, not continuously while idle is spinning.
> > 
> > i'm wondering, what's the motivation, have you actually seen
> > anything bad/undesired happen due to that?
> 
> I saw the outer loop running continuously, from the old
> trace patch which I had applied.

ah - was that an older version of ftrace?

> Nowdays the outer loop runs the NO_HZ stuff, which touches
> quite a few memory locations.  Having say two cpus in idle
> and there is potential for cache thrashing to suck up a good
> part of the local memory bus bandwidth.
> 
> And I suspect as time goes on cpu_idle will end up with
> more work to do.

agreed.

	Ingo

Re: [PATCH] make poll_idle behave more like the other idle methods

[Joe Korty]

On Thu, Aug 28, 2008 at 09:33:35AM -0400, Ingo Molnar wrote:
>> I saw the outer loop running continuously, from the old
>> trace patch which I had applied.
> 
> ah - was that an older version of ftrace?

Believe it or not, it's a very old version of LTTng trace.

We have a guy that keeps dragging it forward (compatibility
reasons).

Joe