Messages in this thread | | | Date | Wed, 3 Feb 2016 11:05:12 +0000 | From | Juri Lelli <> | Subject | Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock |
| |
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
| |