On 28-10-15, 06:54, Rafael J. Wysocki wrote:
On Wednesday, October 28, 2015 10:14:51 AM Viresh Kumar wrote:
In cases where a single policy controls multiple CPUs, a timer is queued for every cpu present in policy->cpus. When we reach the timer handler (which can be on multiple CPUs together) on any CPU, we trace CPU load for all policy->cpus and update the frequency accordingly.
That would be in dbs_timer(), right?
Yeah, and we already do stuff from within the mutex there.
The lock is for protecting multiple CPUs to do the same thing together, as only its required to be done by a single CPU. Once any CPUs handler has completed, it updates the last update time and drops the mutex. At that point of time, other blocked handler (if any) check the last update time and return early.
Well, that would mean we only needed to hold the lock around the need_load_eval() evaluation in dbs_timer() if I'm not mistaken.
Actually yeah, but then the fourth patch of this series uses the timer_mutex to fix a long standing problem (which was fixed by hacking the code earlier). And so we need to take the lock for the entire dbs_timer() routine.
We also should acquire it around updates of the sampling rate, which essentially is set_sampling_rate().
Why? In the worst case we may schedule the next timer for the earlier sampling rate. But do we care that much for that race, that we want to add locks here as well ?
Is there any reason to acquire it in cpufreq_governor_limits(), then, for example?
Yeah, we are calling dbs_check_cpu(dbs_data, cpu) from that path, which will reevaluate the load.