On Tue, Feb 9, 2016 at 4:31 AM, Viresh Kumar <viresh.ku...@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