On 11/10/2019 15:44, Douglas RAILLARD wrote:
[...]
> @@ -539,6 +543,7 @@ static void sugov_update_single(struct update_util_data
> *hook, u64 time,
> unsigned long util, max;
> unsigned int next_f;
> bool busy;
> + unsigned long ramp_boost = 0;
Shouldn't always order local variable declarations from longest to
shortest line?
> sugov_iowait_boost(sg_cpu, time, flags);
> sg_cpu->last_update = time;
> @@ -552,10 +557,10 @@ static void sugov_update_single(struct update_util_data
> *hook, u64 time,
> busy = !sg_policy->need_freq_update && sugov_cpu_is_busy(sg_cpu);
>
> util = sugov_get_util(sg_cpu);
> - sugov_cpu_ramp_boost_update(sg_cpu);
> + ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu);
> max = sg_cpu->max;
> util = sugov_iowait_apply(sg_cpu, time, util, max);
> - next_f = get_next_freq(sg_policy, util, max);
> + next_f = get_next_freq(sg_policy, util, max, ramp_boost);
> /*
> * Do not reduce the frequency if the CPU has not been idle
> * recently, as the reduction is likely to be premature then.
> @@ -587,6 +592,8 @@ static unsigned int sugov_next_freq_shared(struct
> sugov_cpu *sg_cpu, u64 time)
> struct cpufreq_policy *policy = sg_policy->policy;
> unsigned long util = 0, max = 1;
> unsigned int j;
> + unsigned long ramp_boost = 0;
> + unsigned long j_ramp_boost = 0;
Shouldn't always order local variable declarations from longest to
shortest line?
> for_each_cpu(j, policy->cpus) {
> struct sugov_cpu *j_sg_cpu = &per_cpu(sugov_cpu, j);
> @@ -594,7 +601,11 @@ static unsigned int sugov_next_freq_shared(struct
> sugov_cpu *sg_cpu, u64 time)
>
> j_util = sugov_get_util(j_sg_cpu);
> if (j_sg_cpu == sg_cpu)
> - sugov_cpu_ramp_boost_update(sg_cpu);
> + j_ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu);
> + else
> + j_ramp_boost = sugov_cpu_ramp_boost(j_sg_cpu);
> + ramp_boost = max(ramp_boost, j_ramp_boost);
Ah, that's why you call sugov_cpu_ramp_boost_update() from
sugov_next_freq_shared() in 4/6. So sugov_cpu_ramp_boost_update() is
more a sugov_cpu_ramp_boost(..., int update)? Minor detail though.
[...]
