linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cpuidle: add 'failed' statistic
@ 2014-03-07  4:14 Daniel Lezcano
  2014-03-11  2:00 ` Len Brown
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Lezcano @ 2014-03-07  4:14 UTC (permalink / raw)
  To: rjw; +Cc: linux-pm, linux-kernel

The actual statistics give some informations about the different idle states a
cpu entered but unfortunately it does not show if the state is resulting from
good selections or not. This simple patch adds the 'failed' statistic for each
state, so we can easily do a ratio between the 'usage' and the 'failed' to
deduce how efficient the state selections have been.

Is considered 'failed' when a state duration is lesser than the target
residency which means the state consumed more power than the expected power
saving.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/cpuidle/cpuidle.c |    4 ++++
 drivers/cpuidle/sysfs.c   |    3 +++
 include/linux/cpuidle.h   |    1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 09d05ab..c2323e7 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -100,6 +100,10 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
 		 */
 		dev->states_usage[entered_state].time += dev->last_residency;
 		dev->states_usage[entered_state].usage++;
+
+		/* We stayed less than the target residency */
+		if (diff < drv->states[entered_state].target_residency)
+			dev->states_usage[entered_state].failed++;
 	} else {
 		dev->last_residency = 0;
 	}
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index e918b6d..1c7eb90 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -296,6 +296,7 @@ define_show_state_function(exit_latency)
 define_show_state_function(power_usage)
 define_show_state_ull_function(usage)
 define_show_state_ull_function(time)
+define_show_state_ull_function(failed)
 define_show_state_str_function(name)
 define_show_state_str_function(desc)
 define_show_state_ull_function(disable)
@@ -307,6 +308,7 @@ define_one_state_ro(latency, show_state_exit_latency);
 define_one_state_ro(power, show_state_power_usage);
 define_one_state_ro(usage, show_state_usage);
 define_one_state_ro(time, show_state_time);
+define_one_state_ro(failed, show_state_failed);
 define_one_state_rw(disable, show_state_disable, store_state_disable);
 
 static struct attribute *cpuidle_state_default_attrs[] = {
@@ -316,6 +318,7 @@ static struct attribute *cpuidle_state_default_attrs[] = {
 	&attr_power.attr,
 	&attr_usage.attr,
 	&attr_time.attr,
+	&attr_failed.attr,
 	&attr_disable.attr,
 	NULL
 };
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 50fcbb0..bdad544 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -32,6 +32,7 @@ struct cpuidle_driver;
 struct cpuidle_state_usage {
 	unsigned long long	disable;
 	unsigned long long	usage;
+	unsigned long long	failed;
 	unsigned long long	time; /* in US */
 };
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] cpuidle: add 'failed' statistic
  2014-03-07  4:14 [PATCH] cpuidle: add 'failed' statistic Daniel Lezcano
@ 2014-03-11  2:00 ` Len Brown
  2014-03-11 21:52   ` Daniel Lezcano
  0 siblings, 1 reply; 3+ messages in thread
From: Len Brown @ 2014-03-11  2:00 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Rafael J. Wysocki, Linux PM list, linux-kernel@vger.kernel.org

Exactly what use-case do you have in mind for this attribute?

"failed" is a strong word.
Some validation guy is going to send me e-mail when it is non-zero...
I don't like that use-case.

But even if re-named, I don't see see how it will be useful.
When I want to see how C-state predictions are doing, I use ftrace,
which can show me the actual expected and actual times, not just a count
of how man times predicted was < actual.
I would rather see some good tracepoints go upstream.

thanks,
Len Brown, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] cpuidle: add 'failed' statistic
  2014-03-11  2:00 ` Len Brown
@ 2014-03-11 21:52   ` Daniel Lezcano
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Lezcano @ 2014-03-11 21:52 UTC (permalink / raw)
  To: Len Brown; +Cc: Rafael J. Wysocki, Linux PM list, linux-kernel@vger.kernel.org

On 03/11/2014 03:00 AM, Len Brown wrote:
> Exactly what use-case do you have in mind for this attribute?

Nothing more than balance the c-state usage with the selection 
efficiency of this state. The current statistics do not give a lot of 
clues of what is happening.

> "failed" is a strong word.
> Some validation guy is going to send me e-mail when it is non-zero...
> I don't like that use-case.
>
> But even if re-named, I don't see see how it will be useful.
> When I want to see how C-state predictions are doing, I use ftrace,
> which can show me the actual expected and actual times, not just a count
> of how man times predicted was < actual.

Mmh, where do you retrieve the target_residency from userspace ? This 
information is not exported from ftrace neither sysfs.

> I would rather see some good tracepoints go upstream.

Ok, which tracepoints you would like to see ?

target residency=%lu, expected residency=%lu, measured residency=%lu

??

Thanks
   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-03-11 21:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-07  4:14 [PATCH] cpuidle: add 'failed' statistic Daniel Lezcano
2014-03-11  2:00 ` Len Brown
2014-03-11 21:52   ` Daniel Lezcano

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).