Messages in this thread | | | Date | Tue, 02 Jun 2015 11:50:34 +0530 | From | Preeti U Murthy <> | Subject | Re: [RFC PATCH] cpufreq/hotplug: Fix cpu-hotplug cpufreq race conditions |
| |
On 06/02/2015 11:41 AM, Viresh Kumar wrote: > On 02-06-15, 11:33, Preeti U Murthy wrote: >> No, dbs_data is a governor wide data structure and not a policy wide > > Yeah, that's the common part which I was referring to. But normally > its just read for policies in START/STOP, they just update per-cpu > data for policy->cpus. > >> one, which is manipulated in START/STOP calls for drivers where the >> CPUFREQ_HAVE_GOVERNOR_PER_POLICY is not set. >> >> So even if we assume that we hold per-policy locks, the following race >> is still present. Assume that we have just two cpus which do not have a >> governor-per-policy set. >> >> CPU0 CPU1 >> >> store* store* >> >> lock(policy 1) lock(policy 2) >> cpufreq_set_policy() cpufreq_set_policy() >> EXIT() : >> dbs-data->usage_count-- >> >> INIT() >> dbs_data exists > > You missed the usage_count++ here.
Ok, sorry about that. How about the below ? > >> so return >> EXIT() >> dbs_data->usage_count -- = 0 >> kfree(dbs_data) > > And so this shouldn't happen. Else we > are missing locking in governor's > code, rather than cpufreq.c > CPU0 CPU1
store* store*
lock(policy 1) lock(policy 2) cpufreq_set_policy() cpufreq_set_policy() EXIT() : dbs-data->usage_count--
INIT() EXIT() dbs_data exists dbs_data->usage_count -- = 0 kfree(dbs_data) dbs-data->usage_count++ *NULL dereference*
The point is there are potential race conditions. Its just a matter of interleaving ?
Regards Preeti U Murthy
| |