diff mbox

Kernel warning in cpufreq_add_dev()

Message ID 20160824131316.GI25143@ubuntu
State New
Headers show

Commit Message

Viresh Kumar Aug. 24, 2016, 1:13 p.m. UTC 
On 22-08-16, 19:32, Rafael J. Wysocki wrote:
> But it will be called in that path during physical CPU hot-add, won't it?


What about something like this instead (completely untested) ?

@Russell: Can you please try this ??

-- 
viresh

-------------------------8<-------------------------

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Viresh Kumar Aug. 31, 2016, 4:11 a.m. UTC | #1 
On 31-08-16, 03:26, Rafael J. Wysocki wrote:
> On Wednesday, August 24, 2016 06:43:16 PM Viresh Kumar wrote:

> > On 22-08-16, 19:32, Rafael J. Wysocki wrote:

> > > But it will be called in that path during physical CPU hot-add, won't it?

> > 

> > What about something like this instead (completely untested) ?

> 

> Inline, please?


I am not sure what that means. I pasted that inline in my previous mail only.

> > @Russell: Can you please try this ??

> 

> I was thinking about something similar, but won't the WARN_ON()s in

> cpufreq_add/remove_dev_symlink() still trigger, say if there's more

> than one CPU in a policy and both happen to be online initially?


real CPUs should already be online and their device structure should be
available, isn't it ?

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar Sept. 9, 2016, 9:57 a.m. UTC | #2 
On 31-08-16, 03:26, Rafael J. Wysocki wrote:
> I was thinking about something similar, but won't the WARN_ON()s in

> cpufreq_add/remove_dev_symlink() still trigger, say if there's more

> than one CPU in a policy and both happen to be online initially?


Right. I missed that. I have sent a patch just now in reply to the first email
from Russell. That should fix it all..

@Russell: Can you please test/verify the patch I have sent now?

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 3dd4884c6f9e..a702d6246385 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -916,20 +916,11 @@  static struct kobj_type ktype_cpufreq = {
 	.release	= cpufreq_sysfs_release,
 };
 
-static int add_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
+static int add_cpu_dev_symlink(struct cpufreq_policy *policy,
+			       struct device *dev)
 {
-	struct device *cpu_dev;
-
-	pr_debug("%s: Adding symlink for CPU: %u\n", __func__, cpu);
-
-	if (!policy)
-		return 0;
-
-	cpu_dev = get_cpu_device(cpu);
-	if (WARN_ON(!cpu_dev))
-		return 0;
-
-	return sysfs_create_link(&cpu_dev->kobj, &policy->kobj, "cpufreq");
+	dev_dbg(dev, "%s: Adding symlink\n", __func__);
+	return sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
 }
 
 static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
@@ -948,12 +939,17 @@  static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
 /* Add/remove symlinks for all related CPUs */
 static int cpufreq_add_dev_symlink(struct cpufreq_policy *policy)
 {
+	struct device *cpu_dev;
 	unsigned int j;
 	int ret = 0;
 
 	/* Some related CPUs might not be present (physically hotplugged) */
 	for_each_cpu(j, policy->real_cpus) {
-		ret = add_cpu_dev_symlink(policy, j);
+		cpu_dev = get_cpu_device(j);
+		if (WARN_ON(!cpu_dev))
+			continue;
+
+		ret = add_cpu_dev_symlink(policy, cpu_dev);
 		if (ret)
 			break;
 	}
@@ -1073,13 +1069,9 @@  static void handle_update(struct work_struct *work)
 
 static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
 {
-	struct device *dev = get_cpu_device(cpu);
 	struct cpufreq_policy *policy;
 	int ret;
 
-	if (WARN_ON(!dev))
-		return NULL;
-
 	policy = kzalloc(sizeof(*policy), GFP_KERNEL);
 	if (!policy)
 		return NULL;
@@ -1355,7 +1347,7 @@  static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
 	if (!policy || cpumask_test_and_set_cpu(cpu, policy->real_cpus))
 		return 0;
 
-	return add_cpu_dev_symlink(policy, cpu);
+	return add_cpu_dev_symlink(policy, dev);
 }
 
 static void cpufreq_offline(unsigned int cpu)