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 <strat...@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
