Messages in this thread | | | Date | Wed, 10 Feb 2016 00:10:53 +0100 | Subject | Re: [PATCH V4 6/6] cpufreq: governor: Create and traverse list of policy_dbs to fix lockdep | From | "Rafael J. Wysocki" <> |
| |
On Tue, Feb 9, 2016 at 4:31 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > An instance of 'struct dbs_data' can support multiple 'struct > policy_dbs_info' instances. To traverse all policy_dbs supported by a > dbs_data, create a list of policy_dbs within dbs_data. > > We can traverse this list now, instead of traversing the loop for all > online CPUs in update_sampling_rate(), to solve the circular dependency > lockdep reported by Juri (and verified by Shilpa) earlier:
In addition to the previous comment, here's my changelog for this patch:
"The dbs_data_mutex lock is currently used in two places. First, cpufreq_governor_dbs() uses it to guarantee mutual exlusion between invocations of governor operations from the core. Second, it is used by ondemand governor's update_sampling_rate() to ensure the stability of data structures walked by it.
The second usage is quite problematic, because update_sampling_rate() is called from a governor sysfs attribute's ->store callback and that leads to a deadlock scenario involving cpufreq_governor_exit() which runs under dbs_data_mutex. Thus it is better to rework the code so update_sampling_rate() doesn't need to acquire dbs_data_mutex.
To that end, rework update_sampling_rate() to walk a list of policy_dbs objects supported by the dbs_data one it has been called for (instead of walking cpu_dbs_info object for all CPUs). The list manipulation is protected with dbs_data->mutex which also is held around the execution of update_sampling_rate(), it is not necessary to hold dbs_data_mutex in that function any more."
Thanks, Rafael
| |