linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-acpi@vger.kernel.org, arun@linux.vnet.ibm.com,
	Ingo Molnar <mingo@elte.hu>,
	linuxppc-dev@lists.ozlabs.org,
	Arjan van de Ven <arjan@infradead.org>
Subject: Re: [v8 PATCH 2/8]: cpuidle: implement a list based approach to register a set of idle routines.
Date: Thu, 8 Oct 2009 18:40:15 +0530	[thread overview]
Message-ID: <20091008131015.GD3735@dirshya.in.ibm.com> (raw)
In-Reply-To: <1255004737.26976.307.camel@twins>

* Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-10-08 14:25:37]:

> On Thu, 2009-10-08 at 17:31 +0530, Arun R Bharadwaj wrote:
> > 
> > > Uhm, no, it would mean ACPI putting its idle routines on the same level
> > > as all others.
> > > 
> > 
> > Putting them all on the same level would mean, we need an
> > enable/disable routine to enable only the currently active routines.
> 
> What's this enable/disable stuff about?

Don't we need an explicit 'don't use this routine' apart from having
the weights based on power consumption and latency.  In multiple
registration the assumption is that the top most 'set' has all
necessary routines and we do not need any other idle routines for
optimum operation.

Otherwise the governor has to select from larger 'set' which could
have redundant or conflicting idle routines.

For example we now have c1e_idle to start with and then a set of ACPI
C1, C2, C3 routines.  The expectation now is that once we have the
ACPI's routines, we do not need the previous used c1e_idle because
this new set is self contained and picking one from this set based on
latency is good for power savings.
 
> > Also, the way governor works is that, it assumes that idle routines
> > are indexed in the increasing order of power benefit that can be got
> > out of the state. So this would get messed up.
> 
> Right, which is why I initially had a power-savings field in my
> proposal, so it could weight the power savings vs the wakeup latency.
> 
>   http://lkml.org/lkml/2009/8/27/159

This proposal that you had suggested is being used for the 'set' of
idle routines.  The patch changes the idle routines as 'sets' and does
not mix use of routines between two registrations.
 
> There it was said that was exactly what these governors were doing,
> seems its not.

Governors select from a set of idle routines based on latency.  But
there is a probability that any of the routine in the set can be
selected.  

> > > Sounds like something is wrong alright. If you can register an idle
> > > routine you should be able to unregister it too.
> > >
> > 
> > Yes, we can register and unregister in a clean way now.
> > Consider this. We have a set of routines A, B, C currently registered.
> > Now a module comes and registers D and E, and later on at some point
> > of time wants to unregister. So how do you keep track of what all idle
> > routines the module registered and unregister only those?
> > Best way to do that is a stack, which is how I have currently
> > implemented.
> 
> Right, so destroy that inner set thing, that way we only have one
> left ;-)

If un-registration is not needed, then this framework can easily
override the current set with the new one and not worry about the set
of sets.

Ideally, during system boot, we could wait until we know enough
information about idle states and then have a single registration.
The boot process can be in poll-idle until this decision happens.
Like in x86, we can register either c1e_idle or ACPI's routines at
a stage where we know for sure if ACPI is enabled or not.

--Vaidy
 

  reply	other threads:[~2009-10-08 13:10 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-08  9:48 [v8 PATCH 0/8]: cpuidle: Cleanup cpuidle/ Introduce cpuidle to POWER Arun R Bharadwaj
2009-10-08  9:49 ` [v8 PATCH 1/8]: cpuidle: cleanup drivers/cpuidle/cpuidle.c Arun R Bharadwaj
2009-10-12 11:36   ` Balbir Singh
2009-10-14  6:24     ` Arun R Bharadwaj
2009-10-08  9:50 ` [v8 PATCH 2/8]: cpuidle: implement a list based approach to register a set of idle routines Arun R Bharadwaj
2009-10-08 10:36   ` Peter Zijlstra
2009-10-08 10:42     ` Arun R Bharadwaj
2009-10-08 10:50       ` Peter Zijlstra
2009-10-08 11:01         ` Arun R Bharadwaj
2009-10-08 11:25           ` Peter Zijlstra
2009-10-08 12:01             ` Arun R Bharadwaj
2009-10-08 12:25               ` Peter Zijlstra
2009-10-08 13:10                 ` Vaidyanathan Srinivasan [this message]
2009-10-09  9:39                 ` Arun R Bharadwaj
2009-10-12 18:00         ` Andi Kleen
2009-10-14  6:17           ` Arun R Bharadwaj
2009-10-14  7:18             ` Andi Kleen
2009-10-15  6:06               ` Arun R Bharadwaj
2009-10-08  9:51 ` [v8 PATCH 3/8]: x86: refactor x86 idle power management code and remove all instances of pm_idle Arun R Bharadwaj
2009-10-08  9:52 ` [v8 PATCH 4/8]: POWER: enable cpuidle for POWER Arun R Bharadwaj
2009-10-08  9:53 ` [v8 PATCH 5/8]: pSeries/cpuidle: remove dedicate/shared idle loops, which will be moved to arch/powerpc/platforms/pseries/processor_idle.c Arun R Bharadwaj
2009-10-08  9:53 ` [v8 PATCH 6/8]: POWER: add a default_idle idle loop for POWER Arun R Bharadwaj
2009-10-08  9:54 ` [v8 PATCH 7/8]: pSeries: implement pSeries processor idle module Arun R Bharadwaj
2009-10-08  9:56 ` [v8 PATCH 8/8]: POWER: Enable default_idle when power_save=off Arun R Bharadwaj
2009-10-12 10:01 ` [v8 PATCH 0/8]: cpuidle: Cleanup cpuidle/ Introduce cpuidle to POWER Balbir Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091008131015.GD3735@dirshya.in.ibm.com \
    --to=svaidy@linux.vnet.ibm.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=arjan@infradead.org \
    --cc=arun@linux.vnet.ibm.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).