On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote:
On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki rafael@kernel.org wrote:
On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote:
On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli juri.lelli@arm.com wrote:
[cut]
I also don't like this patch because it forces governors to either implement their own macros and management of their attributes or force them to use the governor structs that come with cpufreq_governor.h. cpufreq_governor.h IMHO is very ondemand and conservative governor specific and is very irrelevant for sched-dvfs or any other governors (hint hint).
The only time this ABBA locking is an issue is when governor are changing and trying to add/remove attributes. That can easily be checked in store_governor and dealt with without holding the policy rwsem if the governors can provide their per sys and per policy attribute arrays as part of registering themselves.
I'm sorry that I just keep talking about the idea and not sending out the patches.
I think you have a point, though.
The deadlock really is specific to the governors using the code in cpufreq_governor.c.
That said no other governors in the tree use any sysfs attributes for tunables AFAICS and the out-of-the tree ones are out of interest here.
But if we are expecting sched dvfs to come in, why make it worse for it. It would be completely pointless to try and shoehorn sched dvfs to use cpufreq_governor.c
Also the deadlock happens if one of the tunable attributes is accessed while we're trying to remove it which very well may happen on read access too.
Isn't this THE deadlock we are talking about? The removal of the attributes only happen when governors are changes and we send a POLICY_EXIT and or all the cores are hotplugged out. And my suggestion would work just as well there.
Why are you prefixing your sentence with "Also"? Is there some other case I'm not considering?
-Saravana