On Wed, Apr 12, 2017 at 4:26 PM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 11-04-17, 16:00, Rafael J. Wysocki wrote:
On Tue, Apr 11, 2017 at 12:35 PM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 29-03-17, 23:28, Rafael J. Wysocki wrote:
On Thursday, March 09, 2017 05:15:15 PM Viresh Kumar wrote:
@@ -216,7 +216,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time, if (flags & SCHED_CPUFREQ_RT_DL) { next_f = policy->cpuinfo.max_freq; } else {
sugov_get_util(&util, &max);
sugov_get_util(&util, &max, hook->cpu);Why is this not racy?
Why would reading the utilization values be racy? The only dynamic value here is "util_avg" and I am not sure if reading it is racy.
But, this whole routine has races which I ignored as we may end up updating frequency simultaneously from two threads.
Those races aren't there if we don't update cross-CPU, which is my point. :-)
Of course. There are no races without this series.
sugov_iowait_boost(sg_cpu, &util, &max); next_f = get_next_freq(sg_policy, util, max); }@@ -272,7 +272,7 @@ static void sugov_update_shared(struct update_util_data *hook, u64 time, unsigned long util, max; unsigned int next_f;
- sugov_get_util(&util, &max);
- sugov_get_util(&util, &max, hook->cpu);
And here?
raw_spin_lock(&sg_policy->update_lock);The lock prevents the same here though.
So, if we are going to use this series, then we can use the same update-lock in case of single cpu per policies as well.
No, we can't.
The lock is unavoidable in the mulit-CPU policies case, but there's no way I will agree on using a lock in the single-CPU case.
How do you suggest to avoid the locking here then ? Some atomic variable read/write as done in cpufreq_governor.c ?
That is a very good question. :-)
I need to look at the scheduler code that invokes those things and see what happens in there. Chances are there already is some sufficient mutual exclusion in place.