| Subject: cpufreq: powernow-k8: Remove bogus smp_processor_id() usage |
| From: Thomas Gleixner <tglx@linutronix.de> |
| Date: Sat, 27 Oct 2012 12:26:44 +0200 |
| |
| commit 6889125b (cpufreq/powernow-k8: workqueue user shouldn't migrate |
| the kworker to another CPU) has a broken optimization of calling |
| powernowk8_target_fn() directly from powernowk8_target() which |
| results in the following splat: |
| |
| [ 11.789468] BUG: using smp_processor_id() in preemptible [00000000] code: |
| modprobe/505 |
| [ 11.809594] caller is powernowk8_target+0x20/0x48 [powernow_k8] |
| [ 12.001748] Pid: 505, comm: modprobe Not tainted 3.6.3 #3 |
| [ 12.016836] Call Trace: |
| [ 12.025971] [<ffffffff81241554>] debug_smp_processor_id+0xcc/0xe8 |
| [ 12.042518] [<ffffffffa05bb07f>] powernowk8_target+0x20/0x48 [powernow_k8] |
| [ 12.060733] [<ffffffff813b3c23>] __cpufreq_driver_target+0x82/0x8a |
| [ 12.077550] [<ffffffff813b64a9>] cpufreq_governor_userspace+0x265/0x2c0 |
| [ 12.120378] [<ffffffff81063c5c>] ? __blocking_notifier_call_chain+0x56/0x60 |
| [ 12.138862] [<ffffffff813b3d8b>] __cpufreq_governor+0x8c/0xc9 |
| [ 12.155193] [<ffffffff813b4031>] __cpufreq_set_policy+0x212/0x21e |
| [ 12.172148] [<ffffffff813b501e>] cpufreq_add_dev_interface+0x2a2/0x2bc |
| [ 12.189855] [<ffffffff813b602b>] ? cpufreq_update_policy+0x124/0x124 |
| [ 12.207096] [<ffffffff813b54dc>] cpufreq_add_dev+0x4a4/0x4b4 |
| [ 12.223161] [<ffffffff812f8136>] subsys_interface_register+0x95/0xc5 |
| [ 12.240386] [<ffffffff8149aaf9>] ? _raw_spin_lock_irqsave+0x24/0x46 |
| [ 12.257477] [<ffffffff813b5928>] cpufreq_register_driver+0xd2/0x1bf |
| [ 12.274545] [<ffffffffa05bc087>] powernowk8_init+0x193/0x1dc [powernow_k8] |
| [ 12.292794] [<ffffffffa05bbef4>] ? powernowk8_cpu_init+0xc53/0xc53 [powernow_k8] |
| [ 12.312004] [<ffffffff81002195>] do_one_initcall+0x7f/0x136 |
| [ 12.327594] [<ffffffff8108f48f>] sys_init_module+0x17b0/0x197e |
| [ 12.343718] [<ffffffff81249494>] ? ddebug_proc_write+0xde/0xde |
| [ 12.359767] [<ffffffff8149f639>] system_call_fastpath+0x16/0x1b |
| |
| This is fully preemptible non cpu bound context though the comment in the |
| code says: |
| |
| * Must run on @pol->cpu. cpufreq core is responsible for ensuring |
| * that we're bound to the current CPU and pol->cpu stays online. |
| |
| The core only guarantees that pol->cpu stays online, but it has no way |
| to bind the thread and this needs to be fully preemptible context as |
| powernowk8_target_fn() calls functions which might sleep. |
| |
| So the correct solution is to always go through work_on_cpu(). |
| |
| Reported-and-tested-by: Carsten Emde <C.Emde@osadl.org> |
| Cc: Tejun Heo <tj@kernel.org> |
| Cc: Rafael J. Wysocki <rjw@sisk.pl> |
| Cc: Andreas Herrmann <andreas.herrmann3@amd.com> |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| --- |
| drivers/cpufreq/powernow-k8.c | 11 +++++------ |
| 1 file changed, 5 insertions(+), 6 deletions(-) |
| |
| Index: linux-stable/drivers/cpufreq/powernow-k8.c |
| =================================================================== |
| --- linux-stable.orig/drivers/cpufreq/powernow-k8.c |
| +++ linux-stable/drivers/cpufreq/powernow-k8.c |
| @@ -1224,13 +1224,12 @@ static int powernowk8_target(struct cpuf |
| .relation = relation }; |
| |
| /* |
| - * Must run on @pol->cpu. cpufreq core is responsible for ensuring |
| - * that we're bound to the current CPU and pol->cpu stays online. |
| + * Must run on @pol->cpu. We queue it on the target cpu even |
| + * if we are currently on the target cpu. This is preemptible |
| + * non cpu bound context, so we can't call the target function |
| + * directly. |
| */ |
| - if (smp_processor_id() == pol->cpu) |
| - return powernowk8_target_fn(&pta); |
| - else |
| - return work_on_cpu(pol->cpu, powernowk8_target_fn, &pta); |
| + return work_on_cpu(pol->cpu, powernowk8_target_fn, &pta); |
| } |
| |
| /* Driver entry point to verify the policy and range of frequencies */ |
