Messages in this thread | | | Date | Wed, 3 Feb 2016 12:27:45 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops |
| |
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 :)
-- viresh
| |