On 03/02/16 11:35, Viresh Kumar wrote:
On 02-02-16, 16:49, Juri Lelli wrote:
There are still paths where we call __cpufreq_governor() without holding policy->rwsem, but those should be fixed with my cleanups (that I intend to refresh and post soon). So, I'm not sure we can safely remove this yet.
No, we can't.. Though I haven't seen any races from happening even after removing it, but it doesn't mean we can't.
The deal is that, the entire sequence of events needs to be guaranteed to happen in a particular order without any other code performing similar operations concurrently.
And so we need to preserve the other sites with proper rwsem locking first.
Right. I guess it is what I was trying to do with my cleanups, adding assertions and fixing paths that didn't verify those.
It should be easy to rebase that set (or a part of it) on top of your and/or Rafael changes. I realize that there are multiple sets of changes under discussion; so, please tell me how do you, and Rafael, want to proceed about this.
So, __cpufreq_governor() becomes effectively a wrapper around ->governor() calls and governors are left responsible for implementing the state machine with appropriate checks.
Not really. The core can now guarantee that the entire sequence happens atomically. For example, on governor switch, we need to guarantee that STOP/EXIT happen without any intervention for the old governor. Or, INIT/START/LIMITS happen without any intervention for the new governor, etc..
OK, checking for invalid state transitions (for ondemand and conservative) is still in done cpufreq_governor.c.
Maybe we add a comment somewhere stating exactly how things are meant to work?
But, I guess any other governor that will bypass cpufreq_governor.c, it will also have to implement such checks. I was just proposing to state this somewhere, so that we don't forget.
Best,
- Juri