From 9c8f1ee40b6368e6b2775c9c9f816e2a5dca3c07 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 12 Sep 2013 17:06:33 +0530 Subject: cpufreq: Clear policy->cpus bits in __cpufreq_remove_dev_finish() This broke after a recent change "cedb70a cpufreq: Split __cpufreq_remove_dev() into two parts" from Srivatsa. Consider a scenario where we have two CPUs in a policy (0 & 1) and we are removing CPU 1. On the call to __cpufreq_remove_dev_prepare() we have cleared 1 from policy->cpus and now on a call to __cpufreq_remove_dev_finish() we read cpumask_weight of policy->cpus, which will come as 1 and this code will behave as if we are removing the last CPU from policy :) Fix it by clearing the CPU mask in __cpufreq_remove_dev_finish() instead of __cpufreq_remove_dev_prepare(). Tested-by: Stephen Warren Reviewed-by: Srivatsa S. Bhat Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 43c24aa756f6..dbfe219667d3 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1125,7 +1125,7 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy, int ret; /* first sibling now owns the new sysfs dir */ - cpu_dev = get_cpu_device(cpumask_first(policy->cpus)); + cpu_dev = get_cpu_device(cpumask_any_but(policy->cpus, old_cpu)); /* Don't touch sysfs files during light-weight tear-down */ if (frozen) @@ -1189,12 +1189,9 @@ static int __cpufreq_remove_dev_prepare(struct device *dev, policy->governor->name, CPUFREQ_NAME_LEN); #endif - WARN_ON(lock_policy_rwsem_write(cpu)); + lock_policy_rwsem_read(cpu); cpus = cpumask_weight(policy->cpus); - - if (cpus > 1) - cpumask_clear_cpu(cpu, policy->cpus); - unlock_policy_rwsem_write(cpu); + unlock_policy_rwsem_read(cpu); if (cpu != policy->cpu) { if (!frozen) @@ -1237,9 +1234,12 @@ static int __cpufreq_remove_dev_finish(struct device *dev, return -EINVAL; } - lock_policy_rwsem_read(cpu); + WARN_ON(lock_policy_rwsem_write(cpu)); cpus = cpumask_weight(policy->cpus); - unlock_policy_rwsem_read(cpu); + + if (cpus > 1) + cpumask_clear_cpu(cpu, policy->cpus); + unlock_policy_rwsem_write(cpu); /* If cpu is last user of policy, free policy */ if (cpus == 1) { -- cgit v1.2.3 From 8efd57657d8ef666810b55e609da72de92314dc4 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 17 Sep 2013 10:22:11 +0530 Subject: cpufreq: unlock correct rwsem while updating policy->cpu Current code looks like this: WARN_ON(lock_policy_rwsem_write(cpu)); update_policy_cpu(policy, new_cpu); unlock_policy_rwsem_write(cpu); {lock|unlock}_policy_rwsem_write(cpu) takes/releases policy->cpu's rwsem. Because cpu is changing with the call to update_policy_cpu(), the unlock_policy_rwsem_write() will release the incorrect lock. The right solution would be to release the same lock as was taken earlier. Also update_policy_cpu() was also called from cpufreq_add_dev() without any locks and so its better if we move this locking to inside update_policy_cpu(). This patch fixes a regression introduced in 3.12 by commit f9ba680d23 (cpufreq: Extract the handover of policy cpu to a helper function). Reported-and-tested-by: Jon Medhurst Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index dbfe219667d3..82ecbe39dfb0 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -952,9 +952,20 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu) if (cpu == policy->cpu) return; + /* + * Take direct locks as lock_policy_rwsem_write wouldn't work here. + * Also lock for last cpu is enough here as contention will happen only + * after policy->cpu is changed and after it is changed, other threads + * will try to acquire lock for new cpu. And policy is already updated + * by then. + */ + down_write(&per_cpu(cpu_policy_rwsem, policy->cpu)); + policy->last_cpu = policy->cpu; policy->cpu = cpu; + up_write(&per_cpu(cpu_policy_rwsem, policy->last_cpu)); + #ifdef CONFIG_CPU_FREQ_TABLE cpufreq_frequency_table_update_policy_cpu(policy); #endif @@ -1200,9 +1211,7 @@ static int __cpufreq_remove_dev_prepare(struct device *dev, new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen); if (new_cpu >= 0) { - WARN_ON(lock_policy_rwsem_write(cpu)); update_policy_cpu(policy, new_cpu); - unlock_policy_rwsem_write(cpu); if (!frozen) { pr_debug("%s: policy Kobject moved to cpu: %d " -- cgit v1.2.3 From e1825b25309264bf2d1275a8da9c997bca39b0d8 Mon Sep 17 00:00:00 2001 From: Sudeep KarkadaNagesha Date: Tue, 10 Sep 2013 18:59:46 +0100 Subject: cpufreq: cpufreq-cpu0: assign cpu_dev correctly to cpu0 device Commit f837a9b5ab05c52a07108c6f09ca66f2e0aee757 "cpufreq: cpufreq-cpu0: remove device tree parsing for cpu nodes" assumed the pdev->dev is set to cpu0 device in the platform code. But it actually points to the virtual cpufreq-cpu0 platform device which is not present in the device tree. Most of the information needed by cpufreq is stored in cpu0 DT node. So cpu_dev must point to cpu0 device. This patch fixes the wrong assignment to cpu_dev. Reported-and-tested-by: Guennadi Liakhovetski Cc: Shawn Guo Signed-off-by: Sudeep KarkadaNagesha Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq-cpu0.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index cbfffa91ebdd..78c49d8e0f4a 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -12,6 +12,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include +#include #include #include #include @@ -177,7 +178,11 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) struct device_node *np; int ret; - cpu_dev = &pdev->dev; + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("failed to get cpu0 device\n"); + return -ENODEV; + } np = of_node_get(cpu_dev->of_node); if (!np) { -- cgit v1.2.3 From b494b48dacb4dd848d76bc8871e55716486f0253 Mon Sep 17 00:00:00 2001 From: Sudeep KarkadaNagesha Date: Tue, 10 Sep 2013 18:59:47 +0100 Subject: cpufreq: imx6q-cpufreq: assign cpu_dev correctly to cpu0 device Commit cdc58d602d2e657602a90c190cbf745886c95977 "cpufreq: imx6q-cpufreq: remove device tree parsing for cpu nodes" assumed the pdev->dev is set to cpu0 device in the platform code. But it actually points to the virtual cpufreq-cpu0 platform device which is not present in the device tree. Most of the information needed by cpufreq is stored in cpu0 DT node. So cpu_dev must point to cpu0 device. This patch fixes the wrong assignment to cpu_dev. Reported-by: Guennadi Liakhovetski Tested-by: Shawn Guo Signed-off-by: Sudeep KarkadaNagesha Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/imx6q-cpufreq.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c index 3e396543aea4..c3fd2a101ca0 100644 --- a/drivers/cpufreq/imx6q-cpufreq.c +++ b/drivers/cpufreq/imx6q-cpufreq.c @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -202,7 +203,11 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) unsigned long min_volt, max_volt; int num, ret; - cpu_dev = &pdev->dev; + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("failed to get cpu0 device\n"); + return -ENODEV; + } np = of_node_get(cpu_dev->of_node); if (!np) { -- cgit v1.2.3 From 4dea5806d332f91d640d99943db99a5539e832c3 Mon Sep 17 00:00:00 2001 From: Yinghai Lu Date: Wed, 18 Sep 2013 21:05:20 -0700 Subject: cpufreq: return EEXIST instead of EBUSY for second registering On systems that support intel_pstate, acpi_cpufreq fails to load, and udev keeps trying until trace gets filled up and kernel crashes. The root cause is driver return ret from cpufreq_register_driver(), because when some other driver takes over before, it will return EBUSY and then udev will keep trying ... cpufreq_register_driver() should return EEXIST instead so that the system can boot without appending intel_pstate=disable and still use intel_pstate. Signed-off-by: Yinghai Lu Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 82ecbe39dfb0..89b3c52cd5c3 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2104,7 +2104,7 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data) write_lock_irqsave(&cpufreq_driver_lock, flags); if (cpufreq_driver) { write_unlock_irqrestore(&cpufreq_driver_lock, flags); - return -EBUSY; + return -EEXIST; } cpufreq_driver = driver_data; write_unlock_irqrestore(&cpufreq_driver_lock, flags); -- cgit v1.2.3