Consider a dual core (0/1) system with two CPUs:
- sharing clock/voltage rails and hence cpufreq-policy
- CPU1 is offline while the cpufreq driver is registered
- cpufreq_add_dev() is called from subsys callback for CPU0 and we
create the policy for the CPUs and create link for CPU1.
- cpufreq_add_dev() is called from subsys callback for CPU1, we find
that the cpu is offline and we try to create a sysfs link for CPU1.
- This results in double addition of the sysfs link and we get this:
WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x60/0x7c()
sysfs: cannot create duplicate filename
'/devices/system/cpu/cpu1/cpufreq'
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc2+ #1704
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[<c0013248>] (dump_backtrace) from [<c00133e4>] (show_stack+0x18/0x1c)
r6:c01a1f30 r5:0000001f r4:00000000 r3:00000000
[<c00133cc>] (show_stack) from [<c076920c>] (dump_stack+0x7c/0x98)
[<c0769190>] (dump_stack) from [<c0029ab4>]
(warn_slowpath_common+0x80/0xbc)
r4:d74abbd0 r3:d74c0000
[<c0029a34>] (warn_slowpath_common) from [<c0029b94>]
(warn_slowpath_fmt+0x38/0x40)
r8:ffffffef r7:00000000 r6:d75a8960 r5:c0993280 r4:d6b4d000
[<c0029b60>] (warn_slowpath_fmt) from [<c01a1f30>]
(sysfs_warn_dup+0x60/0x7c)
r3:d6b4dfe7 r2:c0930750
[<c01a1ed0>] (sysfs_warn_dup) from [<c01a22c8>]
(sysfs_do_create_link_sd+0xb8/0xc0)
r6:d75a8960 r5:c0993280 r4:d00aba20
[<c01a2210>] (sysfs_do_create_link_sd) from [<c01a22fc>]
(sysfs_create_link+0x2c/0x3c)
r10:00000001 r8:c14db3c8 r7:d7b89010 r6:c0ae7c60 r5:d7b89010
r4:d00d1200
[<c01a22d0>] (sysfs_create_link) from [<c0506160>]
(add_cpu_dev_symlink+0x34/0x5c)
[<c050612c>] (add_cpu_dev_symlink) from [<c05084d0>]
(cpufreq_add_dev+0x674/0x794)
r5:00000001 r4:00000000
[<c0507e5c>] (cpufreq_add_dev) from [<c03db114>]
(subsys_interface_register+0x8c/0xd0)
r10:00000003 r9:d7bb01f0 r8:c14db3c8 r7:00106738 r6:c0ae7c60
r5:c0acbd08
r4:c0ae7e20
[<c03db088>] (subsys_interface_register) from [<c0508a2c>]
(cpufreq_register_driver+0x104/0x1f4)
The check for offline-cpu in cpufreq_add_dev() is present to ensure that
link gets added for the CPUs, that weren't physically present earlier
and we missed the case where a CPU is offline while registering the
driver.
Fix this by keeping track of CPUs for which link is already created, and
avoiding duplicate sysfs entries.
Fixes: 87549141d516 ("cpufreq: Stop migrating sysfs files on hotplug")
Reported-by: Russell King <li...@arm.linux.org.uk>
Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
---
Russell,
Can you please give this a try? (completely untested).
drivers/cpufreq/cpufreq.c | 26 +++++++++++++++++++++++---
include/linux/cpufreq.h | 1 +
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index cdbe0676d246..12d089b78cad 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -984,17 +984,26 @@ EXPORT_SYMBOL(cpufreq_sysfs_remove_file);
static int add_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
{
struct device *cpu_dev;
+ int ret;
pr_debug("%s: Adding symlink for CPU: %u\n", __func__, cpu);
if (!policy)
return 0;
+ /* Already added for offline CPUS from subsys callback */
+ if (cpumask_test_cpu(cpu, policy->symlinks))
+ 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");
+ ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj, "cpufreq");
+ if (!ret)
+ cpumask_set_cpu(cpu, policy->symlinks);
+
+ return ret;
}
static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
@@ -1007,6 +1016,7 @@ static void remove_cpu_dev_symlink(struct cpufreq_policy
*policy, int cpu)
if (WARN_ON(!cpu_dev))
return;
+ cpumask_clear_cpu(cpu, policy->symlinks);
sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
}
@@ -1038,6 +1048,10 @@ static void cpufreq_remove_dev_symlink(struct
cpufreq_policy *policy)
if (j == policy->kobj_cpu)
continue;
+ /* Already removed for offline CPUS from subsys callback */
+ if (!cpumask_test_cpu(j, policy->symlinks))
+ continue;
+
remove_cpu_dev_symlink(policy, j);
}
}
@@ -1172,11 +1186,14 @@ static struct cpufreq_policy
*cpufreq_policy_alloc(struct device *dev)
if (!zalloc_cpumask_var(&policy->related_cpus, GFP_KERNEL))
goto err_free_cpumask;
+ if (!zalloc_cpumask_var(&policy->symlinks, GFP_KERNEL))
+ goto err_free_related_cpumask;
+
ret = kobject_init_and_add(&policy->kobj, &ktype_cpufreq, &dev->kobj,
"cpufreq");
if (ret) {
pr_err("%s: failed to init policy->kobj: %d\n", __func__, ret);
- goto err_free_rcpumask;
+ goto err_free_symlink_cpumask;
}
INIT_LIST_HEAD(&policy->policy_list);
@@ -1193,7 +1210,9 @@ static struct cpufreq_policy *cpufreq_policy_alloc(struct
device *dev)
return policy;
-err_free_rcpumask:
+err_free_symlink_cpumask:
+ free_cpumask_var(policy->symlinks);
+err_free_related_cpumask:
free_cpumask_var(policy->related_cpus);
err_free_cpumask:
free_cpumask_var(policy->cpus);
@@ -1243,6 +1262,7 @@ static void cpufreq_policy_free(struct cpufreq_policy
*policy, bool notify)
write_unlock_irqrestore(&cpufreq_driver_lock, flags);
cpufreq_policy_put_kobj(policy, notify);
+ free_cpumask_var(policy->symlinks);
free_cpumask_var(policy->related_cpus);
free_cpumask_var(policy->cpus);
kfree(policy);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a82049683016..b4812f6977c6 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -62,6 +62,7 @@ struct cpufreq_policy {
/* CPUs sharing clock, require sw coordination */
cpumask_var_t cpus; /* Online CPUs only */
cpumask_var_t related_cpus; /* Online + Offline CPUs */
+ cpumask_var_t symlinks; /* CPUs for which cpufreq sysfs
directory is present */
unsigned int shared_type; /* ACPI: ANY or ALL affected CPUs
should set cpufreq */
--
2.4.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
