cpufreq: Use rwsem for protecting critical sections

Critical sections of the cpufreq core are protected with the help of
the driver module owner's refcount, which isn't the correct approach,
because it causes rmmod to return an error when some routine has
updated that refcount.

Let's use rwsem for this purpose instead.  Only
cpufreq_unregister_driver() will use write sem
and everybody else will use read sem.

[rjw: Subject & changelog]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f149e14f77c7a0405ec333f9a684cd5cb2809810..c9bbfeef92e355c11189903215942ebb891cda8a 100644 (file)
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -91,6 +91,12 @@ static void unlock_policy_rwsem_##mode(int cpu)                              \
 unlock_policy_rwsem(read, cpu);
 unlock_policy_rwsem(write, cpu);
 
+/*
+ * rwsem to guarantee that cpufreq driver module doesn't unload during critical
+ * sections
+ */
+static DECLARE_RWSEM(cpufreq_rwsem);
+
 /* internal prototypes */
 static int __cpufreq_governor(struct cpufreq_policy *policy,
                unsigned int event);
@@ -178,78 +184,46 @@ u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy)
 }
 EXPORT_SYMBOL_GPL(get_cpu_idle_time);
 
-static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs)
+struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
 {
-       struct cpufreq_policy *policy;
+       struct cpufreq_policy *policy = NULL;
        unsigned long flags;
 
-       if (cpu >= nr_cpu_ids)
-               goto err_out;
+       if (cpufreq_disabled() || (cpu >= nr_cpu_ids))
+               return NULL;
+
+       if (!down_read_trylock(&cpufreq_rwsem))
+               return NULL;
 
        /* get the cpufreq driver */
        read_lock_irqsave(&cpufreq_driver_lock, flags);
 
-       if (!cpufreq_driver)
-               goto err_out_unlock;
-
-       if (!try_module_get(cpufreq_driver->owner))
-               goto err_out_unlock;
+       if (cpufreq_driver) {
+               /* get the CPU */
+               policy = per_cpu(cpufreq_cpu_data, cpu);
+               if (policy)
+                       kobject_get(&policy->kobj);
+       }
 
-       /* get the CPU */
-       policy = per_cpu(cpufreq_cpu_data, cpu);
+       read_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
        if (!policy)
-               goto err_out_put_module;
-
-       if (!sysfs && !kobject_get(&policy->kobj))
-               goto err_out_put_module;
+               up_read(&cpufreq_rwsem);
 
-       read_unlock_irqrestore(&cpufreq_driver_lock, flags);
        return policy;
-
-err_out_put_module:
-       module_put(cpufreq_driver->owner);
-err_out_unlock:
-       read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-err_out:
-       return NULL;
-}
-
-struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
-{
-       if (cpufreq_disabled())
-               return NULL;
-
-       return __cpufreq_cpu_get(cpu, false);
 }
 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 *policy, bool sysfs)
-{
-       if (!sysfs)
-               kobject_put(&policy->kobj);
-       module_put(cpufreq_driver->owner);
-}
-
 void cpufreq_cpu_put(struct cpufreq_policy *policy)
 {
        if (cpufreq_disabled())
                return;
 
-       __cpufreq_cpu_put(policy, false);
+       kobject_put(&policy->kobj);
+       up_read(&cpufreq_rwsem);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cpu_put);
 
-static void cpufreq_cpu_put_sysfs(struct cpufreq_policy *policy)
-{
-       __cpufreq_cpu_put(policy, true);
-}
-
 /*********************************************************************
  *            EXTERNALLY AFFECTING FREQUENCY CHANGES                 *
  *********************************************************************/
@@ -694,12 +668,12 @@ 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_sysfs(policy->cpu);
-       if (!policy)
-               goto no_policy;
+
+       if (!down_read_trylock(&cpufreq_rwsem))
+               goto exit;
 
        if (lock_policy_rwsem_read(policy->cpu) < 0)
-               goto fail;
+               goto up_read;
 
        if (fattr->show)
                ret = fattr->show(policy, buf);
@@ -707,9 +681,10 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
                ret = -EIO;
 
        unlock_policy_rwsem_read(policy->cpu);
