From: Rafael J. Wysocki <[email protected]>

The cpufreq core is a little inconsistent in the way it uses the
driver module refcount.

Namely, if __cpufreq_add_dev() is called for a CPU without siblings
or generally a CPU for which a new policy object has to be created,
it grabs a reference to the driver module to start with, but drops
that reference before returning.  As a result, the driver module
refcount is then equal to 0 after __cpufreq_add_dev() has returned.

On the other hand, if the given CPU is a sibling of some other
CPU already having a policy, cpufreq_add_policy_cpu() is called
to link the new CPU to the existing policy.  In that case,
cpufreq_cpu_get() is called to obtain that policy and grabs a
reference to the driver module, but that reference is not
released and the module refcount will be different from 0 after
__cpufreq_add_dev() returns (unless there is an error).  That
prevents the driver module from being unloaded until
__cpufreq_remove_dev() is called for all the CPUs that
cpufreq_add_policy_cpu() was called for previously.

To remove that inconsistency make cpufreq_add_policy_cpu() execute
cpufreq_cpu_put() for the given policy before returning, which
decrements the driver module refcount so that it will be 0 after
__cpufreq_add_dev() returns, but also make it take a reference to
the policy itself using kobject_get() and do not release that
reference (unless there's an error or system resume is under way),
which again is consistent with the "raw" __cpufreq_add_dev()
behavior.

Accordingly, modify __cpufreq_remove_dev() to use kobject_put() to
drop policy references taken by cpufreq_add_policy_cpu().

Signed-off-by: Rafael J. Wysocki <[email protected]>
---

On top of current linux-pm.git/linux-next.

Thanks,
Rafael

---
 drivers/cpufreq/cpufreq.c |   24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -908,8 +908,10 @@ static int cpufreq_add_policy_cpu(unsign
        unsigned long flags;
 
        policy = cpufreq_cpu_get(sibling);
-       WARN_ON(!policy);
+       if (WARN_ON_ONCE(!policy))
+               return -ENODATA;
 
+       kobject_get(&policy->kobj);
        if (has_target)
                __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
 
@@ -932,14 +934,14 @@ static int cpufreq_add_policy_cpu(unsign
        /* Don't touch sysfs links during light-weight init */
        if (frozen) {
                /* Drop the extra refcount that we took above */
-               cpufreq_cpu_put(policy);
-               return 0;
+               kobject_put(&policy->kobj);
+       } else {
+               ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
+               if (ret)
+                       kobject_put(&policy->kobj);
        }
 
-       ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
-       if (ret)
-               cpufreq_cpu_put(policy);
-
+       cpufreq_cpu_put(policy);
        return ret;
 }
 #endif
@@ -1298,10 +1300,14 @@ static int __cpufreq_remove_dev(struct d
                if (!frozen)
                        cpufreq_policy_free(data);
        } else {
-
+               /*
+                * There are more CPUs using the same policy, so only drop the
+                * reference taken by cpufreq_add_policy_cpu() (unless the
+                * system is suspending).
+                */
                if (!frozen) {
                        pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
-                       cpufreq_cpu_put(data);
+                       kobject_put(&data->kobj);
                }
 
                if (cpufreq_driver->target) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to