All of lore.kernel.org
 help / color / mirror / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>,
	linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: Kernel warning in cpufreq_add_dev()
Date: Wed, 24 Aug 2016 18:43:16 +0530	[thread overview]
Message-ID: <20160824131316.GI25143@ubuntu> (raw)
In-Reply-To: <2310664.2BksGViL4r@vostro.rjw.lan>

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<-------------------------

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)

WARNING: multiple messages have this Message-ID (diff)
From: viresh.kumar@linaro.org (Viresh Kumar)
To: linux-arm-kernel@lists.infradead.org
Subject: Kernel warning in cpufreq_add_dev()
Date: Wed, 24 Aug 2016 18:43:16 +0530	[thread overview]
Message-ID: <20160824131316.GI25143@ubuntu> (raw)
In-Reply-To: <2310664.2BksGViL4r@vostro.rjw.lan>

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<-------------------------

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)

  reply	other threads:[~2016-08-24 13:13 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-19 11:00 Kernel warning in cpufreq_add_dev() Russell King - ARM Linux
2016-08-19 11:00 ` Russell King - ARM Linux
2016-08-20  1:29 ` Rafael J. Wysocki
2016-08-20  1:29   ` Rafael J. Wysocki
2016-08-20  1:46   ` Viresh Kumar
2016-08-20  1:46     ` Viresh Kumar
2016-08-22 17:32     ` Rafael J. Wysocki
2016-08-22 17:32       ` Rafael J. Wysocki
2016-08-24 13:13       ` Viresh Kumar [this message]
2016-08-24 13:13         ` Viresh Kumar
2016-08-31  1:26         ` Rafael J. Wysocki
2016-08-31  1:26           ` Rafael J. Wysocki
2016-08-31  4:11           ` Viresh Kumar
2016-08-31  4:11             ` Viresh Kumar
2016-08-31 11:58             ` Rafael J. Wysocki
2016-08-31 11:58               ` Rafael J. Wysocki
2016-09-09  9:57           ` Viresh Kumar
2016-09-09  9:57             ` Viresh Kumar
2016-09-09  9:54 ` [PATCH] cpufreq: create link to policy only for registered CPUs Viresh Kumar
2016-09-09  9:54   ` Viresh Kumar
2016-09-09 11:16   ` Russell King - ARM Linux
2016-09-09 11:16     ` Russell King - ARM Linux
2016-09-09 11:22     ` Viresh Kumar
2016-09-09 11:22       ` Viresh Kumar
2016-09-09 11:28       ` Russell King - ARM Linux
2016-09-09 11:28         ` Russell King - ARM Linux
2016-09-09 11:34         ` Viresh Kumar
2016-09-09 11:34           ` Viresh Kumar
2016-09-09 12:53           ` Russell King - ARM Linux
2016-09-09 12:53             ` Russell King - ARM Linux
2016-09-12  6:37 ` [PATCH V2] " Viresh Kumar
2016-09-12  6:37   ` Viresh Kumar
2016-09-14  1:00   ` Rafael J. Wysocki
2016-09-14  1:00     ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160824131316.GI25143@ubuntu \
    --to=viresh.kumar@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.