On 04-06-15, 12:34, Preeti U Murthy wrote:
My point is do we really need to treat them as separate problems ?
Yes.
Will not serializing sequence of events help solve both issues ?
That's not the point. Even if it solves the problem, it may not be the right approach. There are two problems here: - One lies in cpufreq.c or in policies domain. - Other one is about governor. Governor code shouldn't rely of cpufreq.c locking to guarantee that access to its structures isn't racy.
And the way you proposed to solve it (yes the original idea was from one of my earlier patches) is not the right way to do it.
For example, cpufreq_set_policy() shouldn't care about how the governor code is placed. It should just do enough to get rid of racy code belonging to that policy.
But with our other approach, we are trying to stop the governor to be used by anyone else in the kernel. Who the hell is that 'policy' to decide who can access the governor ?
That's why I divided it up, so that we don't come to it again. I haven't learnt these things earlier, and wrote messy locking code earlier. But after looking/working on core code like timers etc, I understood the importance of right design and proper partitioning of responsibilities.
When we know the problem, why not fix it proper, rather than breaking it up ?
What are we breaking here ?
This patch is trying to solve only the first problem. "
I NEVER claimed that I solved all the issues.
That is true. My intention was to point out explicitly what still remains to be solved. It is true that you have mentioned that the problem in the cpufreq core is about sequencing of events. I intended to highlight what it was.
Okay, that should be the next target we have to fix after applying these patches.
I would have restrained from pointing it out had the issues that I am seeing waned a wee bit, but it has not, which is why I did not see value in having the third patch as a stand alone patch, with more going in as series.
Its solving what it is intended to solve. But we need more patches over it to fix problems outside the scope of governors.