Re: [PATCH 2/2] cpufreq: serialize calls to __cpufreq_governor()

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Friday, September 13, 2013 05:14:26 PM Viresh Kumar wrote:
> On 4 September 2013 01:10, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Tuesday, September 03, 2013 06:50:05 PM Srivatsa S. Bhat wrote:
> 
> >> This doesn't solve the problem completely: it prevents the store_*() task
> >> from continuing *only* when it concurrently executes the __cpufreq_governor()
> >> function along with the CPU offline task. But if the two calls don't overlap,
> >> we will still have the possibility where the store_*() task tries to acquire
> >> the timer mutex after the CPU offline task has just finished destroying it.
> >
> > Yeah, I overlooked that.
> 
> As a background, I had a IRC chat with Srivatsa on this mail.. (I have marked
> this unread as there were other important topics to close)..
> 
> And he had some other code in mind and these synchronization problems
> aren't there with my patch at all (as per him too)..
> 
> Rafael, probably both me and Srivatsa are missing something that you
> understood, can you please share what problem you see here with my patch?
> 
> And yes, even with Srivatsa's patchset I found a problem:
> Two threads, one changing governor from ondemand->conservative and other
> one changing min/max freq..
> 
> First one will try to STOP governor and other one will try to change
> limits of gov.
> Suppose 2nd one gets to ->governor() and after this first one stops
> the governor.
> Now the first one tries to access lock and crashes..
> 
> On IRC, Srivatsa agreed about the problem..
> 
> So, probably the first thing to do is to get this patch back, i.e.
> revert of Srivatsa's
> patch:
> 
> commit 56d07db274b7b15ca38b60ea4a762d40de093000
> Author: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Date:   Sat Sep 7 01:23:55 2013 +0530
> 
>     cpufreq: Remove temporary fix for race between CPU hotplug and sysfs-writes
> 
> 
> Srivatsa also asked if we can get a big lock around call to ->governor()
> which would create recursive locks on a call to EXIT event (as we have seen it
> earlier..)..
> 
> But he believes he can pull it off and will try this later.. So, for
> now we can revert
> the above patch :)

OK, I'll think about that, but not today and probably not within the next
few days, because I'm heading to New Orleans in several hours and really
have other stuff to take care of now.  Sorry about that.

Thanks,
Rafael