All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Gautham R Shenoy <ego@in.ibm.com>,
	linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
	Arun Bharadwaj <arun@linux.vnet.ibm.com>,
	Ingo Molnar <mingo@elte.hu>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [v4 PATCH 1/5]: cpuidle: Cleanup drivers/cpuidle/cpuidle.c
Date: Thu, 3 Sep 2009 10:12:53 +0530	[thread overview]
Message-ID: <20090903044253.GA31928@linux.vnet.ibm.com> (raw)
In-Reply-To: <1251870144.7547.48.camel@twins>

* Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-09-02 07:42:24]:

> On Tue, 2009-09-01 at 17:08 +0530, Arun R Bharadwaj wrote:
> > * Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-09-01 17:07:04]:
> > 
> > Cleanup drivers/cpuidle/cpuidle.c
> > 
> > Cpuidle maintains a pm_idle_old void pointer because, currently in x86
> > there is no clean way of registering and unregistering a idle function.
> 
> Right, and instead of fixing that, they build this cpuidle crap on top,
> instead of replacing the current crap with it.
> 
> > So remove pm_idle_old and leave the responsibility of maintaining the
> > list of registered idle loops to the architecture specific code. If the
> > architecture registers cpuidle_idle_call as its idle loop, only then
> > this loop is called.
> 
> OK, that's a start I guess. Best would be to replace all of pm_idle with
> cpuidle, which is what should have been done from the very start.
> 
> If cpuidle cannot fully replace the pm_idle functionality, then it needs
> to fix that. But having two layers of idle functions is just silly.
> 
> Looking at patch 2 and 3, you're making the same mistake on power, after
> those patches there are multiple ways of registering idle functions, one
> through some native interface and one through cpuidle, this strikes me
> as undesirable.
> 
> If cpuidle is a good idle function manager, then it should be good
> enough to be the sole one, if its not, then why bother with it at all.
> 

Okay, I'm giving this approach a shot now. i.e. trying to make cpuidle
as _the_ sole idle function manager. This would mean doing away with
pm_idle and ppc_md.power_save. And, cpuidle_idle_call() which is the
main idle loop of cpuidle, present in drivers/cpuidle/cpuidle.c will
have to be called from arch specific code of cpu_idle()

Also this would mean enabling cpuidle for all platforms, even if the
platform doesn't have multiple idle states. So suppose a platform doesnt
have multiple states, it wouldn't want the bloated code of cpuidle
governors, and would want just a simple cpuidle loop.

--arun
> 

WARNING: multiple messages have this Message-ID (diff)
From: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Joel Schopp <jschopp@austin.ibm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@elte.hu>,
	Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
	Dipankar Sarma <dipankar@in.ibm.com>,
	Balbir Singh <balbir@in.ibm.com>,
	Gautham R Shenoy <ego@in.ibm.com>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Arun Bharadwaj <arun@linux.vnet.ibm.com>
Subject: Re: [v4 PATCH 1/5]: cpuidle: Cleanup drivers/cpuidle/cpuidle.c
Date: Thu, 3 Sep 2009 10:12:53 +0530	[thread overview]
Message-ID: <20090903044253.GA31928@linux.vnet.ibm.com> (raw)
In-Reply-To: <1251870144.7547.48.camel@twins>

* Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-09-02 07:42:24]:

> On Tue, 2009-09-01 at 17:08 +0530, Arun R Bharadwaj wrote:
> > * Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-09-01 17:07:04]:
> > 
> > Cleanup drivers/cpuidle/cpuidle.c
> > 
> > Cpuidle maintains a pm_idle_old void pointer because, currently in x86
> > there is no clean way of registering and unregistering a idle function.
> 
> Right, and instead of fixing that, they build this cpuidle crap on top,
> instead of replacing the current crap with it.
> 
> > So remove pm_idle_old and leave the responsibility of maintaining the
> > list of registered idle loops to the architecture specific code. If the
> > architecture registers cpuidle_idle_call as its idle loop, only then
> > this loop is called.
> 
> OK, that's a start I guess. Best would be to replace all of pm_idle with
> cpuidle, which is what should have been done from the very start.
> 
> If cpuidle cannot fully replace the pm_idle functionality, then it needs
> to fix that. But having two layers of idle functions is just silly.
> 
> Looking at patch 2 and 3, you're making the same mistake on power, after
> those patches there are multiple ways of registering idle functions, one
> through some native interface and one through cpuidle, this strikes me
> as undesirable.
> 
> If cpuidle is a good idle function manager, then it should be good
> enough to be the sole one, if its not, then why bother with it at all.
> 

Okay, I'm giving this approach a shot now. i.e. trying to make cpuidle
as _the_ sole idle function manager. This would mean doing away with
pm_idle and ppc_md.power_save. And, cpuidle_idle_call() which is the
main idle loop of cpuidle, present in drivers/cpuidle/cpuidle.c will
have to be called from arch specific code of cpu_idle()

Also this would mean enabling cpuidle for all platforms, even if the
platform doesn't have multiple idle states. So suppose a platform doesnt
have multiple states, it wouldn't want the bloated code of cpuidle
governors, and would want just a simple cpuidle loop.

--arun
> 

  reply	other threads:[~2009-09-03  4:43 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-01 11:37 [v4 PATCH 0/5]: cpuidle/POWER (REDISIGN): Introducing cpuidle to POWER Arun R Bharadwaj
2009-09-01 11:37 ` Arun R Bharadwaj
2009-09-01 11:38 ` [v4 PATCH 1/5]: cpuidle: Cleanup drivers/cpuidle/cpuidle.c Arun R Bharadwaj
2009-09-01 11:38   ` Arun R Bharadwaj
2009-09-01 17:28   ` Balbir Singh
2009-09-01 17:28     ` Balbir Singh
2009-09-02  5:21     ` Arun R Bharadwaj
2009-09-02  5:21       ` Arun R Bharadwaj
2009-09-02  5:45     ` Arun R Bharadwaj
2009-09-02  5:45       ` Arun R Bharadwaj
2009-09-02  5:42   ` Peter Zijlstra
2009-09-02  5:42     ` Peter Zijlstra
2009-09-03  4:42     ` Arun R Bharadwaj [this message]
2009-09-03  4:42       ` Arun R Bharadwaj
2009-09-03  9:40       ` Peter Zijlstra
2009-09-03  9:40         ` Peter Zijlstra
2009-09-01 11:39 ` [v4 PATCH 2/5]: cpuidle: Implement routines to register and unregister idle function Arun R Bharadwaj
2009-09-01 11:39   ` Arun R Bharadwaj
2009-09-01 11:40 ` [v4 PATCH 3/5]: pSeries: Incorporate registering of idle loop for pSeries Arun R Bharadwaj
2009-09-01 11:40   ` Arun R Bharadwaj
2009-09-01 11:41 ` [v4 PATCH 4/5]: cpuidle: Add Kconfig entry to enable cpuidle for POWER Arun R Bharadwaj
2009-09-01 11:41   ` Arun R Bharadwaj
2009-09-01 11:42 ` [v4 PATCH 5/5]: pSeries: Implement pSeries processor idle module Arun R Bharadwaj
2009-09-01 11:42   ` Arun R Bharadwaj

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=20090903044253.GA31928@linux.vnet.ibm.com \
    --to=arun@linux.vnet.ibm.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=ego@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.