Skip to content

Commit

Permalink
cpufreq: Fix sysfs deadlock with concurrent hotplug/frequency switch
Browse files Browse the repository at this point in the history
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 torvalds#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 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 <[email protected]>
(cherry picked from commit f82ef51)

Signed-off-by: Ramakrishna Prasad N <[email protected]>
(cherry picked from commit 31f517f)

Change-Id: Ic70261763b6697a08ed1c3d34ac969bda9648fa0
Signed-off-by: Ramakrishna Prasad N <[email protected]>
  • Loading branch information
bebarino authored and Ramakrishna Prasad N committed Jul 24, 2012
1 parent 69c592d commit ddda0cf
Showing 1 changed file with 27 additions and 8 deletions.
35 changes: 27 additions & 8 deletions drivers/cpufreq/cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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 *
Expand Down Expand Up @@ -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;

Expand All @@ -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;
}
Expand All @@ -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;

Expand All @@ -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;
}
Expand Down

0 comments on commit ddda0cf

Please sign in to comment.