From ddda0cf21dc6cb487aacd81fdb1a7343d2746d94 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 17 Jul 2012 14:33:57 -0700 Subject: [PATCH] cpufreq: Fix sysfs deadlock with concurrent hotplug/frequency switch Running one program that continuously hotplugs and replugs a cpu concurrently with another program that continuously writes to the scaling_set_speed node eventually deadlocks with: ============================================= [ INFO: possible recursive locking detected ] 3.4.0 #37 Tainted: G W --------------------------------------------- filemonkey/122 is trying to acquire lock: (s_active#13){++++.+}, at: [] sysfs_remove_dir+0x9c/0xb4 but task is already holding lock: (s_active#13){++++.+}, at: [] sysfs_write_file+0xe8/0x140 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(s_active#13); lock(s_active#13); *** DEADLOCK *** May be due to missing lock nesting notation 2 locks held by filemonkey/122: #0: (&buffer->mutex){+.+.+.}, at: [] sysfs_write_file+0x28/0x140 #1: (s_active#13){++++.+}, at: [] sysfs_write_file+0xe8/0x140 stack backtrace: [] (unwind_backtrace+0x0/0x120) from [] (validate_chain+0x6f8/0x1054) [] (validate_chain+0x6f8/0x1054) from [] (__lock_acquire+0x81c/0x8d8) [] (__lock_acquire+0x81c/0x8d8) from [] (lock_acquire+0x18c/0x1e8) [] (lock_acquire+0x18c/0x1e8) from [] (sysfs_addrm_finish+0xd0/0x180) [] (sysfs_addrm_finish+0xd0/0x180) from [] (sysfs_remove_dir+0x9c/0xb4) [] (sysfs_remove_dir+0x9c/0xb4) from [] (kobject_del+0x10/0x38) [] (kobject_del+0x10/0x38) from [] (kobject_release+0xf0/0x194) [] (kobject_release+0xf0/0x194) from [] (cpufreq_cpu_put+0xc/0x24) [] (cpufreq_cpu_put+0xc/0x24) from [] (store+0x6c/0x74) [] (store+0x6c/0x74) from [] (sysfs_write_file+0x10c/0x140) [] (sysfs_write_file+0x10c/0x140) from [] (vfs_write+0xb0/0x128) [] (vfs_write+0xb0/0x128) from [] (sys_write+0x3c/0x68) [] (sys_write+0x3c/0x68) from [] (ret_fast_syscall+0x0/0x3c) This is because store() in cpufreq.c indirectly grabs a kobject with kobject_get() and is the last one to call kobject_put() indirectly via cpufreq_cpu_put(). Fix this deadlock by introducing two new functions, cpufreq_cpu_get_sysfs() and cpufreq_cpu_put_sysfs() which do the same thing as cpufreq_cpu_{get,put}() but don't grab the kobject. CRs-fixed: 366560 Signed-off-by: Stephen Boyd (cherry picked from commit f82ef51be26b44b83d86afa56b3cd9c743023470) Signed-off-by: Ramakrishna Prasad N (cherry picked from commit 31f517f4b270b89ad1ceee0131940806bd11f02d) Change-Id: Ic70261763b6697a08ed1c3d34ac969bda9648fa0 Signed-off-by: Ramakrishna Prasad N --- drivers/cpufreq/cpufreq.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index ff15497e919956..440d6da9a04a6e 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -133,7 +133,7 @@ pure_initcall(init_cpufreq_transition_notifier_list); static LIST_HEAD(cpufreq_governor_list); static DEFINE_MUTEX(cpufreq_governor_mutex); -struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) +static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, int sysfs) { struct cpufreq_policy *data; unsigned long flags; @@ -157,7 +157,7 @@ struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) if (!data) goto err_out_put_module; - if (!kobject_get(&data->kobj)) + if (!sysfs && !kobject_get(&data->kobj)) goto err_out_put_module; spin_unlock_irqrestore(&cpufreq_driver_lock, flags); @@ -170,16 +170,35 @@ struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) err_out: return NULL; } + +struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) +{ + return __cpufreq_cpu_get(cpu, 0); +} EXPORT_SYMBOL_GPL(cpufreq_cpu_get); +static struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu) +{ + return __cpufreq_cpu_get(cpu, 1); +} -void cpufreq_cpu_put(struct cpufreq_policy *data) +static void __cpufreq_cpu_put(struct cpufreq_policy *data, int sysfs) { - kobject_put(&data->kobj); + if (!sysfs) + kobject_put(&data->kobj); module_put(cpufreq_driver->owner); } + +void cpufreq_cpu_put(struct cpufreq_policy *data) +{ + __cpufreq_cpu_put(data, 0); +} EXPORT_SYMBOL_GPL(cpufreq_cpu_put); +static void cpufreq_cpu_put_sysfs(struct cpufreq_policy *data) +{ + __cpufreq_cpu_put(data, 1); +} /********************************************************************* * EXTERNALLY AFFECTING FREQUENCY CHANGES * @@ -613,7 +632,7 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf) struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); ssize_t ret = -EINVAL; - policy = cpufreq_cpu_get(policy->cpu); + policy = cpufreq_cpu_get_sysfs(policy->cpu); if (!policy) goto no_policy; @@ -627,7 +646,7 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf) unlock_policy_rwsem_read(policy->cpu); fail: - cpufreq_cpu_put(policy); + cpufreq_cpu_put_sysfs(policy); no_policy: return ret; } @@ -638,7 +657,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); ssize_t ret = -EINVAL; - policy = cpufreq_cpu_get(policy->cpu); + policy = cpufreq_cpu_get_sysfs(policy->cpu); if (!policy) goto no_policy; @@ -652,7 +671,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, unlock_policy_rwsem_write(policy->cpu); fail: - cpufreq_cpu_put(policy); + cpufreq_cpu_put_sysfs(policy); no_policy: return ret; }