-fail:
-       cpufreq_cpu_put_sysfs(policy);
-no_policy:
+
+up_read:
+       up_read(&cpufreq_rwsem);
+exit:
        return ret;
 }
 
@@ -719,12 +694,12 @@ 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_sysfs(policy->cpu);
-       if (!policy)
-               goto no_policy;
+
+       if (!down_read_trylock(&cpufreq_rwsem))
+               goto exit;
 
        if (lock_policy_rwsem_write(policy->cpu) < 0)
-               goto fail;
+               goto up_read;
 
        if (fattr->store)
                ret = fattr->store(policy, buf, count);
@@ -732,9 +707,10 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
                ret = -EIO;
 
        unlock_policy_rwsem_write(policy->cpu);
-fail:
-       cpufreq_cpu_put_sysfs(policy);
-no_policy:
+
+up_read:
+       up_read(&cpufreq_rwsem);
+exit:
        return ret;
 }
 
@@ -1003,25 +979,24 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
                return 0;
        }
 
+       if (!down_read_trylock(&cpufreq_rwsem))
+               return 0;
+
 #ifdef CONFIG_HOTPLUG_CPU
        /* Check if this cpu was hot-unplugged earlier and has siblings */
        read_lock_irqsave(&cpufreq_driver_lock, flags);
        list_for_each_entry(tpolicy, &cpufreq_policy_list, policy_list) {
                if (cpumask_test_cpu(cpu, tpolicy->related_cpus)) {
                        read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-                       return cpufreq_add_policy_cpu(tpolicy, cpu, dev,
-                                       frozen);
+                       ret = cpufreq_add_policy_cpu(tpolicy, cpu, dev, frozen);
+                       up_read(&cpufreq_rwsem);
+                       return ret;
                }
        }
        read_unlock_irqrestore(&cpufreq_driver_lock, flags);
 #endif
 #endif
 
-       if (!try_module_get(cpufreq_driver->owner)) {
-               ret = -EINVAL;
-               goto module_out;
-       }
-
        if (frozen)
                /* Restore the saved policy when doing light-weight init */
                policy = cpufreq_policy_restore(cpu);
@@ -1094,7 +1069,8 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
        cpufreq_init_policy(policy);
 
        kobject_uevent(&policy->kobj, KOBJ_ADD);
-       module_put(cpufreq_driver->owner);
+       up_read(&cpufreq_rwsem);
+
        pr_debug("initialization complete\n");
 
        return 0;
@@ -1112,8 +1088,8 @@ err_set_policy_cpu:
        per_cpu(cpufreq_policy_cpu, cpu) = -1;
        cpufreq_policy_free(policy);
 nomem_out:
-       module_put(cpufreq_driver->owner);
-module_out:
+       up_read(&cpufreq_rwsem);
+
        return ret;
 }
 
@@ -1425,10 +1401,9 @@ static unsigned int __cpufreq_get(unsigned int cpu)
 unsigned int cpufreq_get(unsigned int cpu)
 {
        unsigned int ret_freq = 0;
-       struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
 
-       if (!policy)
-               goto out;
+       if (!down_read_trylock(&cpufreq_rwsem))
+               return 0;
 
        if (unlikely(lock_policy_rwsem_read(cpu)))
                goto out_policy;
@@ -1438,8 +1413,8 @@ unsigned int cpufreq_get(unsigned int cpu)
        unlock_policy_rwsem_read(cpu);
 
 out_policy:
-       cpufreq_cpu_put(policy);
-out:
+       up_read(&cpufreq_rwsem);
+
        return ret_freq;
 }
 EXPORT_SYMBOL(cpufreq_get);
@@ -2132,9 +2107,13 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver)
        subsys_interface_unregister(&cpufreq_interface);
        unregister_hotcpu_notifier(&cpufreq_cpu_notifier);
 
+       down_write(&cpufreq_rwsem);
        write_lock_irqsave(&cpufreq_driver_lock, flags);
+
        cpufreq_driver = NULL;
+
        write_unlock_irqrestore(&cpufreq_driver_lock, flags);
+       up_write(&cpufreq_rwsem);
 
        return 0;
 }