On 28-10-15, 05:05, Rafael J. Wysocki wrote:
On Tuesday, October 13, 2015 01:39:01 PM Viresh Kumar wrote:
'timer_mutex' is required to sync work-handlers of policy->cpus. update_sampling_rate() is just canceling the works and queuing them again. This isn't protecting anything at all in update_sampling_rate() and is not gonna be of any use.
Even if a work-handler is already running for a CPU, cancel_delayed_work_sync() will wait for it to finish.
Drop these unnecessary locks.
Reviewed-by: Preeti U Murthy preeti@linux.vnet.ibm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
I'm queuing this up for 4.4, although I think that the changelog is not right.
While at it, what are the race conditions the lock is protecting against?
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.
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.
And then there are enough minute things that can go wrong if multiple CPUs do the load evaluation and freq-update at the same time, apart from it being an time wasting effort.
And so I still think that the commit log isn't that bad. The timer_mutex lock isn't required in other parts of the governor, they are just for synchronizing the work-handlers of CPUs belonging to the same policy.