cpufreq core uses two locks:
- cpufreq_driver_lock: General lock for driver and cpufreq_cpu_data array.
- cpu_policy_rwsemfix locking: per CPU reader-writer semaphore designed to cure
all cpufreq/hotplug/workqueue/etc related lock issues.
These locks were not used properly and are placed against their principle
(present before their definition) at various places. This patch is an attempt to
fix their use.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
* mode before doing so.
*
* Additional rules:
* mode before doing so.
*
* Additional rules:
- * - All holders of the lock should check to make sure that the CPU they
- * are concerned with are online after they get the lock.
* - Governor routines that can be called in cpufreq hotplug path should not
* take this sem as top level hotplug notifier handler takes this.
* - Lock should not be held across
* - Governor routines that can be called in cpufreq hotplug path should not
* take this sem as top level hotplug notifier handler takes this.
* - Lock should not be held across
void cpufreq_notify_transition(struct cpufreq_freqs *freqs, unsigned int state)
{
struct cpufreq_policy *policy;
void cpufreq_notify_transition(struct cpufreq_freqs *freqs, unsigned int state)
{
struct cpufreq_policy *policy;
pr_debug("notification %u of frequency transition to %u kHz\n",
state, freqs->new);
pr_debug("notification %u of frequency transition to %u kHz\n",
state, freqs->new);
+ spin_lock_irqsave(&cpufreq_driver_lock, flags);
policy = per_cpu(cpufreq_cpu_data, freqs->cpu);
policy = per_cpu(cpufreq_cpu_data, freqs->cpu);
+ spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
+
switch (state) {
case CPUFREQ_PRECHANGE:
switch (state) {
case CPUFREQ_PRECHANGE:
policy = cpufreq_cpu_get(sibling);
WARN_ON(!policy);
policy = cpufreq_cpu_get(sibling);
WARN_ON(!policy);
- per_cpu(cpufreq_policy_cpu, cpu) = policy->cpu;
-
- lock_policy_rwsem_write(cpu);
-
__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+ lock_policy_rwsem_write(sibling);
+
spin_lock_irqsave(&cpufreq_driver_lock, flags);
spin_lock_irqsave(&cpufreq_driver_lock, flags);
cpumask_set_cpu(cpu, policy->cpus);
cpumask_set_cpu(cpu, policy->cpus);
+ per_cpu(cpufreq_policy_cpu, cpu) = policy->cpu;
per_cpu(cpufreq_cpu_data, cpu) = policy;
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
per_cpu(cpufreq_cpu_data, cpu) = policy;
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
+ unlock_policy_rwsem_write(sibling);
+
__cpufreq_governor(policy, CPUFREQ_GOV_START);
__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
__cpufreq_governor(policy, CPUFREQ_GOV_START);
__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
- unlock_policy_rwsem_write(cpu);
-
ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
if (ret) {
cpufreq_cpu_put(policy);
ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
if (ret) {
cpufreq_cpu_put(policy);
#ifdef CONFIG_HOTPLUG_CPU
/* Check if this cpu was hot-unplugged earlier and has siblings */
#ifdef CONFIG_HOTPLUG_CPU
/* Check if this cpu was hot-unplugged earlier and has siblings */
+ spin_lock_irqsave(&cpufreq_driver_lock, flags);
for_each_online_cpu(sibling) {
struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
for_each_online_cpu(sibling) {
struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
- if (cp && cpumask_test_cpu(cpu, cp->related_cpus))
+ if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) {
+ spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
return cpufreq_add_policy_cpu(cpu, sibling, dev);
return cpufreq_add_policy_cpu(cpu, sibling, dev);
+ spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
/* Initially set CPU itself as the policy_cpu */
per_cpu(cpufreq_policy_cpu, cpu) = cpu;
/* Initially set CPU itself as the policy_cpu */
per_cpu(cpufreq_policy_cpu, cpu) = cpu;
- ret = (lock_policy_rwsem_write(cpu) < 0);
- WARN_ON(ret);
init_completion(&policy->kobj_unregister);
INIT_WORK(&policy->update, handle_update);
init_completion(&policy->kobj_unregister);
INIT_WORK(&policy->update, handle_update);
ret = cpufreq_driver->init(policy);
if (ret) {
pr_debug("initialization failed\n");
ret = cpufreq_driver->init(policy);
if (ret) {
pr_debug("initialization failed\n");
- goto err_unlock_policy;
+ goto err_set_policy_cpu;
}
/* related cpus should atleast have policy->cpus */
}
/* related cpus should atleast have policy->cpus */
if (ret)
goto err_out_unregister;
if (ret)
goto err_out_unregister;
- unlock_policy_rwsem_write(cpu);
-
kobject_uevent(&policy->kobj, KOBJ_ADD);
module_put(cpufreq_driver->owner);
pr_debug("initialization complete\n");
kobject_uevent(&policy->kobj, KOBJ_ADD);
module_put(cpufreq_driver->owner);
pr_debug("initialization complete\n");
kobject_put(&policy->kobj);
wait_for_completion(&policy->kobj_unregister);
kobject_put(&policy->kobj);
wait_for_completion(&policy->kobj_unregister);
-err_unlock_policy:
- unlock_policy_rwsem_write(cpu);
+err_set_policy_cpu:
+ per_cpu(cpufreq_policy_cpu, cpu) = -1;
free_cpumask_var(policy->related_cpus);
err_free_cpumask:
free_cpumask_var(policy->cpus);
free_cpumask_var(policy->related_cpus);
err_free_cpumask:
free_cpumask_var(policy->cpus);
pr_debug("%s: unregistering CPU %u\n", __func__, cpu);
spin_lock_irqsave(&cpufreq_driver_lock, flags);
pr_debug("%s: unregistering CPU %u\n", __func__, cpu);
spin_lock_irqsave(&cpufreq_driver_lock, flags);
data = per_cpu(cpufreq_cpu_data, cpu);
data = per_cpu(cpufreq_cpu_data, cpu);
+ per_cpu(cpufreq_cpu_data, cpu) = NULL;
+
+ spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
if (!data) {
pr_debug("%s: No cpu_data found\n", __func__);
if (!data) {
pr_debug("%s: No cpu_data found\n", __func__);
- spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
- unlock_policy_rwsem_write(cpu);
CPUFREQ_NAME_LEN);
#endif
CPUFREQ_NAME_LEN);
#endif
- per_cpu(cpufreq_cpu_data, cpu) = NULL;
+ WARN_ON(lock_policy_rwsem_write(cpu));
cpus = cpumask_weight(data->cpus);
cpumask_clear_cpu(cpu, data->cpus);
cpus = cpumask_weight(data->cpus);
cpumask_clear_cpu(cpu, data->cpus);
+ unlock_policy_rwsem_write(cpu);
if (cpu != data->cpu) {
sysfs_remove_link(&dev->kobj, "cpufreq");
if (cpu != data->cpu) {
sysfs_remove_link(&dev->kobj, "cpufreq");
ret = kobject_move(&data->kobj, &cpu_dev->kobj);
if (ret) {
pr_err("%s: Failed to move kobj: %d", __func__, ret);
ret = kobject_move(&data->kobj, &cpu_dev->kobj);
if (ret) {
pr_err("%s: Failed to move kobj: %d", __func__, ret);
+
+ WARN_ON(lock_policy_rwsem_write(cpu));
cpumask_set_cpu(cpu, data->cpus);
cpumask_set_cpu(cpu, data->cpus);
- ret = sysfs_create_link(&cpu_dev->kobj, &data->kobj,
- "cpufreq");
+
+ spin_lock_irqsave(&cpufreq_driver_lock, flags);
+ per_cpu(cpufreq_cpu_data, cpu) = data;
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
unlock_policy_rwsem_write(cpu);
unlock_policy_rwsem_write(cpu);
+
+ ret = sysfs_create_link(&cpu_dev->kobj, &data->kobj,
+ "cpufreq");
+ WARN_ON(lock_policy_rwsem_write(cpu));
update_policy_cpu(data, cpu_dev->id);
update_policy_cpu(data, cpu_dev->id);
+ unlock_policy_rwsem_write(cpu);
pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n",
__func__, cpu_dev->id, cpu);
}
pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n",
__func__, cpu_dev->id, cpu);
}
- spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
-
pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
cpufreq_cpu_put(data);
pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
cpufreq_cpu_put(data);
- unlock_policy_rwsem_write(cpu);
/* If cpu is last user of policy, free policy */
if (cpus == 1) {
/* If cpu is last user of policy, free policy */
if (cpus == 1) {
- lock_policy_rwsem_write(cpu);
+ lock_policy_rwsem_read(cpu);
kobj = &data->kobj;
cmp = &data->kobj_unregister;
kobj = &data->kobj;
cmp = &data->kobj_unregister;
- unlock_policy_rwsem_write(cpu);
+ unlock_policy_rwsem_read(cpu);
kobject_put(kobj);
/* we need to make sure that the underlying kobj is actually
kobject_put(kobj);
/* we need to make sure that the underlying kobj is actually
wait_for_completion(cmp);
pr_debug("wait complete\n");
wait_for_completion(cmp);
pr_debug("wait complete\n");
- lock_policy_rwsem_write(cpu);
if (cpufreq_driver->exit)
cpufreq_driver->exit(data);
if (cpufreq_driver->exit)
cpufreq_driver->exit(data);
- unlock_policy_rwsem_write(cpu);
free_cpumask_var(data->related_cpus);
free_cpumask_var(data->cpus);
free_cpumask_var(data->related_cpus);
free_cpumask_var(data->cpus);
__cpufreq_governor(data, CPUFREQ_GOV_LIMITS);
}
__cpufreq_governor(data, CPUFREQ_GOV_LIMITS);
}
+ per_cpu(cpufreq_policy_cpu, cpu) = -1;
if (cpu_is_offline(cpu))
return 0;
if (cpu_is_offline(cpu))
return 0;
- if (unlikely(lock_policy_rwsem_write(cpu)))
- BUG();
-
retval = __cpufreq_remove_dev(dev, sif);
return retval;
}
retval = __cpufreq_remove_dev(dev, sif);
return retval;
}
break;
case CPU_DOWN_PREPARE:
case CPU_DOWN_PREPARE_FROZEN:
break;
case CPU_DOWN_PREPARE:
case CPU_DOWN_PREPARE_FROZEN:
- if (unlikely(lock_policy_rwsem_write(cpu)))
- BUG();
-
__cpufreq_remove_dev(dev, NULL);
break;
case CPU_DOWN_FAILED:
__cpufreq_remove_dev(dev, NULL);
break;
case CPU_DOWN_FAILED: