Re: [PATCH 08/12] cpufreq: governor: synchronize work-handler with governor callbacks

[Viresh Kumar <viresh.kumar@linaro.org>]

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.

-- 
viresh