* [PATCH] cpufreq: remove sysfs files for CPU which failed to come back after resume
@ 2013-12-20 5:08 Viresh Kumar
2013-12-20 15:14 ` Rafael J. Wysocki
0 siblings, 1 reply; 3+ messages in thread
From: Viresh Kumar @ 2013-12-20 5:08 UTC (permalink / raw)
To: rjw
Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
srivatsa.bhat, Viresh Kumar
There are cases where cpufreq_add_dev() may fail for some CPUs during resume.
With the current code we will still have sysfs cpufreq files for such CPUs, and
struct cpufreq_policy would be already freed for them. Hence any operation on
those sysfs files would result in kernel warnings.
To fix this, lets remove those sysfs files or put the associated kobject in case
of such errors. Also, to make it simple lets remove the sysfs links from all the
CPUs (except policy->cpu) during suspend as that operation wouldn't result with a
loss of sysfs file permissions. And so we will create those links during resume
as well.
Reported-and-tested-by: Bjørn Mork <bjorn@mork.no>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 61 ++++++++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 02d534d..cea96c9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -845,8 +845,7 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy)
#ifdef CONFIG_HOTPLUG_CPU
static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
- unsigned int cpu, struct device *dev,
- bool frozen)
+ unsigned int cpu, struct device *dev)
{
int ret = 0;
unsigned long flags;
@@ -877,9 +876,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
}
}
- /* Don't touch sysfs links during light-weight init */
- if (!frozen)
- ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
+ ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
return ret;
}
@@ -926,6 +923,27 @@ err_free_policy:
return NULL;
}
+static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
+{
+ struct kobject *kobj;
+ struct completion *cmp;
+
+ down_read(&policy->rwsem);
+ kobj = &policy->kobj;
+ cmp = &policy->kobj_unregister;
+ up_read(&policy->rwsem);
+ kobject_put(kobj);
+
+ /*
+ * We need to make sure that the underlying kobj is
+ * actually not referenced anymore by anybody before we
+ * proceed with unloading.
+ */
+ pr_debug("waiting for dropping of refcount\n");
+ wait_for_completion(cmp);
+ pr_debug("wait complete\n");
+}
+
static void cpufreq_policy_free(struct cpufreq_policy *policy)
{
free_cpumask_var(policy->related_cpus);
@@ -986,7 +1004,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
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);
- ret = cpufreq_add_policy_cpu(tpolicy, cpu, dev, frozen);
+ ret = cpufreq_add_policy_cpu(tpolicy, cpu, dev);
up_read(&cpufreq_rwsem);
return ret;
}
@@ -1096,7 +1114,10 @@ err_get_freq:
if (cpufreq_driver->exit)
cpufreq_driver->exit(policy);
err_set_policy_cpu:
+ if (frozen)
+ cpufreq_policy_put_kobj(policy);
cpufreq_policy_free(policy);
+
nomem_out:
up_read(&cpufreq_rwsem);
@@ -1118,7 +1139,7 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
}
static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
- unsigned int old_cpu, bool frozen)
+ unsigned int old_cpu)
{
struct device *cpu_dev;
int ret;
@@ -1126,10 +1147,6 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
/* first sibling now owns the new sysfs dir */
cpu_dev = get_cpu_device(cpumask_any_but(policy->cpus, old_cpu));
- /* Don't touch sysfs files during light-weight tear-down */
- if (frozen)
- return cpu_dev->id;
-
sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
ret = kobject_move(&policy->kobj, &cpu_dev->kobj);
if (ret) {
@@ -1196,7 +1213,7 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
if (!frozen)
sysfs_remove_link(&dev->kobj, "cpufreq");
} else if (cpus > 1) {
- new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen);
+ new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu);
if (new_cpu >= 0) {
update_policy_cpu(policy, new_cpu);
@@ -1218,8 +1235,6 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
int ret;
unsigned long flags;
struct cpufreq_policy *policy;
- struct kobject *kobj;
- struct completion *cmp;
read_lock_irqsave(&cpufreq_driver_lock, flags);
policy = per_cpu(cpufreq_cpu_data, cpu);
@@ -1249,22 +1264,8 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
}
}
- if (!frozen) {
- down_read(&policy->rwsem);
- kobj = &policy->kobj;
- cmp = &policy->kobj_unregister;
- up_read(&policy->rwsem);
- kobject_put(kobj);
-
- /*
- * We need to make sure that the underlying kobj is
- * actually not referenced anymore by anybody before we
- * proceed with unloading.
- */
- pr_debug("waiting for dropping of refcount\n");
- wait_for_completion(cmp);
- pr_debug("wait complete\n");
- }
+ if (!frozen)
+ cpufreq_policy_put_kobj(policy);
/*
* Perform the ->exit() even during light-weight tear-down,
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cpufreq: remove sysfs files for CPU which failed to come back after resume
2013-12-20 5:08 [PATCH] cpufreq: remove sysfs files for CPU which failed to come back after resume Viresh Kumar
@ 2013-12-20 15:14 ` Rafael J. Wysocki
2013-12-20 15:56 ` Viresh Kumar
0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2013-12-20 15:14 UTC (permalink / raw)
To: Viresh Kumar
Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel, srivatsa.bhat
On Friday, December 20, 2013 10:38:20 AM Viresh Kumar wrote:
> There are cases where cpufreq_add_dev() may fail for some CPUs during resume.
> With the current code we will still have sysfs cpufreq files for such CPUs, and
> struct cpufreq_policy would be already freed for them. Hence any operation on
> those sysfs files would result in kernel warnings.
>
> To fix this, lets remove those sysfs files or put the associated kobject in case
> of such errors. Also, to make it simple lets remove the sysfs links from all the
> CPUs (except policy->cpu) during suspend as that operation wouldn't result with a
> loss of sysfs file permissions. And so we will create those links during resume
> as well.
>
> Reported-and-tested-by: Bjørn Mork <bjorn@mork.no>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 61 ++++++++++++++++++++++++-----------------------
> 1 file changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 02d534d..cea96c9 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -845,8 +845,7 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy)
>
> #ifdef CONFIG_HOTPLUG_CPU
> static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
> - unsigned int cpu, struct device *dev,
> - bool frozen)
> + unsigned int cpu, struct device *dev)
> {
> int ret = 0;
> unsigned long flags;
> @@ -877,9 +876,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
> }
> }
>
> - /* Don't touch sysfs links during light-weight init */
> - if (!frozen)
> - ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
> + ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
>
> return ret;
> }
Well, this just looks odd. Please do
return sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
And I'm fine with the rest of the patch.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cpufreq: remove sysfs files for CPU which failed to come back after resume
2013-12-20 15:14 ` Rafael J. Wysocki
@ 2013-12-20 15:56 ` Viresh Kumar
0 siblings, 0 replies; 3+ messages in thread
From: Viresh Kumar @ 2013-12-20 15:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Lists linaro-kernel, Patch Tracking, cpufreq, linux-pm,
Linux Kernel Mailing List, Srivatsa S. Bhat
On 20 December 2013 20:44, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> Well, this just looks odd. Please do
>
> return sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
Yeah, that was stupid :)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-12-20 15:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-20 5:08 [PATCH] cpufreq: remove sysfs files for CPU which failed to come back after resume Viresh Kumar
2013-12-20 15:14 ` Rafael J. Wysocki
2013-12-20 15:56 ` Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®