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_setspeed 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: [<c01a3d28>] sysfs_remove_dir+0x9c/0xb4 but task is already holding lock: (s_active#13){++++.+}, at: [<c01a22f0>] 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: [<c01a2230>] sysfs_write_file+0x28/0x140 #1: (s_active#13){++++.+}, at: [<c01a22f0>] sysfs_write_file+0xe8/0x140 stack backtrace: [<c0014fcc>] (unwind_backtrace+0x0/0x120) from [<c00ca600>] (validate_chain+0x6f8/0x1054) [<c00ca600>] (validate_chain+0x6f8/0x1054) from [<c00cb778>] (__lock_acquire+0x81c/0x8d8) [<c00cb778>] (__lock_acquire+0x81c/0x8d8) from [<c00cb9c0>] (lock_acquire+0x18c/0x1e8) [<c00cb9c0>] (lock_acquire+0x18c/0x1e8) from [<c01a3ba8>] (sysfs_addrm_finish+0xd0/0x180) [<c01a3ba8>] (sysfs_addrm_finish+0xd0/0x180) from [<c01a3d28>] (sysfs_remove_dir+0x9c/0xb4) [<c01a3d28>] (sysfs_remove_dir+0x9c/0xb4) from [<c02d0e5c>] (kobject_del+0x10/0x38) [<c02d0e5c>] (kobject_del+0x10/0x38) from [<c02d0f74>] (kobject_release+0xf0/0x194) [<c02d0f74>] (kobject_release+0xf0/0x194) from [<c0565a98>] (cpufreq_cpu_put+0xc/0x24) [<c0565a98>] (cpufreq_cpu_put+0xc/0x24) from [<c05683f0>] (store+0x6c/0x74) [<c05683f0>] (store+0x6c/0x74) from [<c01a2314>] (sysfs_write_file+0x10c/0x140) [<c01a2314>] (sysfs_write_file+0x10c/0x140) from [<c014af44>] (vfs_write+0xb0/0x128) [<c014af44>] (vfs_write+0xb0/0x128) from [<c014b06c>] (sys_write+0x3c/0x68) [<c014b06c>] (sys_write+0x3c/0x68) from [<c000e0e0>] (ret_fast_syscall+0x0/0x3c) This is because store() in cpufreq.c indirectly calls kobject_get() via cpufreq_cpu_get() and is the last one to call kobject_put() via cpufreq_cpu_put(). Sysfs code should not call kobject_get() or kobject_put() directly (see the comment around sysfs_schedule_callback() for more information). Fix this deadlock by introducing two new functions: struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu) void cpufreq_cpu_put_sysfs(struct cpufreq_policy *data) which do the same thing as cpufreq_cpu_{get,put}() but don't call kobject functions. To easily trigger this deadlock you can insert an msleep() with a reasonably large value right after the fail label at the bottom of the store() function in cpufreq.c and then write scaling_setspeed in one task and offline the cpu in another. The first task will hang and be detected by the hung task detector. Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
This commit is contained in:
committed by
Rafael J. Wysocki
parent
53df1ad525
commit
a914443627
@@ -138,7 +138,7 @@ void disable_cpufreq(void)
|
|||||||
static LIST_HEAD(cpufreq_governor_list);
|
static LIST_HEAD(cpufreq_governor_list);
|
||||||
static DEFINE_MUTEX(cpufreq_governor_mutex);
|
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, bool sysfs)
|
||||||
{
|
{
|
||||||
struct cpufreq_policy *data;
|
struct cpufreq_policy *data;
|
||||||
unsigned long flags;
|
unsigned long flags;
|
||||||
@@ -162,7 +162,7 @@ struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
|
|||||||
if (!data)
|
if (!data)
|
||||||
goto err_out_put_module;
|
goto err_out_put_module;
|
||||||
|
|
||||||
if (!kobject_get(&data->kobj))
|
if (!sysfs && !kobject_get(&data->kobj))
|
||||||
goto err_out_put_module;
|
goto err_out_put_module;
|
||||||
|
|
||||||
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
|
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
|
||||||
@@ -175,16 +175,35 @@ err_out_unlock:
|
|||||||
err_out:
|
err_out:
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
|
||||||
|
{
|
||||||
|
return __cpufreq_cpu_get(cpu, false);
|
||||||
|
}
|
||||||
EXPORT_SYMBOL_GPL(cpufreq_cpu_get);
|
EXPORT_SYMBOL_GPL(cpufreq_cpu_get);
|
||||||
|
|
||||||
|
static struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu)
|
||||||
|
{
|
||||||
|
return __cpufreq_cpu_get(cpu, true);
|
||||||
|
}
|
||||||
|
|
||||||
|
static void __cpufreq_cpu_put(struct cpufreq_policy *data, bool sysfs)
|
||||||
|
{
|
||||||
|
if (!sysfs)
|
||||||
|
kobject_put(&data->kobj);
|
||||||
|
module_put(cpufreq_driver->owner);
|
||||||
|
}
|
||||||
|
|
||||||
void cpufreq_cpu_put(struct cpufreq_policy *data)
|
void cpufreq_cpu_put(struct cpufreq_policy *data)
|
||||||
{
|
{
|
||||||
kobject_put(&data->kobj);
|
__cpufreq_cpu_put(data, false);
|
||||||
module_put(cpufreq_driver->owner);
|
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(cpufreq_cpu_put);
|
EXPORT_SYMBOL_GPL(cpufreq_cpu_put);
|
||||||
|
|
||||||
|
static void cpufreq_cpu_put_sysfs(struct cpufreq_policy *data)
|
||||||
|
{
|
||||||
|
__cpufreq_cpu_put(data, true);
|
||||||
|
}
|
||||||
|
|
||||||
/*********************************************************************
|
/*********************************************************************
|
||||||
* EXTERNALLY AFFECTING FREQUENCY CHANGES *
|
* EXTERNALLY AFFECTING FREQUENCY CHANGES *
|
||||||
@@ -617,7 +636,7 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
|
|||||||
struct cpufreq_policy *policy = to_policy(kobj);
|
struct cpufreq_policy *policy = to_policy(kobj);
|
||||||
struct freq_attr *fattr = to_attr(attr);
|
struct freq_attr *fattr = to_attr(attr);
|
||||||
ssize_t ret = -EINVAL;
|
ssize_t ret = -EINVAL;
|
||||||
policy = cpufreq_cpu_get(policy->cpu);
|
policy = cpufreq_cpu_get_sysfs(policy->cpu);
|
||||||
if (!policy)
|
if (!policy)
|
||||||
goto no_policy;
|
goto no_policy;
|
||||||
|
|
||||||
@@ -631,7 +650,7 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
|
|||||||
|
|
||||||
unlock_policy_rwsem_read(policy->cpu);
|
unlock_policy_rwsem_read(policy->cpu);
|
||||||
fail:
|
fail:
|
||||||
cpufreq_cpu_put(policy);
|
cpufreq_cpu_put_sysfs(policy);
|
||||||
no_policy:
|
no_policy:
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
@@ -642,7 +661,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
|
|||||||
struct cpufreq_policy *policy = to_policy(kobj);
|
struct cpufreq_policy *policy = to_policy(kobj);
|
||||||
struct freq_attr *fattr = to_attr(attr);
|
struct freq_attr *fattr = to_attr(attr);
|
||||||
ssize_t ret = -EINVAL;
|
ssize_t ret = -EINVAL;
|
||||||
policy = cpufreq_cpu_get(policy->cpu);
|
policy = cpufreq_cpu_get_sysfs(policy->cpu);
|
||||||
if (!policy)
|
if (!policy)
|
||||||
goto no_policy;
|
goto no_policy;
|
||||||
|
|
||||||
@@ -656,7 +675,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
|
|||||||
|
|
||||||
unlock_policy_rwsem_write(policy->cpu);
|
unlock_policy_rwsem_write(policy->cpu);
|
||||||
fail:
|
fail:
|
||||||
cpufreq_cpu_put(policy);
|
cpufreq_cpu_put_sysfs(policy);
|
||||||
no_policy:
|
no_policy:
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user