Hi Rafael,
On 02/02/16 17:35, Rafael J. Wysocki wrote:
On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli juri.lelli@arm.com wrote:
Hi Viresh,
On 02/02/16 16:27, Viresh Kumar wrote:
Until now, governors (ondemand/conservative) were using the 'global-attr' or 'freq-attr', depending on the sysfs location where we want to create governor's directory.
The problem is that, in case of 'freq-attr', we are forced to use show()/store() present in cpufreq.c, which always take policy->rwsem.
And because of that we were facing some ABBA lockups during governor callback event CPUFREQ_GOV_POLICY_EXIT. And so we were dropping the rwsem right before calling governor callback for CPUFREQ_GOV_POLICY_EXIT event.
That caused further problems and it never worked perfectly.
This patch attempts to fix that by creating separate sysfs-ops for cpufreq governors.
Because things got much simplified now, we don't need separate show/store callbacks for governor-for-system and governor-per-policy cases.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
This patch cleans things up a lot, that's good.
One thing I'm still concerned about, though: don't we need some locking in place for some of the store operations on governors attributes? Are store_{ignore_nice_load, sampling_down_fact, etc} safe without locking?
That would require some investigation I suppose.
It seems that we can call them from different cpus concurrently.
Yes, we can.
One quick-and-dirty way of dealing with that might be to introduce a "sysfs lock" into struct dbs_data and hold that around the invocation of gattr->store() in the sysfs_ops's ->store callback.
There is value in trying to solve this issue by using some of the existing locks, IMHO.
Can't we actually try to use the policy->rwsem (or one of the core locks) + wait_for_completion approach as we do in cpufreq core?
BTW, you could have dropped the stuff below this line from your reply message. That at least would have prevented tools like Patchwork from storing useless garbage.
Right. Sorry for the garbage; I'll check twice that I trim my replies in the future.
Best,
- Juri