cpufreq-stats: fix time_in_state statistics

PATCH In review Andreas Schwab 2 participants 2 messages Aug 2, 2013 – Aug 5, 2013 CPU frequency scaling framework

lore

Share & tools

lore

Andreas Schwab

author

2013-08-02 18:31 UTC #0 lore raw

PATCH 1/1 cpufreq-stats: fix time_in_state statistics

The time_in_state numbers are in units of jiffies, not cputime.

Signed-off-by Andreas Schwab <[email protected]>

---
 drivers/cpufreq/cpufreq_stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/cpufreq/cpufreq_stats.c +1 -1

index d37568c..10e6138 100644
@@ -81,7 +81,7 @@ static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf)
 	for (i = 0; i < stat->state_num; i++) {
 		len += sprintf(buf + len, "%u %llu\n", stat->freq_table[i],
 			(unsigned long long)
-			cputime64_to_clock_t(stat->time_in_state[i]));
+			jiffies_64_to_clock_t(stat->time_in_state[i]));
 	}
 	return len;
 }

Signature · 6 lines

1.8.3.4

-- 
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Viresh Kumar

maintainer Acked-by on v1 1/1

2013-08-05 06:45 UTC #1 lore raw

Please see MAINTAINERS file to get list of people/lists you need to send your patch to.

Quoted 12 lines · Andreas Schwab

The time_in_state numbers are in units of jiffies, not cputime.
Signed-off-by Andreas Schwab <[email protected]>
---
 drivers/cpufreq/cpufreq_stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/cpufreq_stats.c +0 -0
index d37568c..10e6138 100644

drivers/cpufreq/cpufreq_stats.c static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf) · line 84

Quoted 5 lines

drivers/cpufreq/cpufreq_stats.c +1 -1

@@ -81,7 +81,7 @@ static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf)
        for (i = 0; i < stat->state_num; i++) {
                len += sprintf(buf + len, "%u %llu\n", stat->freq_table[i],
                        (unsigned long long)
-                       cputime64_to_clock_t(stat->time_in_state[i]));
+                       jiffies_64_to_clock_t(stat->time_in_state[i]));

I tried to go to the history a bit but really couldn't make out what was the real intention initially:

Following patches did this change:

- stat->time_in_state[stat->last_index] +=
- delta_time(stat->last_time, jiffies);
- stat->last_time = jiffies;
+ stat->time_in_state[stat->last_index] =
+ cputime64_add(stat->time_in_state[stat->last_index],
+ cputime_sub(cur_time, stat->last_time));

commit 58f1df25403988b73d7129fcd2c4d4c24017f1af

Author: Venkatesh Pallipadi <[email protected]>
Date:   Wed May 25 14:46:50 2005 -0700

[PATCH] cpufreq-stats driver updates

And it was then simplified by:

commit 648616343cdbe904c585a6c12e323d3b3c72e46f

Author: Martin Schwidefsky <[email protected]>
Date:   Thu Dec 15 14:56:09 2011 +0100

[S390] cputime: add sparse checking and cleanup

- stat->time_in_state[stat->last_index] =
- cputime64_add(stat->time_in_state[stat->last_index],
- cputime_sub(cur_time, stat->last_time));
+ stat->time_in_state[stat->last_index] +=
+ cur_time - stat->last_time;

And obviously cur_time is in jiffies... I am okay with your patch if Rafael doesn't see any issues with it...

Acked-by Viresh Kumar <[email protected]>