Re: [v3.10 regression] deadlock on cpu hotplug

On 07/09/2013 07:51 PM, Bartlomiej Zolnierkiewicz wrote:
[snip]

It doesn't help and unfortunately it just can't help as it only
addresses lockdep functionality while the issue is not a lockdep
problem but a genuine locking problem. CPU hot-unplug invokes
_cpu_down() which calls cpu_hotplug_begin() which in turn takes
&cpu_hotplug.lock. The lock is then hold during __cpu_notify()
call. Notifier chain goes up to cpufreq_governor_dbs() which for
CPUFREQ_GOV_STOP event does gov_cancel_work(). This function
flushes pending work and waits for it to finish. The all above
happens in one kernel thread. At the same time the other kernel
thread is doing the work we are waiting to complete and it also
happens to do gov_queue_work() which calls get_online_cpus().
Then the code tries to take &cpu_hotplug.lock which is already
held by the first thread and deadlocks.

Hmm...I think I get your point, some thread hold the lock and
flush some work which also try to hold the same lock, correct?

Ok, that's a problem, let's figure out a good way to solve it :)

Regards,
Michael Wang

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

quoted

diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 5af40ad..aa05eaa 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c

@@ -229,6 +229,8 @@ static void set_sampling_rate(struct dbs_data *dbs_data,
        }
 }
 
+static struct lock_class_key j_cdbs_key;
+
 int cpufreq_governor_dbs(struct cpufreq_policy *policy,
                struct common_dbs_data *cdata, unsigned int event)
 {

@@ -366,6 +368,8 @@ int (struct cpufreq_policy *policy,
                                        kcpustat_cpu(j).cpustat[CPUTIME_NICE];
 
                        mutex_init(&j_cdbs->timer_mutex);
+                       lockdep_set_class(&j_cdbs->timer_mutex, &j_cdbs_key);
+
                        INIT_DEFERRABLE_WORK(&j_cdbs->work,
                                             dbs_data->cdata->gov_dbs_timer);
                }

Regards,
Michael Wang