Messages in this thread | | | Date | Wed, 10 Feb 2010 14:56:43 +0800 | From | Cong Wang <> | Subject | Re: [Patch] cpufreq: fix a deadlock during shutting down |
| |
David Rientjes wrote: > On Tue, 9 Feb 2010, Amerigo Wang wrote: > >> Benjamin reported that, the machine deadlocks right after printing the >> following when doing a shutdown: >> >> halt/4071 is trying to acquire lock: >> (s_active){++++.+}, at: [<c0000000001ef868>] .sysfs_addrm_finish+0x58/0xc0 >> >> but task is already holding lock: >> (&per_cpu(cpu_policy_rwsem, cpu)){+.+.+.}, at: [<c0000000004cd6ac>] .lock_policy_rwsem_write+0x84/0xf4 >> >> which lock already depends on the new lock. >> >> the existing dependency chain (in reverse order) is: >> >> <nothing else ... machine deadlocked here> >> >> >> This is because we are trying to kobject_put() a kobject while >> we are holding cpu policy rwsem. So just move kobject_put() >> down after releasing the rwsem. >> >> Totally untested. >> >> Reported-by: Xiaotian Feng <xtfeng@gmail.com> >> Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Signed-off-by: WANG Cong <amwang@redhat.com> >> Cc: Dave Jones <davej@redhat.com> >> Cc: Thomas Renninger <trenn@suse.de> >> Cc: Prarit Bhargava <prarit@redhat.com> >> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> >> >> --- >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index 67bc2ec..222b35f 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -1113,6 +1113,7 @@ static int __cpufreq_remove_dev(struct sys_device *sys_dev) >> unsigned int cpu = sys_dev->id; >> unsigned long flags; >> struct cpufreq_policy *data; >> + struct kobject *kobj; >> #ifdef CONFIG_SMP >> struct sys_device *cpu_sys_dev; >> unsigned int j; >> @@ -1192,7 +1193,7 @@ static int __cpufreq_remove_dev(struct sys_device *sys_dev) >> if (cpufreq_driver->target) >> __cpufreq_governor(data, CPUFREQ_GOV_STOP); >> >> - kobject_put(&data->kobj); >> + kobj = &data->kobj; >> >> /* we need to make sure that the underlying kobj is actually >> * not referenced anymore by anybody before we proceed with > > NACK. > > If you read this comment, it says: > > /* we need to make sure that the underlying kobj is actually > * not referenced anymore by anybody before we proceed with > * unloading. > */ > > That would suggest that the wait_for_completion(&data->kobj_unregister); > would never actually return if you're holding a reference to it in your > patch since it only completes when the last reference is dropped (the > ->release function is cpufreq_sysfs_release()).
Oh, my bad.
Then this case seems to be more complex... But anyway, this is _not_ a bogus.
Thanks.
| |