[RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[Stratos Karafotis]

[-- Attachment #1: Type: text/plain, Size: 6372 bytes --]

Ondemand increases frequency only if the load_freq is greater than
up_threshold. This seems to produce oscillations of frequency between
min and max because a relatively small load can easily saturate minimum
frequency and lead the CPU to max. Then, the CPU will decrease back to
min due to a small load_freq.

With this patch the frequency can be increased to any value,
proportional to load, if the load is below up_threshold. Thus, middle
frequencies are used more. Absolute load is used for the calculation of
frequency.

Phoronix benchmark results of Linux Kernel Compilation 3.1 tests are
attached with and without this patch. cpufreq_stats (time_in_state) are
also included. Tested on Intel i7-3770 CPU @ 3.40GH and on 
Quad core 1500 MHz Krait.

Signed-off-by: Stratos Karafotis <stratosk@semaphore.gr>
---
 drivers/cpufreq/cpufreq_governor.c | 10 +---------
 drivers/cpufreq/cpufreq_governor.h |  1 -
 drivers/cpufreq/cpufreq_ondemand.c | 39 ++++++++------------------------------
 3 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 5af40ad..eeab30a 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -97,7 +97,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 
 	policy = cdbs->cur_policy;
 
-	/* Get Absolute Load (in terms of freq for ondemand gov) */
+	/* Get Absolute Load */
 	for_each_cpu(j, policy->cpus) {
 		struct cpu_dbs_common_info *j_cdbs;
 		u64 cur_wall_time, cur_idle_time;
@@ -148,14 +148,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 
 		load = 100 * (wall_time - idle_time) / wall_time;
 
-		if (dbs_data->cdata->governor == GOV_ONDEMAND) {
-			int freq_avg = __cpufreq_driver_getavg(policy, j);
-			if (freq_avg <= 0)
-				freq_avg = policy->cur;
-
-			load *= freq_avg;
-		}
-
 		if (load > max_load)
 			max_load = load;
 	}
diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
index e16a961..e899c11 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -169,7 +169,6 @@ struct od_dbs_tuners {
 	unsigned int sampling_rate;
 	unsigned int sampling_down_factor;
 	unsigned int up_threshold;
-	unsigned int adj_up_threshold;
 	unsigned int powersave_bias;
 	unsigned int io_is_busy;
 };
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 4b9bb5d..bf2ae64 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -29,11 +29,9 @@
 #include "cpufreq_governor.h"
 
 /* On-demand governor macros */
-#define DEF_FREQUENCY_DOWN_DIFFERENTIAL		(10)
 #define DEF_FREQUENCY_UP_THRESHOLD		(80)
 #define DEF_SAMPLING_DOWN_FACTOR		(1)
 #define MAX_SAMPLING_DOWN_FACTOR		(100000)
-#define MICRO_FREQUENCY_DOWN_DIFFERENTIAL	(3)
 #define MICRO_FREQUENCY_UP_THRESHOLD		(95)
 #define MICRO_FREQUENCY_MIN_SAMPLE_RATE		(10000)
 #define MIN_FREQUENCY_UP_THRESHOLD		(11)
@@ -159,14 +157,12 @@ static void dbs_freq_increase(struct cpufreq_policy *p, unsigned int freq)
 
 /*
  * Every sampling_rate, we check, if current idle time is less than 20%
- * (default), then we try to increase frequency. Every sampling_rate, we look
- * for the lowest frequency which can sustain the load while keeping idle time
- * over 30%. If such a frequency exist, we try to decrease to this frequency.
+ * (default), then we try to increase frequency. Else, we adjust the frequency
+ * proportional to load.
  *
- * Any frequency increase takes it to the maximum frequency. Frequency reduction
- * happens at minimum steps of 5% (default) of current frequency
+ * Any frequency increase takes it to the maximum frequency.
  */
-static void od_check_cpu(int cpu, unsigned int load_freq)
+static void od_check_cpu(int cpu, unsigned int load)
 {
 	struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
 	struct cpufreq_policy *policy = dbs_info->cdbs.cur_policy;
@@ -176,29 +172,17 @@ static void od_check_cpu(int cpu, unsigned int load_freq)
 	dbs_info->freq_lo = 0;
 
 	/* Check for frequency increase */
-	if (load_freq > od_tuners->up_threshold * policy->cur) {
+	if (load > od_tuners->up_threshold) {
 		/* If switching to max speed, apply sampling_down_factor */
 		if (policy->cur < policy->max)
 			dbs_info->rate_mult =
 				od_tuners->sampling_down_factor;
 		dbs_freq_increase(policy, policy->max);
 		return;
-	}
-
-	/* Check for frequency decrease */
-	/* if we cannot reduce the frequency anymore, break out early */
-	if (policy->cur == policy->min)
-		return;
-
-	/*
-	 * The optimal frequency is the frequency that is the lowest that can
-	 * support the current CPU usage without triggering the up policy. To be
-	 * safe, we focus 10 points under the threshold.
-	 */
-	if (load_freq < od_tuners->adj_up_threshold
-			* policy->cur) {
+	} else {
+		/* Calculate the next frequency proportional to load */
 		unsigned int freq_next;
-		freq_next = load_freq / od_tuners->adj_up_threshold;
+		freq_next = load * policy->max / 100;
 
 		/* No longer fully busy, reset rate_mult */
 		dbs_info->rate_mult = 1;
@@ -372,9 +356,6 @@ static ssize_t store_up_threshold(struct dbs_data *dbs_data, const char *buf,
 			input < MIN_FREQUENCY_UP_THRESHOLD) {
 		return -EINVAL;
 	}
-	/* Calculate the new adj_up_threshold */
-	od_tuners->adj_up_threshold += input;
-	od_tuners->adj_up_threshold -= od_tuners->up_threshold;
 
 	od_tuners->up_threshold = input;
 	return count;
@@ -523,8 +504,6 @@ static int od_init(struct dbs_data *dbs_data)
 	if (idle_time != -1ULL) {
 		/* Idle micro accounting is supported. Use finer thresholds */
 		tuners->up_threshold = MICRO_FREQUENCY_UP_THRESHOLD;
-		tuners->adj_up_threshold = MICRO_FREQUENCY_UP_THRESHOLD -
-			MICRO_FREQUENCY_DOWN_DIFFERENTIAL;
 		/*
 		 * In nohz/micro accounting case we set the minimum frequency
 		 * not depending on HZ, but fixed (very low). The deferred
@@ -533,8 +512,6 @@ static int od_init(struct dbs_data *dbs_data)
 		dbs_data->min_sampling_rate = MICRO_FREQUENCY_MIN_SAMPLE_RATE;
 	} else {
 		tuners->up_threshold = DEF_FREQUENCY_UP_THRESHOLD;
-		tuners->adj_up_threshold = DEF_FREQUENCY_UP_THRESHOLD -
-			DEF_FREQUENCY_DOWN_DIFFERENTIAL;
 
 		/* For correct statistics, we need 10 ticks for each measure */
 		dbs_data->min_sampling_rate = MIN_SAMPLING_RATE_RATIO *
-- 
1.8.1.4


[-- Attachment #2: test_results_with --]
[-- Type: text/plain, Size: 1154 bytes --]

Timed Linux Kernel Compilation 3.1:
    pts/build-linux-kernel-1.3.0
    Test 1 of 1
    Estimated Trial Run Count:    3
    Estimated Time To Completion: 2 Minutes
        Running Pre-Test Script @ 22:39:56
        Started Run 1 @ 22:40:06
        Running Interim Test Script @ 22:40:20
        Started Run 2 @ 22:40:23
        Running Interim Test Script @ 22:40:33
        Started Run 3 @ 22:40:36
        Running Interim Test Script @ 22:40:46  [Std. Dev: 3.99%]
        Started Run 4 @ 22:40:49
        Running Interim Test Script @ 22:40:59  [Std. Dev: 3.73%]
        Started Run 5 @ 22:41:02
        Running Interim Test Script @ 22:41:12  [Std. Dev: 3.54%]
        Started Run 6 @ 22:41:15  [Std. Dev: 3.34%]
        Running Post-Test Script @ 22:41:25

    Test Results:
        10.175578832626
        9.4103870391846
        9.668200969696
        9.4109489917755
        9.3668100833893
        9.3667180538177

    Average: 9.57 Seconds


cpufreq_stats time_in_state

3401000 3340
3400000 0
3300000 27
3100000 17
3000000 14
2900000 8
2800000 17
2600000 7
2500000 8
2400000 10
2200000 5
2100000 7
2000000 8
1900000 28
1700000 5
1600000 5854


[-- Attachment #3: test_results_without --]
[-- Type: text/plain, Size: 1145 bytes --]

Linux Kernel Compilation 3.1:
    pts/build-linux-kernel-1.3.0
    Test 1 of 1
    Estimated Trial Run Count:    3
    Estimated Time To Completion: 3 Minutes
        Running Pre-Test Script @ 22:28:26
        Started Run 1 @ 22:28:38
        Running Interim Test Script @ 22:28:51
        Started Run 2 @ 22:28:55
        Running Interim Test Script @ 22:29:05
        Started Run 3 @ 22:29:10
        Running Interim Test Script @ 22:29:20  [Std. Dev: 5.10%]
        Started Run 4 @ 22:29:23
        Running Interim Test Script @ 22:29:33  [Std. Dev: 4.73%]
        Started Run 5 @ 22:29:37
        Running Interim Test Script @ 22:29:46  [Std. Dev: 4.23%]
        Started Run 6 @ 22:29:50  [Std. Dev: 4.01%]
        Running Post-Test Script @ 22:30:00

    Test Results:
        10.521644830704
        9.7866048812866
        9.5475811958313
        9.5281581878662
        9.6297731399536
        9.497542142868

    Average: 9.75 Seconds


cpufreq_stats time_in_state

3401000 3594
3400000 3
3300000 10
3100000 6
3000000 3
2900000 5
2800000 7
2600000 3
2500000 2
2400000 18
2200000 6
2100000 1
2000000 2
1900000 64
1700000 2
1600000 6522

Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[Rafael J. Wysocki]

On Monday, May 27, 2013 11:49:19 PM Stratos Karafotis wrote:
> Ondemand increases frequency only if the load_freq is greater than
> up_threshold. This seems to produce oscillations of frequency between
> min and max because a relatively small load can easily saturate minimum
> frequency and lead the CPU to max. Then, the CPU will decrease back to
> min due to a small load_freq.

I think this is a correct observation.

> With this patch the frequency can be increased to any value,

What exactly does "any value" mean here?

> proportional to load, if the load is below up_threshold. Thus, middle
> frequencies are used more. Absolute load is used for the calculation of
> frequency.
> 
> Phoronix benchmark results of Linux Kernel Compilation 3.1 tests are
> attached with and without this patch. cpufreq_stats (time_in_state) are
> also included. Tested on Intel i7-3770 CPU @ 3.40GH and on 
> Quad core 1500 MHz Krait.

Can you please comment the results in the changelog?  Attachments don't
show up in git logs. :-)

> Signed-off-by: Stratos Karafotis <stratosk@semaphore.gr>
> ---
>  drivers/cpufreq/cpufreq_governor.c | 10 +---------
>  drivers/cpufreq/cpufreq_governor.h |  1 -
>  drivers/cpufreq/cpufreq_ondemand.c | 39 ++++++++------------------------------
>  3 files changed, 9 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index 5af40ad..eeab30a 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -97,7 +97,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
>  
>  	policy = cdbs->cur_policy;
>  
> -	/* Get Absolute Load (in terms of freq for ondemand gov) */
> +	/* Get Absolute Load */
>  	for_each_cpu(j, policy->cpus) {
>  		struct cpu_dbs_common_info *j_cdbs;
>  		u64 cur_wall_time, cur_idle_time;
> @@ -148,14 +148,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
>  
>  		load = 100 * (wall_time - idle_time) / wall_time;
>  
> -		if (dbs_data->cdata->governor == GOV_ONDEMAND) {
> -			int freq_avg = __cpufreq_driver_getavg(policy, j);
> -			if (freq_avg <= 0)
> -				freq_avg = policy->cur;
> -
> -			load *= freq_avg;
> -		}
> -
>  		if (load > max_load)
>  			max_load = load;
>  	}
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index e16a961..e899c11 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -169,7 +169,6 @@ struct od_dbs_tuners {
>  	unsigned int sampling_rate;
>  	unsigned int sampling_down_factor;
>  	unsigned int up_threshold;
> -	unsigned int adj_up_threshold;
>  	unsigned int powersave_bias;
>  	unsigned int io_is_busy;
>  };
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index 4b9bb5d..bf2ae64 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -29,11 +29,9 @@
>  #include "cpufreq_governor.h"
>  
>  /* On-demand governor macros */
> -#define DEF_FREQUENCY_DOWN_DIFFERENTIAL		(10)
>  #define DEF_FREQUENCY_UP_THRESHOLD		(80)
>  #define DEF_SAMPLING_DOWN_FACTOR		(1)
>  #define MAX_SAMPLING_DOWN_FACTOR		(100000)
> -#define MICRO_FREQUENCY_DOWN_DIFFERENTIAL	(3)
>  #define MICRO_FREQUENCY_UP_THRESHOLD		(95)
>  #define MICRO_FREQUENCY_MIN_SAMPLE_RATE		(10000)
>  #define MIN_FREQUENCY_UP_THRESHOLD		(11)
> @@ -159,14 +157,12 @@ static void dbs_freq_increase(struct cpufreq_policy *p, unsigned int freq)
>  
>  /*
>   * Every sampling_rate, we check, if current idle time is less than 20%
> - * (default), then we try to increase frequency. Every sampling_rate, we look
> - * for the lowest frequency which can sustain the load while keeping idle time
> - * over 30%. If such a frequency exist, we try to decrease to this frequency.
> + * (default), then we try to increase frequency. Else, we adjust the frequency
> + * proportional to load.
>   *
> - * Any frequency increase takes it to the maximum frequency. Frequency reduction
> - * happens at minimum steps of 5% (default) of current frequency
> + * Any frequency increase takes it to the maximum frequency.
>   */
> -static void od_check_cpu(int cpu, unsigned int load_freq)
> +static void od_check_cpu(int cpu, unsigned int load)
>  {
>  	struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
>  	struct cpufreq_policy *policy = dbs_info->cdbs.cur_policy;
> @@ -176,29 +172,17 @@ static void od_check_cpu(int cpu, unsigned int load_freq)
>  	dbs_info->freq_lo = 0;
>  
>  	/* Check for frequency increase */
> -	if (load_freq > od_tuners->up_threshold * policy->cur) {
> +	if (load > od_tuners->up_threshold) {
>  		/* If switching to max speed, apply sampling_down_factor */
>  		if (policy->cur < policy->max)
>  			dbs_info->rate_mult =
>  				od_tuners->sampling_down_factor;
>  		dbs_freq_increase(policy, policy->max);
>  		return;
> -	}
> -
> -	/* Check for frequency decrease */
> -	/* if we cannot reduce the frequency anymore, break out early */
> -	if (policy->cur == policy->min)
> -		return;
> -
> -	/*
> -	 * The optimal frequency is the frequency that is the lowest that can
> -	 * support the current CPU usage without triggering the up policy. To be
> -	 * safe, we focus 10 points under the threshold.
> -	 */
> -	if (load_freq < od_tuners->adj_up_threshold
> -			* policy->cur) {
> +	} else {
> +		/* Calculate the next frequency proportional to load */
>  		unsigned int freq_next;
> -		freq_next = load_freq / od_tuners->adj_up_threshold;
> +		freq_next = load * policy->max / 100;

Can you please explain why this is the right formula?

>  		/* No longer fully busy, reset rate_mult */
>  		dbs_info->rate_mult = 1;
> @@ -372,9 +356,6 @@ static ssize_t store_up_threshold(struct dbs_data *dbs_data, const char *buf,
>  			input < MIN_FREQUENCY_UP_THRESHOLD) {
>  		return -EINVAL;
>  	}
> -	/* Calculate the new adj_up_threshold */
> -	od_tuners->adj_up_threshold += input;
> -	od_tuners->adj_up_threshold -= od_tuners->up_threshold;
>  
>  	od_tuners->up_threshold = input;
>  	return count;
> @@ -523,8 +504,6 @@ static int od_init(struct dbs_data *dbs_data)
>  	if (idle_time != -1ULL) {
>  		/* Idle micro accounting is supported. Use finer thresholds */
>  		tuners->up_threshold = MICRO_FREQUENCY_UP_THRESHOLD;
> -		tuners->adj_up_threshold = MICRO_FREQUENCY_UP_THRESHOLD -
> -			MICRO_FREQUENCY_DOWN_DIFFERENTIAL;
>  		/*
>  		 * In nohz/micro accounting case we set the minimum frequency
>  		 * not depending on HZ, but fixed (very low). The deferred
> @@ -533,8 +512,6 @@ static int od_init(struct dbs_data *dbs_data)
>  		dbs_data->min_sampling_rate = MICRO_FREQUENCY_MIN_SAMPLE_RATE;
>  	} else {
>  		tuners->up_threshold = DEF_FREQUENCY_UP_THRESHOLD;
> -		tuners->adj_up_threshold = DEF_FREQUENCY_UP_THRESHOLD -
> -			DEF_FREQUENCY_DOWN_DIFFERENTIAL;
>  
>  		/* For correct statistics, we need 10 ticks for each measure */
>  		dbs_data->min_sampling_rate = MIN_SAMPLING_RATE_RATIO *

Overall it looks like an improvement.

Thanks,
Rafael


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

Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[Stratos Karafotis]

Hi Rafael,

Thank you for your prompt reply and your comments!

On 05/28/2013 02:43 PM, Rafael J. Wysocki wrote:
>> With this patch the frequency can be increased to any value,
> 
> What exactly does "any value" mean here?
> 

I mean any value of freq table. Please let me know if you want me to rephrase
it in description.

> Can you please comment the results in the changelog?  Attachments don't
> show up in git logs. :-)

I'm sorry, you are right. I added comments in the patch description.

> 
> Can you please explain why this is the right formula?
> 


Without this patch, we compare load_freq with up_threshold to decide about
the max frequency.
	load_freq = load * freq_avg

In most cpufreq drivers getavg function is not implemented (I found that
it's implemented only in acpi-cpufreq). Therefore:
	freq_avg = policy->cur. 

Thus, in the comparison with up_threshold to increase frequency we actually
do this (in cases that getavg is not implemented):
if (load > up_theshold)
	increase to max

So, after the patch we keep the same comparison to decide about the max frequency.
I thought, that below up_threshold is 'fair' to decide about the next 
frequency with formula that frequency is proportional to load.
For example in a CPU with min freq 100MHz and max 1000MHz with a 
load 50 target frequency should be 500MHz. 


Thanks,
Stratos

-----------------------8<---------------------------------------------
Ondemand increases frequency only if the load_freq is greater than
up_threshold. This seems to produce oscillations of frequency between
min and max because a relatively small load can easily saturate minimum
frequency and lead the CPU to max. Then, the CPU will decrease back to
min due to a small load_freq.

With this patch the frequency can be increased to any value,
proportional to load, if the load is below up_threshold. Thus, middle
frequencies are used more. Absolute load is used for the calculation of
frequency.

Tested on Intel i7-3770 CPU @ 3.40GHz and on Quad core 1500MHz Krait.
Phoronix benchmark of Linux Kernel Compilation 3.1 test shows an
increase 1.5% to performance. cpufreq_stats (time_in_state) shows
that middle frequencies are used more, with this patch. Highest 
and lowest frequencies were used less by ~9% 

Signed-off-by: Stratos Karafotis <stratosk@semaphore.gr>
---
 drivers/cpufreq/cpufreq_governor.c | 10 +---------
 drivers/cpufreq/cpufreq_governor.h |  1 -
 drivers/cpufreq/cpufreq_ondemand.c | 39 +++++++-------------------------------
 3 files changed, 8 insertions(+), 42 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 5af40ad..eeab30a 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -97,7 +97,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 
 	policy = cdbs->cur_policy;
 
-	/* Get Absolute Load (in terms of freq for ondemand gov) */
+	/* Get Absolute Load */
 	for_each_cpu(j, policy->cpus) {
 		struct cpu_dbs_common_info *j_cdbs;
 		u64 cur_wall_time, cur_idle_time;
@@ -148,14 +148,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 
 		load = 100 * (wall_time - idle_time) / wall_time;
 
-		if (dbs_data->cdata->governor == GOV_ONDEMAND) {
-			int freq_avg = __cpufreq_driver_getavg(policy, j);
-			if (freq_avg <= 0)
-				freq_avg = policy->cur;
-
-			load *= freq_avg;
-		}
-
 		if (load > max_load)
 			max_load = load;
 	}
diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
index e16a961..e899c11 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -169,7 +169,6 @@ struct od_dbs_tuners {
 	unsigned int sampling_rate;
 	unsigned int sampling_down_factor;
 	unsigned int up_threshold;
-	unsigned int adj_up_threshold;
 	unsigned int powersave_bias;
 	unsigned int io_is_busy;
 };
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 4b9bb5d..c1e6d3e 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -29,11 +29,9 @@
 #include "cpufreq_governor.h"
 
 /* On-demand governor macros */
-#define DEF_FREQUENCY_DOWN_DIFFERENTIAL		(10)
 #define DEF_FREQUENCY_UP_THRESHOLD		(80)
 #define DEF_SAMPLING_DOWN_FACTOR		(1)
 #define MAX_SAMPLING_DOWN_FACTOR		(100000)
-#define MICRO_FREQUENCY_DOWN_DIFFERENTIAL	(3)
 #define MICRO_FREQUENCY_UP_THRESHOLD		(95)
 #define MICRO_FREQUENCY_MIN_SAMPLE_RATE		(10000)
 #define MIN_FREQUENCY_UP_THRESHOLD		(11)
@@ -159,14 +157,10 @@ static void dbs_freq_increase(struct cpufreq_policy *p, unsigned int freq)
 
 /*
  * Every sampling_rate, we check, if current idle time is less than 20%
- * (default), then we try to increase frequency. Every sampling_rate, we look
- * for the lowest frequency which can sustain the load while keeping idle time
- * over 30%. If such a frequency exist, we try to decrease to this frequency.
- *
- * Any frequency increase takes it to the maximum frequency. Frequency reduction
- * happens at minimum steps of 5% (default) of current frequency
+ * (default), then we try to increase frequency. Else, we adjust the frequency
+ * proportional to load.
  */
-static void od_check_cpu(int cpu, unsigned int load_freq)
+static void od_check_cpu(int cpu, unsigned int load)
 {
 	struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
 	struct cpufreq_policy *policy = dbs_info->cdbs.cur_policy;
@@ -176,29 +170,17 @@ static void od_check_cpu(int cpu, unsigned int load_freq)
 	dbs_info->freq_lo = 0;
 
 	/* Check for frequency increase */
-	if (load_freq > od_tuners->up_threshold * policy->cur) {
+	if (load > od_tuners->up_threshold) {
 		/* If switching to max speed, apply sampling_down_factor */
 		if (policy->cur < policy->max)
 			dbs_info->rate_mult =
 				od_tuners->sampling_down_factor;
 		dbs_freq_increase(policy, policy->max);
 		return;
-	}
-
-	/* Check for frequency decrease */
-	/* if we cannot reduce the frequency anymore, break out early */
-	if (policy->cur == policy->min)
-		return;
-
-	/*
-	 * The optimal frequency is the frequency that is the lowest that can
-	 * support the current CPU usage without triggering the up policy. To be
-	 * safe, we focus 10 points under the threshold.
-	 */
-	if (load_freq < od_tuners->adj_up_threshold
-			* policy->cur) {
+	} else {
+		/* Calculate the next frequency proportional to load */
 		unsigned int freq_next;
-		freq_next = load_freq / od_tuners->adj_up_threshold;
+		freq_next = load * policy->max / 100;
 
 		/* No longer fully busy, reset rate_mult */
 		dbs_info->rate_mult = 1;
@@ -372,9 +354,6 @@ static ssize_t store_up_threshold(struct dbs_data *dbs_data, const char *buf,
 			input < MIN_FREQUENCY_UP_THRESHOLD) {
 		return -EINVAL;
 	}
-	/* Calculate the new adj_up_threshold */
-	od_tuners->adj_up_threshold += input;
-	od_tuners->adj_up_threshold -= od_tuners->up_threshold;
 
 	od_tuners->up_threshold = input;
 	return count;
@@ -523,8 +502,6 @@ static int od_init(struct dbs_data *dbs_data)
 	if (idle_time != -1ULL) {
 		/* Idle micro accounting is supported. Use finer thresholds */
 		tuners->up_threshold = MICRO_FREQUENCY_UP_THRESHOLD;
-		tuners->adj_up_threshold = MICRO_FREQUENCY_UP_THRESHOLD -
-			MICRO_FREQUENCY_DOWN_DIFFERENTIAL;
 		/*
 		 * In nohz/micro accounting case we set the minimum frequency
 		 * not depending on HZ, but fixed (very low). The deferred
@@ -533,8 +510,6 @@ static int od_init(struct dbs_data *dbs_data)
 		dbs_data->min_sampling_rate = MICRO_FREQUENCY_MIN_SAMPLE_RATE;
 	} else {
 		tuners->up_threshold = DEF_FREQUENCY_UP_THRESHOLD;
-		tuners->adj_up_threshold = DEF_FREQUENCY_UP_THRESHOLD -
-			DEF_FREQUENCY_DOWN_DIFFERENTIAL;
 
 		/* For correct statistics, we need 10 ticks for each measure */
 		dbs_data->min_sampling_rate = MIN_SAMPLING_RATE_RATIO *
-- 
1.8.1.4

Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[Rafael J. Wysocki]

On Tuesday, May 28, 2013 08:03:19 PM Stratos Karafotis wrote:
> Hi Rafael,
> 
> Thank you for your prompt reply and your comments!
> 
> On 05/28/2013 02:43 PM, Rafael J. Wysocki wrote:
> >> With this patch the frequency can be increased to any value,
> > 
> > What exactly does "any value" mean here?
> > 
> 
> I mean any value of freq table. Please let me know if you want me to rephrase
> it in description.

Yes, it would be nice to be more precise.

> > Can you please comment the results in the changelog?  Attachments don't
> > show up in git logs. :-)
> 
> I'm sorry, you are right. I added comments in the patch description.
> 
> > 
> > Can you please explain why this is the right formula?
> > 
> 
> 
> Without this patch, we compare load_freq with up_threshold to decide about
> the max frequency.
> 	load_freq = load * freq_avg
> 
> In most cpufreq drivers getavg function is not implemented (I found that
> it's implemented only in acpi-cpufreq). Therefore:
> 	freq_avg = policy->cur. 

Which is equivalent to saying that __cpufreq_driver_getavg() is not useful and
may be removed (but the patch doesn't do that and I wonder why?), but surely
the developer who added it wouldn't agree.

So, please explain: why can we drop __cpufreq_driver_getavg()?

> Thus, in the comparison with up_threshold to increase frequency we actually
> do this (in cases that getavg is not implemented):
> if (load > up_theshold)
> 	increase to max
> 
> So, after the patch we keep the same comparison to decide about the max frequency.
> I thought, that below up_threshold is 'fair' to decide about the next 
> frequency with formula that frequency is proportional to load.
> For example in a CPU with min freq 100MHz and max 1000MHz with a 
> load 50 target frequency should be 500MHz. 

Well, that sounds reasonable, but the patch actually does more than that.

Thanks,
Rafael


> -----------------------8<---------------------------------------------
> Ondemand increases frequency only if the load_freq is greater than
> up_threshold. This seems to produce oscillations of frequency between
> min and max because a relatively small load can easily saturate minimum
> frequency and lead the CPU to max. Then, the CPU will decrease back to
> min due to a small load_freq.
> 
> With this patch the frequency can be increased to any value,
> proportional to load, if the load is below up_threshold. Thus, middle
> frequencies are used more. Absolute load is used for the calculation of
> frequency.
> 
> Tested on Intel i7-3770 CPU @ 3.40GHz and on Quad core 1500MHz Krait.
> Phoronix benchmark of Linux Kernel Compilation 3.1 test shows an
> increase 1.5% to performance. cpufreq_stats (time_in_state) shows
> that middle frequencies are used more, with this patch. Highest 
> and lowest frequencies were used less by ~9% 
> 
> Signed-off-by: Stratos Karafotis <stratosk@semaphore.gr>
> ---
>  drivers/cpufreq/cpufreq_governor.c | 10 +---------
>  drivers/cpufreq/cpufreq_governor.h |  1 -
>  drivers/cpufreq/cpufreq_ondemand.c | 39 +++++++-------------------------------
>  3 files changed, 8 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index 5af40ad..eeab30a 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -97,7 +97,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
>  
>  	policy = cdbs->cur_policy;
>  
> -	/* Get Absolute Load (in terms of freq for ondemand gov) */
> +	/* Get Absolute Load */
>  	for_each_cpu(j, policy->cpus) {
>  		struct cpu_dbs_common_info *j_cdbs;
>  		u64 cur_wall_time, cur_idle_time;
> @@ -148,14 +148,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
>  
>  		load = 100 * (wall_time - idle_time) / wall_time;
>  
> -		if (dbs_data->cdata->governor == GOV_ONDEMAND) {
> -			int freq_avg = __cpufreq_driver_getavg(policy, j);
> -			if (freq_avg <= 0)
> -				freq_avg = policy->cur;
> -
> -			load *= freq_avg;
> -		}
> -
>  		if (load > max_load)
>  			max_load = load;
>  	}
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index e16a961..e899c11 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -169,7 +169,6 @@ struct od_dbs_tuners {
>  	unsigned int sampling_rate;
>  	unsigned int sampling_down_factor;
>  	unsigned int up_threshold;
> -	unsigned int adj_up_threshold;
>  	unsigned int powersave_bias;
>  	unsigned int io_is_busy;
>  };
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index 4b9bb5d..c1e6d3e 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -29,11 +29,9 @@
>  #include "cpufreq_governor.h"
>  
>  /* On-demand governor macros */
> -#define DEF_FREQUENCY_DOWN_DIFFERENTIAL		(10)
>  #define DEF_FREQUENCY_UP_THRESHOLD		(80)
>  #define DEF_SAMPLING_DOWN_FACTOR		(1)
>  #define MAX_SAMPLING_DOWN_FACTOR		(100000)
> -#define MICRO_FREQUENCY_DOWN_DIFFERENTIAL	(3)
>  #define MICRO_FREQUENCY_UP_THRESHOLD		(95)
>  #define MICRO_FREQUENCY_MIN_SAMPLE_RATE		(10000)
>  #define MIN_FREQUENCY_UP_THRESHOLD		(11)
> @@ -159,14 +157,10 @@ static void dbs_freq_increase(struct cpufreq_policy *p, unsigned int freq)
>  
>  /*
>   * Every sampling_rate, we check, if current idle time is less than 20%
> - * (default), then we try to increase frequency. Every sampling_rate, we look
> - * for the lowest frequency which can sustain the load while keeping idle time
> - * over 30%. If such a frequency exist, we try to decrease to this frequency.
> - *
> - * Any frequency increase takes it to the maximum frequency. Frequency reduction
> - * happens at minimum steps of 5% (default) of current frequency
> + * (default), then we try to increase frequency. Else, we adjust the frequency
> + * proportional to load.
>   */
> -static void od_check_cpu(int cpu, unsigned int load_freq)
> +static void od_check_cpu(int cpu, unsigned int load)
>  {
>  	struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
>  	struct cpufreq_policy *policy = dbs_info->cdbs.cur_policy;
> @@ -176,29 +170,17 @@ static void od_check_cpu(int cpu, unsigned int load_freq)
>  	dbs_info->freq_lo = 0;
>  
>  	/* Check for frequency increase */
> -	if (load_freq > od_tuners->up_threshold * policy->cur) {
> +	if (load > od_tuners->up_threshold) {
>  		/* If switching to max speed, apply sampling_down_factor */
>  		if (policy->cur < policy->max)
>  			dbs_info->rate_mult =
>  				od_tuners->sampling_down_factor;
>  		dbs_freq_increase(policy, policy->max);
>  		return;
> -	}
> -
> -	/* Check for frequency decrease */
> -	/* if we cannot reduce the frequency anymore, break out early */
> -	if (policy->cur == policy->min)
> -		return;
> -
> -	/*
> -	 * The optimal frequency is the frequency that is the lowest that can
> -	 * support the current CPU usage without triggering the up policy. To be
> -	 * safe, we focus 10 points under the threshold.
> -	 */
> -	if (load_freq < od_tuners->adj_up_threshold
> -			* policy->cur) {
> +	} else {
> +		/* Calculate the next frequency proportional to load */
>  		unsigned int freq_next;
> -		freq_next = load_freq / od_tuners->adj_up_threshold;
> +		freq_next = load * policy->max / 100;
>  
>  		/* No longer fully busy, reset rate_mult */
>  		dbs_info->rate_mult = 1;
> @@ -372,9 +354,6 @@ static ssize_t store_up_threshold(struct dbs_data *dbs_data, const char *buf,
>  			input < MIN_FREQUENCY_UP_THRESHOLD) {
>  		return -EINVAL;
>  	}
> -	/* Calculate the new adj_up_threshold */
> -	od_tuners->adj_up_threshold += input;
> -	od_tuners->adj_up_threshold -= od_tuners->up_threshold;
>  
>  	od_tuners->up_threshold = input;
>  	return count;
> @@ -523,8 +502,6 @@ static int od_init(struct dbs_data *dbs_data)
>  	if (idle_time != -1ULL) {
>  		/* Idle micro accounting is supported. Use finer thresholds */
>  		tuners->up_threshold = MICRO_FREQUENCY_UP_THRESHOLD;
> -		tuners->adj_up_threshold = MICRO_FREQUENCY_UP_THRESHOLD -
> -			MICRO_FREQUENCY_DOWN_DIFFERENTIAL;
>  		/*
>  		 * In nohz/micro accounting case we set the minimum frequency
>  		 * not depending on HZ, but fixed (very low). The deferred
> @@ -533,8 +510,6 @@ static int od_init(struct dbs_data *dbs_data)
>  		dbs_data->min_sampling_rate = MICRO_FREQUENCY_MIN_SAMPLE_RATE;
>  	} else {
>  		tuners->up_threshold = DEF_FREQUENCY_UP_THRESHOLD;
> -		tuners->adj_up_threshold = DEF_FREQUENCY_UP_THRESHOLD -
> -			DEF_FREQUENCY_DOWN_DIFFERENTIAL;
>  
>  		/* For correct statistics, we need 10 ticks for each measure */
>  		dbs_data->min_sampling_rate = MIN_SAMPLING_RATE_RATIO *
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[Stratos Karafotis]

On 05/28/2013 11:54 PM, Rafael J. Wysocki wrote:
> On Tuesday, May 28, 2013 08:03:19 PM Stratos Karafotis wrote:
>> I mean any value of freq table. Please let me know if you want me to rephrase
>> it in description.
> 
> Yes, it would be nice to be more precise.

OK sure, I will add a more precise description.


> Which is equivalent to saying that __cpufreq_driver_getavg() is not useful and
> may be removed (but the patch doesn't do that and I wonder why?), but surely
> the developer who added it wouldn't agree.
> 
> So, please explain: why can we drop __cpufreq_driver_getavg()?
> 

With the new proposed method the next frequency is not dependent from current
or average frequency. The next frequency is dependent only from load.
So, we don't need support for freq feedback from hardware anymore.
Even if, due to underlying hardware coordination mechanism, CPU runs in different 
frequency than the actual, the calculation of load and of target frequency will
remain the unaffected, with this patch.

With full respect to ondemand coauthor, and if the new method is acceptable,
I could send a patch to revert the original one about the IA32_APERF and
IA32_MPERF MSR support.


>> Thus, in the comparison with up_threshold to increase frequency we actually
>> do this (in cases that getavg is not implemented):
>> if (load > up_theshold)
>> 	increase to max
>>
>> So, after the patch we keep the same comparison to decide about the max frequency.
>> I thought, that below up_threshold is 'fair' to decide about the next
>> frequency with formula that frequency is proportional to load.
>> For example in a CPU with min freq 100MHz and max 1000MHz with a
>> load 50 target frequency should be 500MHz.
> 
> Well, that sounds reasonable, but the patch actually does more than that.

I'm sorry, but I didn't understand your last point.


Thanks for your comments,
Stratos

Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[Rafael J. Wysocki]

On Wednesday, May 29, 2013 06:15:56 PM Stratos Karafotis wrote:
> On 05/28/2013 11:54 PM, Rafael J. Wysocki wrote:
> > On Tuesday, May 28, 2013 08:03:19 PM Stratos Karafotis wrote:
> >> I mean any value of freq table. Please let me know if you want me to rephrase
> >> it in description.
> > 
> > Yes, it would be nice to be more precise.
> 
> OK sure, I will add a more precise description.
> 
> 
> > Which is equivalent to saying that __cpufreq_driver_getavg() is not useful and
> > may be removed (but the patch doesn't do that and I wonder why?), but surely
> > the developer who added it wouldn't agree.
> > 
> > So, please explain: why can we drop __cpufreq_driver_getavg()?
> > 
> 
> With the new proposed method the next frequency is not dependent from current
> or average frequency. The next frequency is dependent only from load.
> So, we don't need support for freq feedback from hardware anymore.

OK, but that's a more significant change than the changelog suggests.
The changelog should tell the whole story and explain the rationale. :-)

So, please explain that in fact it is not necessary to use the current or
average frequency to compute the target and why that is the case.

Also the patch should remove __cpufreq_driver_getavg() and the callback used by
it, since that code will be dead after applying it anyway.

> Even if, due to underlying hardware coordination mechanism, CPU runs in different 
> frequency than the actual, the calculation of load and of target frequency will
> remain the unaffected, with this patch.
> 
> With full respect to ondemand coauthor, and if the new method is acceptable,
> I could send a patch to revert the original one about the IA32_APERF and
> IA32_MPERF MSR support.

I'm not sure what you mean by "revert", but please do as I said above.

> >> Thus, in the comparison with up_threshold to increase frequency we actually
> >> do this (in cases that getavg is not implemented):
> >> if (load > up_theshold)
> >> 	increase to max
> >>
> >> So, after the patch we keep the same comparison to decide about the max frequency.
> >> I thought, that below up_threshold is 'fair' to decide about the next
> >> frequency with formula that frequency is proportional to load.
> >> For example in a CPU with min freq 100MHz and max 1000MHz with a
> >> load 50 target frequency should be 500MHz.
> > 
> > Well, that sounds reasonable, but the patch actually does more than that.
> 
> I'm sorry, but I didn't understand your last point.

Please see above.

The changelog doesn't even mention that the code is being switched from using
measured past frequencies to not using them, because you think that there's a
better way of computing the target (which by the way I can agree with :-)).

Thanks,
Rafael


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

Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

[Stratos Karafotis]

On 05/30/2013 01:29 AM, Rafael J. Wysocki wrote:
> On Wednesday, May 29, 2013 06:15:56 PM Stratos Karafotis wrote:
>> On 05/28/2013 11:54 PM, Rafael J. Wysocki wrote:
>>> On Tuesday, May 28, 2013 08:03:19 PM Stratos Karafotis wrote:
>>>> I mean any value of freq table. Please let me know if you want me to rephrase
>>>> it in description.
>>>
>>> Yes, it would be nice to be more precise.
>>
>> OK sure, I will add a more precise description.
>>
>>
>>> Which is equivalent to saying that __cpufreq_driver_getavg() is not useful and
>>> may be removed (but the patch doesn't do that and I wonder why?), but surely
>>> the developer who added it wouldn't agree.
>>>
>>> So, please explain: why can we drop __cpufreq_driver_getavg()?
>>>
>>
>> With the new proposed method the next frequency is not dependent from current
>> or average frequency. The next frequency is dependent only from load.
>> So, we don't need support for freq feedback from hardware anymore.
>
> OK, but that's a more significant change than the changelog suggests.
> The changelog should tell the whole story and explain the rationale. :-)
>
> So, please explain that in fact it is not necessary to use the current or
> average frequency to compute the target and why that is the case.
>
> Also the patch should remove __cpufreq_driver_getavg() and the callback used by
> it, since that code will be dead after applying it anyway.
>
>> Even if, due to underlying hardware coordination mechanism, CPU runs in different
>> frequency than the actual, the calculation of load and of target frequency will
>> remain the unaffected, with this patch.
>>
>> With full respect to ondemand coauthor, and if the new method is acceptable,
>> I could send a patch to revert the original one about the IA32_APERF and
>> IA32_MPERF MSR support.
>
> I'm not sure what you mean by "revert", but please do as I said above.
>
>>>> Thus, in the comparison with up_threshold to increase frequency we actually
>>>> do this (in cases that getavg is not implemented):
>>>> if (load > up_theshold)
>>>> 	increase to max
>>>>
>>>> So, after the patch we keep the same comparison to decide about the max frequency.
>>>> I thought, that below up_threshold is 'fair' to decide about the next
>>>> frequency with formula that frequency is proportional to load.
>>>> For example in a CPU with min freq 100MHz and max 1000MHz with a
>>>> load 50 target frequency should be 500MHz.
>>>
>>> Well, that sounds reasonable, but the patch actually does more than that.
>>
>> I'm sorry, but I didn't understand your last point.
>
> Please see above.
>
> The changelog doesn't even mention that the code is being switched from using
> measured past frequencies to not using them, because you think that there's a
> better way of computing the target (which by the way I can agree with :-)).
>
> Thanks,
> Rafael
>
>

OK, I will send a new patch that includes your corrections and suggestions.

Thanks so much for your time and your comments!

Stratos