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.e...@osadl.org>
Cc: Tejun Heo <t...@kernel.org>
Cc: Rafael J. Wysocki <r...@sisk.pl>
Cc: Andreas Herrmann <andreas.herrma...@amd.com>
Cc: sta...@vger.kernel.org
Signed-off-by: Thomas Gleixner <t...@linutronix.de>
---
drivers/cpufreq/powernow-k8.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
Index: linux-2.6/drivers/cpufreq/powernow-k8.c
===================================================================
--- linux-2.6.orig/drivers/cpufreq/powernow-k8.c
+++ linux-2.6/drivers/cpufreq/powernow-k8.c
@@ -1053,13 +1053,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 */
--
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/
