We call __find_governor() during addition of first CPU of every policy from __cpufreq_add_dev() to find the last governor used for this CPU before it was hotplugged-out.
After that we call cpufreq_parse_governor() in cpufreq_init_policy() either with this governor or default governor. And right after that policy->governor is set to NULL.
There is no functional problem here with this code, but it is just that some of the code required in cpufreq_init_policy() is being called by its caller, i.e. __cpufreq_add_dev(). So, it would make more sense to get all these together at a single place to make code more readable.
So, instead of doing this move the relevant parts to cpufreq_init_policy() policy only and initialize policy->governor to NULL at the beginning.
In order to clean the code a bit more, some of the #ifdefs for CONFIG_HOTPLUG_CPU are also removed which used to maintain the last governor used for any CPU. Keeping that code wouldn't harm us much but would improve the cleanliness of code.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- V1->V2: - Improved commit logs - Removed unrelated whitespace changes - Removed some #ifdef CONFIG_HOTPLUG_CPU to clean code a bit.
drivers/cpufreq/cpufreq.c | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 56b7b1b..fff2968 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -42,10 +42,8 @@ static DEFINE_RWLOCK(cpufreq_driver_lock); DEFINE_MUTEX(cpufreq_governor_lock); static LIST_HEAD(cpufreq_policy_list);
-#ifdef CONFIG_HOTPLUG_CPU /* This one keeps track of the previously set governor of a removed CPU */ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor); -#endif
static inline bool has_target(void) { @@ -879,18 +877,25 @@ err_out_kobj_put:
static void cpufreq_init_policy(struct cpufreq_policy *policy) { + struct cpufreq_governor *gov = NULL; struct cpufreq_policy new_policy; int ret = 0;
memcpy(&new_policy, policy, sizeof(*policy));
+ /* Update governor of new_policy to the governor used before hotplug */ + gov = __find_governor(per_cpu(cpufreq_cpu_governor, policy->cpu)); + if (gov) + pr_debug("Restoring governor %s for cpu %d\n", + policy->governor->name, policy->cpu); + else + gov = CPUFREQ_DEFAULT_GOVERNOR; + + new_policy.governor = gov; + /* Use the default policy if its valid. */ if (cpufreq_driver->setpolicy) - cpufreq_parse_governor(policy->governor->name, - &new_policy.policy, NULL); - - /* assure that the starting sequence is run in cpufreq_set_policy */ - policy->governor = NULL; + cpufreq_parse_governor(gov->name, &new_policy.policy, NULL);
/* set default policy */ ret = cpufreq_set_policy(policy, &new_policy); @@ -949,6 +954,8 @@ static struct cpufreq_policy *cpufreq_policy_restore(unsigned int cpu)
read_unlock_irqrestore(&cpufreq_driver_lock, flags);
+ policy->governor = NULL; + return policy; }
@@ -1036,7 +1043,6 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, unsigned long flags; #ifdef CONFIG_HOTPLUG_CPU struct cpufreq_policy *tpolicy; - struct cpufreq_governor *gov; #endif
if (cpu_is_offline(cpu)) @@ -1094,7 +1100,6 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, else policy->cpu = cpu;
- policy->governor = CPUFREQ_DEFAULT_GOVERNOR; cpumask_copy(policy->cpus, cpumask_of(cpu));
init_completion(&policy->kobj_unregister); @@ -1179,15 +1184,6 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, blocking_notifier_call_chain(&cpufreq_policy_notifier_list, CPUFREQ_START, policy);
-#ifdef CONFIG_HOTPLUG_CPU - gov = __find_governor(per_cpu(cpufreq_cpu_governor, cpu)); - if (gov) { - policy->governor = gov; - pr_debug("Restoring governor %s for cpu %d\n", - policy->governor->name, cpu); - } -#endif - if (!frozen) { ret = cpufreq_add_dev_interface(policy, dev); if (ret) @@ -1312,11 +1308,9 @@ static int __cpufreq_remove_dev_prepare(struct device *dev, } }
-#ifdef CONFIG_HOTPLUG_CPU if (!cpufreq_driver->setpolicy) strncpy(per_cpu(cpufreq_cpu_governor, cpu), policy->governor->name, CPUFREQ_NAME_LEN); -#endif
down_read(&policy->rwsem); cpus = cpumask_weight(policy->cpus); @@ -1955,9 +1949,7 @@ EXPORT_SYMBOL_GPL(cpufreq_register_governor);
void cpufreq_unregister_governor(struct cpufreq_governor *governor) { -#ifdef CONFIG_HOTPLUG_CPU int cpu; -#endif
if (!governor) return; @@ -1965,14 +1957,12 @@ void cpufreq_unregister_governor(struct cpufreq_governor *governor) if (cpufreq_disabled()) return;
-#ifdef CONFIG_HOTPLUG_CPU for_each_present_cpu(cpu) { if (cpu_online(cpu)) continue; if (!strcmp(per_cpu(cpufreq_cpu_governor, cpu), governor->name)) strcpy(per_cpu(cpufreq_cpu_governor, cpu), "\0"); } -#endif
mutex_lock(&cpufreq_governor_mutex); list_del(&governor->governor_list);
Policy must be fully initialized before it is being made available for use by others. Otherwise cpufreq_cpu_get() would be able to grab a half initialized policy structure that might not have affected_cpus (for example) filled. And so anybody accessing those fields will get the wrong value and hence the results would be unpredictable.
So, in order to fix this lets do all the necessary initialization before we make policy structure available via cpufreq_cpu_get(). With this we can guarantee that any code accessing fields of policy would be stable enough, as policy would be completely initialized by now.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- V1->V2: - Improved commit logs
drivers/cpufreq/cpufreq.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index fff2968..3c6f9a5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1114,6 +1114,20 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, goto err_set_policy_cpu; }
+ /* related cpus should atleast have policy->cpus */ + cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus); + + /* + * affected cpus must always be the one, which are online. We aren't + * managing offline cpus here. + */ + cpumask_and(policy->cpus, policy->cpus, cpu_online_mask); + + if (!frozen) { + policy->user_policy.min = policy->min; + policy->user_policy.max = policy->max; + } + write_lock_irqsave(&cpufreq_driver_lock, flags); for_each_cpu(j, policy->cpus) per_cpu(cpufreq_cpu_data, j) = policy; @@ -1167,20 +1181,6 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, } }
- /* related cpus should atleast have policy->cpus */ - cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus); - - /* - * affected cpus must always be the one, which are online. We aren't - * managing offline cpus here. - */ - cpumask_and(policy->cpus, policy->cpus, cpu_online_mask); - - if (!frozen) { - policy->user_policy.min = policy->min; - policy->user_policy.max = policy->max; - } - blocking_notifier_call_chain(&cpufreq_policy_notifier_list, CPUFREQ_START, policy);
policy->rwsem is used to lock access to all parts of code modifying struct cpufreq_policy but wasn't used on a new policy created from __cpufreq_add_dev().
Because of which if we call cpufreq_update_policy() repeatedly on one CPU and do offline/online of another CPU then we might see these crashes:
Unable to handle kernel NULL pointer dereference at virtual address 00000020 pgd = c0003000 [00000020] *pgd=80000000004003, *pmd=00000000 Internal error: Oops: 206 [#1] PREEMPT SMP ARM
PC is at __cpufreq_governor+0x10/0x1ac LR is at cpufreq_update_policy+0x114/0x150
---[ end trace f23a8defea6cd706 ]--- Kernel panic - not syncing: Fatal exception CPU0: stopping CPU: 0 PID: 7136 Comm: mpdecision Tainted: G D W 3.10.0-gd727407-00074-g979ede8 #396
[<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) from [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) from [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) from [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) from [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) from [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) from [<c0afe180>] (notifier_call_chain+0x40/0x68) [<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02812dc>] (__cpu_notify+0x28/0x44) [<c02812dc>] (__cpu_notify+0x28/0x44) from [<c0aeed90>] (_cpu_up+0xf4/0x1dc) [<c0aeed90>] (_cpu_up+0xf4/0x1dc) from [<c0aeeed4>] (cpu_up+0x5c/0x78) [<c0aeeed4>] (cpu_up+0x5c/0x78) from [<c0aec808>] (store_online+0x44/0x74) [<c0aec808>] (store_online+0x44/0x74) from [<c03a40f4>] (sysfs_write_file+0x108/0x14c) [<c03a40f4>] (sysfs_write_file+0x108/0x14c) from [<c03517d4>] (vfs_write+0xd0/0x180) [<c03517d4>] (vfs_write+0xd0/0x180) from [<c0351ca8>] (SyS_write+0x38/0x68) [<c0351ca8>] (SyS_write+0x38/0x68) from [<c0205de0>] (ret_fast_syscall+0x0/0x30)
Fix these by taking locks at appropriate places in __cpufreq_add_dev() as well.
Reported-by: Saravana Kannan skannan@codeaurora.org Suggested-by: Srivatsa S. Bhat srivatsa.bhat@linux.vnet.ibm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- V1->V2: No change
drivers/cpufreq/cpufreq.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3c6f9a5..e2a1e67 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1128,6 +1128,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, policy->user_policy.max = policy->max; }
+ down_write(&policy->rwsem); write_lock_irqsave(&cpufreq_driver_lock, flags); for_each_cpu(j, policy->cpus) per_cpu(cpufreq_cpu_data, j) = policy; @@ -1202,6 +1203,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, policy->user_policy.policy = policy->policy; policy->user_policy.governor = policy->governor; } + up_write(&policy->rwsem);
kobject_uevent(&policy->kobj, KOBJ_ADD); up_read(&cpufreq_rwsem);
On Tuesday, March 04, 2014 11:44:01 AM Viresh Kumar wrote:
policy->rwsem is used to lock access to all parts of code modifying struct cpufreq_policy but wasn't used on a new policy created from __cpufreq_add_dev().
Because of which if we call cpufreq_update_policy() repeatedly on one CPU and do offline/online of another CPU then we might see these crashes:
Unable to handle kernel NULL pointer dereference at virtual address 00000020 pgd = c0003000 [00000020] *pgd=80000000004003, *pmd=00000000 Internal error: Oops: 206 [#1] PREEMPT SMP ARM
PC is at __cpufreq_governor+0x10/0x1ac LR is at cpufreq_update_policy+0x114/0x150
---[ end trace f23a8defea6cd706 ]--- Kernel panic - not syncing: Fatal exception CPU0: stopping CPU: 0 PID: 7136 Comm: mpdecision Tainted: G D W 3.10.0-gd727407-00074-g979ede8 #396
[<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) from [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) from [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) from [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) from [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) from [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) from [<c0afe180>] (notifier_call_chain+0x40/0x68) [<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02812dc>] (__cpu_notify+0x28/0x44) [<c02812dc>] (__cpu_notify+0x28/0x44) from [<c0aeed90>] (_cpu_up+0xf4/0x1dc) [<c0aeed90>] (_cpu_up+0xf4/0x1dc) from [<c0aeeed4>] (cpu_up+0x5c/0x78) [<c0aeeed4>] (cpu_up+0x5c/0x78) from [<c0aec808>] (store_online+0x44/0x74) [<c0aec808>] (store_online+0x44/0x74) from [<c03a40f4>] (sysfs_write_file+0x108/0x14c) [<c03a40f4>] (sysfs_write_file+0x108/0x14c) from [<c03517d4>] (vfs_write+0xd0/0x180) [<c03517d4>] (vfs_write+0xd0/0x180) from [<c0351ca8>] (SyS_write+0x38/0x68) [<c0351ca8>] (SyS_write+0x38/0x68) from [<c0205de0>] (ret_fast_syscall+0x0/0x30)
Fix these by taking locks at appropriate places in __cpufreq_add_dev() as well.
Reported-by: Saravana Kannan skannan@codeaurora.org Suggested-by: Srivatsa S. Bhat srivatsa.bhat@linux.vnet.ibm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
I've rebased this one on top of 3.14-rc5 and queued it up for 3.14-rc6.
Please check the bleeding-edge branch for the result.
V1->V2: No change
drivers/cpufreq/cpufreq.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3c6f9a5..e2a1e67 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1128,6 +1128,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, policy->user_policy.max = policy->max; }
- down_write(&policy->rwsem); write_lock_irqsave(&cpufreq_driver_lock, flags); for_each_cpu(j, policy->cpus) per_cpu(cpufreq_cpu_data, j) = policy;
@@ -1202,6 +1203,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, policy->user_policy.policy = policy->policy; policy->user_policy.governor = policy->governor; }
- up_write(&policy->rwsem);
kobject_uevent(&policy->kobj, KOBJ_ADD); up_read(&cpufreq_rwsem);
On Thursday, March 06, 2014 02:04:39 AM Rafael J. Wysocki wrote:
On Tuesday, March 04, 2014 11:44:01 AM Viresh Kumar wrote:
policy->rwsem is used to lock access to all parts of code modifying struct cpufreq_policy but wasn't used on a new policy created from __cpufreq_add_dev().
Because of which if we call cpufreq_update_policy() repeatedly on one CPU and do offline/online of another CPU then we might see these crashes:
Unable to handle kernel NULL pointer dereference at virtual address 00000020 pgd = c0003000 [00000020] *pgd=80000000004003, *pmd=00000000 Internal error: Oops: 206 [#1] PREEMPT SMP ARM
PC is at __cpufreq_governor+0x10/0x1ac LR is at cpufreq_update_policy+0x114/0x150
---[ end trace f23a8defea6cd706 ]--- Kernel panic - not syncing: Fatal exception CPU0: stopping CPU: 0 PID: 7136 Comm: mpdecision Tainted: G D W 3.10.0-gd727407-00074-g979ede8 #396
[<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) from [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) from [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) from [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) from [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) from [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) from [<c0afe180>] (notifier_call_chain+0x40/0x68) [<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02812dc>] (__cpu_notify+0x28/0x44) [<c02812dc>] (__cpu_notify+0x28/0x44) from [<c0aeed90>] (_cpu_up+0xf4/0x1dc) [<c0aeed90>] (_cpu_up+0xf4/0x1dc) from [<c0aeeed4>] (cpu_up+0x5c/0x78) [<c0aeeed4>] (cpu_up+0x5c/0x78) from [<c0aec808>] (store_online+0x44/0x74) [<c0aec808>] (store_online+0x44/0x74) from [<c03a40f4>] (sysfs_write_file+0x108/0x14c) [<c03a40f4>] (sysfs_write_file+0x108/0x14c) from [<c03517d4>] (vfs_write+0xd0/0x180) [<c03517d4>] (vfs_write+0xd0/0x180) from [<c0351ca8>] (SyS_write+0x38/0x68) [<c0351ca8>] (SyS_write+0x38/0x68) from [<c0205de0>] (ret_fast_syscall+0x0/0x30)
Fix these by taking locks at appropriate places in __cpufreq_add_dev() as well.
Reported-by: Saravana Kannan skannan@codeaurora.org Suggested-by: Srivatsa S. Bhat srivatsa.bhat@linux.vnet.ibm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
I've rebased this one on top of 3.14-rc5 and queued it up for 3.14-rc6.
Please check the bleeding-edge branch for the result.
Actually, I think I'll queue up [2-3/3] for 3.14-rc6 instead.
V1->V2: No change
drivers/cpufreq/cpufreq.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3c6f9a5..e2a1e67 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1128,6 +1128,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, policy->user_policy.max = policy->max; }
- down_write(&policy->rwsem); write_lock_irqsave(&cpufreq_driver_lock, flags); for_each_cpu(j, policy->cpus) per_cpu(cpufreq_cpu_data, j) = policy;
@@ -1202,6 +1203,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, policy->user_policy.policy = policy->policy; policy->user_policy.governor = policy->governor; }
- up_write(&policy->rwsem);
kobject_uevent(&policy->kobj, KOBJ_ADD); up_read(&cpufreq_rwsem);
On 03/05/2014 05:06 PM, Rafael J. Wysocki wrote:
On Thursday, March 06, 2014 02:04:39 AM Rafael J. Wysocki wrote:
On Tuesday, March 04, 2014 11:44:01 AM Viresh Kumar wrote:
policy->rwsem is used to lock access to all parts of code modifying struct cpufreq_policy but wasn't used on a new policy created from __cpufreq_add_dev().
Because of which if we call cpufreq_update_policy() repeatedly on one CPU and do offline/online of another CPU then we might see these crashes:
Unable to handle kernel NULL pointer dereference at virtual address 00000020 pgd = c0003000 [00000020] *pgd=80000000004003, *pmd=00000000 Internal error: Oops: 206 [#1] PREEMPT SMP ARM
PC is at __cpufreq_governor+0x10/0x1ac LR is at cpufreq_update_policy+0x114/0x150
---[ end trace f23a8defea6cd706 ]--- Kernel panic - not syncing: Fatal exception CPU0: stopping CPU: 0 PID: 7136 Comm: mpdecision Tainted: G D W 3.10.0-gd727407-00074-g979ede8 #396
[<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) from [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) from [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) from [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) from [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) from [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) from [<c0afe180>] (notifier_call_chain+0x40/0x68) [<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02812dc>] (__cpu_notify+0x28/0x44) [<c02812dc>] (__cpu_notify+0x28/0x44) from [<c0aeed90>] (_cpu_up+0xf4/0x1dc) [<c0aeed90>] (_cpu_up+0xf4/0x1dc) from [<c0aeeed4>] (cpu_up+0x5c/0x78) [<c0aeeed4>] (cpu_up+0x5c/0x78) from [<c0aec808>] (store_online+0x44/0x74) [<c0aec808>] (store_online+0x44/0x74) from [<c03a40f4>] (sysfs_write_file+0x108/0x14c) [<c03a40f4>] (sysfs_write_file+0x108/0x14c) from [<c03517d4>] (vfs_write+0xd0/0x180) [<c03517d4>] (vfs_write+0xd0/0x180) from [<c0351ca8>] (SyS_write+0x38/0x68) [<c0351ca8>] (SyS_write+0x38/0x68) from [<c0205de0>] (ret_fast_syscall+0x0/0x30)
Fix these by taking locks at appropriate places in __cpufreq_add_dev() as well.
Reported-by: Saravana Kannan skannan@codeaurora.org Suggested-by: Srivatsa S. Bhat srivatsa.bhat@linux.vnet.ibm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
I've rebased this one on top of 3.14-rc5 and queued it up for 3.14-rc6.
Please check the bleeding-edge branch for the result.
Actually, I think I'll queue up [2-3/3] for 3.14-rc6 instead.
Pretty close to having this tested and reported back. So, if you can wait, that would be better. Should probably see an email by Fri evening PST.
-Saravana
On Wednesday, March 05, 2014 05:10:01 PM Saravana Kannan wrote:
On 03/05/2014 05:06 PM, Rafael J. Wysocki wrote:
On Thursday, March 06, 2014 02:04:39 AM Rafael J. Wysocki wrote:
On Tuesday, March 04, 2014 11:44:01 AM Viresh Kumar wrote:
policy->rwsem is used to lock access to all parts of code modifying struct cpufreq_policy but wasn't used on a new policy created from __cpufreq_add_dev().
Because of which if we call cpufreq_update_policy() repeatedly on one CPU and do offline/online of another CPU then we might see these crashes:
Unable to handle kernel NULL pointer dereference at virtual address 00000020 pgd = c0003000 [00000020] *pgd=80000000004003, *pmd=00000000 Internal error: Oops: 206 [#1] PREEMPT SMP ARM
PC is at __cpufreq_governor+0x10/0x1ac LR is at cpufreq_update_policy+0x114/0x150
---[ end trace f23a8defea6cd706 ]--- Kernel panic - not syncing: Fatal exception CPU0: stopping CPU: 0 PID: 7136 Comm: mpdecision Tainted: G D W 3.10.0-gd727407-00074-g979ede8 #396
[<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) [<c02a23ac>] (__blocking_notifier_call_chain+0x40/0x58) from [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) [<c02a23d8>] (blocking_notifier_call_chain+0x14/0x1c) from [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) [<c0803c68>] (cpufreq_set_policy+0xd4/0x2b8) from [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) [<c0803e7c>] (cpufreq_init_policy+0x30/0x98) from [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) [<c0805a18>] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) from [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) [<c0805d38>] (cpufreq_cpu_callback+0x58/0x84) from [<c0afe180>] (notifier_call_chain+0x40/0x68) [<c0afe180>] (notifier_call_chain+0x40/0x68) from [<c02812dc>] (__cpu_notify+0x28/0x44) [<c02812dc>] (__cpu_notify+0x28/0x44) from [<c0aeed90>] (_cpu_up+0xf4/0x1dc) [<c0aeed90>] (_cpu_up+0xf4/0x1dc) from [<c0aeeed4>] (cpu_up+0x5c/0x78) [<c0aeeed4>] (cpu_up+0x5c/0x78) from [<c0aec808>] (store_online+0x44/0x74) [<c0aec808>] (store_online+0x44/0x74) from [<c03a40f4>] (sysfs_write_file+0x108/0x14c) [<c03a40f4>] (sysfs_write_file+0x108/0x14c) from [<c03517d4>] (vfs_write+0xd0/0x180) [<c03517d4>] (vfs_write+0xd0/0x180) from [<c0351ca8>] (SyS_write+0x38/0x68) [<c0351ca8>] (SyS_write+0x38/0x68) from [<c0205de0>] (ret_fast_syscall+0x0/0x30)
Fix these by taking locks at appropriate places in __cpufreq_add_dev() as well.
Reported-by: Saravana Kannan skannan@codeaurora.org Suggested-by: Srivatsa S. Bhat srivatsa.bhat@linux.vnet.ibm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
I've rebased this one on top of 3.14-rc5 and queued it up for 3.14-rc6.
Please check the bleeding-edge branch for the result.
Actually, I think I'll queue up [2-3/3] for 3.14-rc6 instead.
Pretty close to having this tested and reported back. So, if you can wait, that would be better. Should probably see an email by Fri evening PST.
OK
It won't hurt if they stay in bleeding-edge/linux-next till then, though.
Thanks!
On 6 March 2014 09:04, Rafael J. Wysocki rjw@rjwysocki.net wrote:
I've rebased this one on top of 3.14-rc5 and queued it up for 3.14-rc6.
Please check the bleeding-edge branch for the result.
Yeah, it looks fine. And I assume that you are planning to take 1/3 in 3.15? Or going to drop it?
On Thursday, March 06, 2014 10:24:38 AM Viresh Kumar wrote:
On 6 March 2014 09:04, Rafael J. Wysocki rjw@rjwysocki.net wrote:
I've rebased this one on top of 3.14-rc5 and queued it up for 3.14-rc6.
Please check the bleeding-edge branch for the result.
Yeah, it looks fine. And I assume that you are planning to take 1/3 in 3.15? Or going to drop it?
I'm going to queue it up for 3.15.
Thanks!
linaro-kernel@lists.linaro.org