On 15-06-15, 13:53, Preeti U Murthy wrote:
This patch is not convincing. What are the precise races between cpufreq_governor_dbs() and the work handlers ?
The most important problem was restarting of workqueues from the handler, which can happen at the same time when the governor-callbacks are in progress..
It is true that the work handlers can get queued after a governor exit due to a race between different callers of cpufreq_governor_dbs() and they can dereference freed data structures. But that is about invalid interleaving of states which your next patch aims at fixing.
AFAICT, preventing invalid states from succeeding will avoid this race. I don't see the need for another lock here.
Another lock? I am taking exactly the same lock at all places and actually removing the need of two separate locks.
So, this patch replaces 'timer_mutex' with 'cdata->mutex' so that both work-handler and callback are in sync.
Also in update_sampling_rate() we are dropping the mutex while canceling delayed-work. There is no need to do that, keep lock active
Why? What is the race that we are fixing by taking the lock here ?
This can also queue work on CPUs.