On 02/02/2016 10:57 PM, Viresh Kumar wrote:
On 02-02-16, 20:03, Saravana Kannan wrote:
This is the hotplug case I mentioned. The sysfs file removals will happen
only for the last CPU in that policy (we thankfully optimized that part last
year). We also know that multiple CPUs can't be hotplugged at the same time.
So, in the start of cpufreq_offline_prepare, we just need to check if this
is the last CPU in the policy and if that's the case, do the gov sysfs
remove and then grab the policy lock and do all our crap. If a read is going
on, that's going to finish before the sysfs attr remove can go ahead and it
can grab the policy lock if it needs to and that still won't cause a
deadlock because we haven't yet grabbed the policy lock in
cpufreq_offline_prepare(). If the read comes after the sysfs remove, then
the read is obviously going to fail (we can depend on the sysfs framework on
doing its job there).
IMHO, these are all dirty hacks we should stay away from. Adding such
hunks in code is considered a band-aid kind of solution and hurts
readability badly. The new solution (new governor show/store)
implement this in a very clean and proper way I feel..
Others are free to disagree though :)
I think it looks clean since we haven't sorted out the race conditions
that Juri pointed out. So, it's early to call this series clean :)
Also, I don't see it as a dirty hack at all. What's so hacky about it?
We are just identifying conditions when we'll have to remove the sysfs
files and removing them before grabbing the policy lock.
The unlock/lock that we have now is what is a dirty hack.
-Saravana
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project