cpuidle: add 'failed' statistic

Message ID 1394165650-6927-1-git-send-email-daniel.lezcano@linaro.org
State New
Headers show

Commit Message

Daniel Lezcano March 7, 2014, 4:14 a.m.
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(+)

Comments

Len Brown March 11, 2014, 2 a.m. | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Lezcano March 11, 2014, 9:52 p.m. | #2
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

Patch

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 */
 };