Skip to contentSkip to conversation

cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors

PATCH In review Andreas Schwab 3 participants 6 messages Oct 22, 2012 – Nov 24, 2012 CPU frequency scaling framework

lore
Share & tools
lore

Andreas Schwab

author
#0 lore raw

PATCH 1/1 cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors

The function get_cpu_idle_time_jiffy in both the conservative and ondemand governors use jiffies_to_usecs to convert a cputime value to usecs which gives the wrong value on architectures where cputime and jiffies use different units. Only matters if NO_HZ is disabled, since otherwise get_cpu_idle_time_us should already return a valid value, and get_cpu_idle_time_jiffy isn't actually called.

Signed-off-by Andreas Schwab <[email protected]>
---
 drivers/cpufreq/cpufreq_conservative.c | 4 ++--
 drivers/cpufreq/cpufreq_ondemand.c     | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/cpufreq/cpufreq_conservative.c +2 -2
index a152af7..96af7d5 100644
@@ -112,9 +112,9 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
 
 	idle_time = cur_wall_time - busy_time;
 	if (wall)
-		*wall = jiffies_to_usecs(cur_wall_time);
+		*wall = cputime_to_usecs(cur_wall_time);
 
-	return jiffies_to_usecs(idle_time);
+	return cputime_to_usecs(idle_time);
 }
 
 static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
drivers/cpufreq/cpufreq_ondemand.c +2 -2
index 396322f..6c19a66 100644
@@ -136,9 +136,9 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
 
 	idle_time = cur_wall_time - busy_time;
 	if (wall)
-		*wall = jiffies_to_usecs(cur_wall_time);
+		*wall = cputime_to_usecs(cur_wall_time);
 
-	return jiffies_to_usecs(idle_time);
+	return cputime_to_usecs(idle_time);
 }
 
 static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
Signature · 7 lines
1.8.0


-- 
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
#1 lore raw
Quoted 8 lines · On Monday 22 of October 2012 15:35:29 Andreas Schwab wrote:

The function get_cpu_idle_time_jiffy in both the conservative and ondemand governors use jiffies_to_usecs to convert a cputime value to usecs which gives the wrong value on architectures where cputime and jiffies use different units. Only matters if NO_HZ is disabled, since otherwise get_cpu_idle_time_us should already return a valid value, and get_cpu_idle_time_jiffy isn't actually called.

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

I have rebased your patch on top of some other changes in linux-pm.git/linux-next.

Please have a look at

http://git.kernel.org/?p=linux/kernel/git/rafael/linux-pm.git;a=shortlog;h=refs/heads/linux-next

and let me know if that's what you meant.

Thanks,
Rafael

Quoted 38 lines
---
 drivers/cpufreq/cpufreq_conservative.c | 4 ++--
 drivers/cpufreq/cpufreq_ondemand.c     | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/cpufreq/cpufreq_conservative.c +2 -2
index a152af7..96af7d5 100644
@@ -112,9 +112,9 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
 
 	idle_time = cur_wall_time - busy_time;
 	if (wall)
-		*wall = jiffies_to_usecs(cur_wall_time);
+		*wall = cputime_to_usecs(cur_wall_time);
 
-	return jiffies_to_usecs(idle_time);
+	return cputime_to_usecs(idle_time);
 }
 
 static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
drivers/cpufreq/cpufreq_ondemand.c +2 -2
index 396322f..6c19a66 100644
@@ -136,9 +136,9 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
 
 	idle_time = cur_wall_time - busy_time;
 	if (wall)
-		*wall = jiffies_to_usecs(cur_wall_time);
+		*wall = cputime_to_usecs(cur_wall_time);
 
-	return jiffies_to_usecs(idle_time);
+	return cputime_to_usecs(idle_time);
 }
 
 static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#4 lore raw
Quoted 8 lines · Rafael J. Wysocki
Quoted 5 lines · Andreas Schwab
Quoted 1 line · Rafael J. Wysocki

http://git.kernel.org/?p=linux/kernel/git/rafael/linux-pm.git;a=shortlog;h=refs/heads/linux-next

91f347c looks good, unfortunately d7d6f64 undoes the change.

I didn't notice that, sorry. Thanks for the heads up.

So, I'm going to apply the following patch to fix this again.

Thanks,
Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: cpufreq: governors: Fix jiffies/cputime mixup (revisited)

This change was made by commit 8636fd2 (cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors) before, but then it has been reverted inadvertently by commit 4471a34 (cpufreq: governors:
remove redundant code).

The changelog of commit 8636fd2 says:

The function get_cpu_idle_time_jiffy in both the conservative and ondemand governors use jiffies_to_usecs to convert a cputime value to usecs which gives the wrong value on architectures where cputime and jiffies use different units. Only matters if NO_HZ is disabled, since otherwise get_cpu_idle_time_us should already return a valid value, and get_cpu_idle_time_jiffy isn't actually called.

Since now we have only one common get_cpu_idle_time_jiffy() used by both governors in question, modify it along the lines of commit 8636fd2 to restore the correct behavior.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/cpufreq/cpufreq_governor.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/cpufreq/cpufreq_governor.c +2 -2
Index: linux/drivers/cpufreq/cpufreq_governor.c
===================================================================
@@ -45,9 +45,9 @@ static inline u64 get_cpu_idle_time_jiff
 
 	idle_time = cur_wall_time - busy_time;
 	if (wall)
-		*wall = jiffies_to_usecs(cur_wall_time);
+		*wall = cputime_to_usecs(cur_wall_time);
 
-	return jiffies_to_usecs(idle_time);
+	return cputime_to_usecs(idle_time);
 }
 
 u64 get_cpu_idle_time(unsigned int cpu, u64 *wall)
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
maintainer Reviewed-by
#5 lore raw
Quoted 18 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>
Subject: cpufreq: governors: Fix jiffies/cputime mixup (revisited)

This change was made by commit 8636fd2 (cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors) before, but then it has been reverted inadvertently by commit 4471a34 (cpufreq: governors:
remove redundant code).

The changelog of commit 8636fd2 says:

The function get_cpu_idle_time_jiffy in both the conservative and ondemand governors use jiffies_to_usecs to convert a cputime value to usecs which gives the wrong value on architectures where cputime and jiffies use different units. Only matters if NO_HZ is disabled, since otherwise get_cpu_idle_time_us should already return a valid value, and get_cpu_idle_time_jiffy isn't actually called.

Quoted 5 lines

Since now we have only one common get_cpu_idle_time_jiffy() used by both governors in question, modify it along the lines of commit 8636fd2 to restore the correct behavior.

Signed-off-by Rafael J. Wysocki <[email protected]>
Reviewed-by Viresh Kumar <[email protected]>