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

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

[Andreas Schwab]

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 <schwab@linux-m68k.org>
---
 drivers/cpufreq/cpufreq_conservative.c | 4 ++--
 drivers/cpufreq/cpufreq_ondemand.c     | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index a152af7..96af7d5 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -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)
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 396322f..6c19a66 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -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)
-- 
1.8.0


-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors

[Rafael J. Wysocki]

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 <schwab@linux-m68k.org>

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


> ---
>  drivers/cpufreq/cpufreq_conservative.c | 4 ++--
>  drivers/cpufreq/cpufreq_ondemand.c     | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index a152af7..96af7d5 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -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)
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index 396322f..6c19a66 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -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)
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors

[Andreas Schwab]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

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

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors

[Rafael J. Wysocki]

On Friday, October 26, 2012 02:26:34 PM Andreas Schwab wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > 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.

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors

[Rafael J. Wysocki]

On Friday, October 26, 2012 11:10:10 PM Rafael J. Wysocki wrote:
> On Friday, October 26, 2012 02:26:34 PM Andreas Schwab wrote:
> > "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> > 
> > > 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 <rafael.j.wysocki@intel.com>
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 <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq_governor.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux.orig/drivers/cpufreq/cpufreq_governor.c
+++ 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)


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors

[Viresh Kumar]

On 24 November 2012 03:18, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 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 <rafael.j.wysocki@intel.com>

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>