From: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
To: Carsten Emde <C.Emde@osadl.org>
Cc: Len Brown <len.brown@intel.com>, Kevin Hilman <khilman@ti.com>,
Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] Honor state disabling in the cpuidle ladder governor
Date: Wed, 18 Jul 2012 12:06:03 +0530 [thread overview]
Message-ID: <50065953.9040904@linux.vnet.ibm.com> (raw)
In-Reply-To: <20120717190330.700421963@osadl.org>
On 07/18/2012 12:29 AM, Carsten Emde wrote:
> There are two cpuidle governors ladder and menu. While the ladder
> governor is always available, if CONFIG_CPU_IDLE is selected, the
> menu governor additionally requires CONFIG_NO_HZ.
>
> A particular C state can be disabled by writing to the sysfs file
> /sys/devices/system/cpu/cpuN/cpuidle/stateN/disable, but this mechanism
> is only implemented in the menu governor. Thus, in a system where
> CONFIG_NO_HZ is not selected, the ladder governor becomes default and
> always will walk through all sleep states - irrespective of whether the
> C state was disabled via sysfs or not. The only way to select a specific
> C state was to write the related latency to /dev/cpu_dma_latency and
> keep the file open as long as this setting was required - not very
> practical and not suitable for setting a single core in an SMP system.
>
> With this patch, the ladder governor only will promote to the next
> C state, if it has not been disabled, and it will demote, if the
> current C state was disabled.
Yes, I agree that currently that disabling a particular C-state
is not reflected in working of ladder governor. This patch is needed
to fix it on ladder too.
Also wanted to clarify on the intended implementation here,
if there are say 5 C-states on a system, disabling 2nd
state would also end by disabling all the remaining 3 deeper states too
as ladder governor enters the lightest state first, and will only move
on to the next deeper state if a idle period was long enough as
per the implementation.
If one is disabling only the deepest state, then it would
work as intended.
Cheers,
Deepthi
>
> Signed-off-by: Carsten Emde <C.Emde@osadl.org>
>
> ---
> drivers/cpuidle/governors/ladder.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-3.4.4-rt14-rc2-64/drivers/cpuidle/governors/ladder.c
> ===================================================================
> --- linux-3.4.4-rt14-rc2-64.orig/drivers/cpuidle/governors/ladder.c
> +++ linux-3.4.4-rt14-rc2-64/drivers/cpuidle/governors/ladder.c
> @@ -88,6 +88,7 @@ static int ladder_select_state(struct cp
>
> /* consider promotion */
> if (last_idx < drv->state_count - 1 &&
> + !drv->states[last_idx + 1].disable &&
> last_residency > last_state->threshold.promotion_time &&
> drv->states[last_idx + 1].exit_latency <= latency_req) {
> last_state->stats.promotion_count++;
> @@ -100,7 +101,8 @@ static int ladder_select_state(struct cp
>
> /* consider demotion */
> if (last_idx > CPUIDLE_DRIVER_STATE_START &&
> - drv->states[last_idx].exit_latency > latency_req) {
> + (drv->states[last_idx].disable ||
> + drv->states[last_idx].exit_latency > latency_req)) {
> int i;
>
> for (i = last_idx - 1; i > CPUIDLE_DRIVER_STATE_START; i--) {
>
next prev parent reply other threads:[~2012-07-18 6:36 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-17 18:59 [PATCH 0/1] cpuidle: allow to disable C states of the ladder governor Carsten Emde
2012-07-17 18:59 ` [PATCH 1/1] Honor state disabling in the cpuidle " Carsten Emde
2012-07-18 6:36 ` Deepthi Dharwar [this message]
2012-07-18 11:02 ` Carsten Emde
2012-07-18 11:48 ` Deepthi Dharwar
2012-07-18 14:09 ` [PATCH 1/1 v2] Honor state disabling in the cpuidle ladder governor - documented Carsten Emde
2012-07-18 14:38 ` [PATCH 1/1 v3] Honor state disabling in the cpuidle ladder governor - with sanitizer Carsten Emde
2012-07-19 11:14 ` Deepthi Dharwar
2012-07-19 11:39 ` Carsten Emde
2012-07-19 18:42 ` Rafael J. Wysocki
2012-07-19 18:52 ` [PATCH 0/1 v2] cpuidle: allow to disable C states of the ladder governor Carsten Emde
2012-07-19 18:52 ` [PATCH 1/1 v2] Honor state disabling in the cpuidle " Carsten Emde
2012-07-19 19:30 ` Rafael J. Wysocki
2012-07-19 20:34 ` [PATCH 0/1 v3] cpuidle: allow to disable C states of the " Carsten Emde
2012-07-19 20:34 ` [PATCH 1/1 v3] Honor state disabling in the cpuidle " Carsten Emde
2012-07-19 21:48 ` Rafael J. Wysocki
2012-07-19 22:22 ` Carsten Emde
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=50065953.9040904@linux.vnet.ibm.com \
--to=deepthi@linux.vnet.ibm.com \
--cc=C.Emde@osadl.org \
--cc=khilman@ti.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
/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).