diff options
author | Eric Dumazet <eric.dumazet@gmail.com> | 2011-01-25 11:31:54 -0500 |
---|---|---|
committer | Ingo Molnar <mingo@elte.hu> | 2011-01-26 02:10:49 -0500 |
commit | 889a7a6a5d5e64063effd40056bdc7b8fb336bd1 (patch) | |
tree | 84f366024832785f6eff971c96129be6e131f6c8 /arch/x86 | |
parent | 9a57c3e487d25f69715705dfeef6eb9e4d666ad7 (diff) |
percpu, x86: Fix percpu_xchg_op()
These recent percpu commits:
2485b6464cf8: x86,percpu: Move out of place 64 bit ops into X86_64 section
8270137a0d50: cpuops: Use cmpxchg for xchg to avoid lock semantics
Caused this 'perf top' crash:
Kernel panic - not syncing: Fatal exception in interrupt
Pid: 0, comm: swapper Tainted: G D
2.6.38-rc2-00181-gef71723 #413 Call Trace: <IRQ> [<ffffffff810465b5>]
? panic
? kmsg_dump
? kmsg_dump
? oops_end
? no_context
? __bad_area_nosemaphore
? perf_output_begin
? bad_area_nosemaphore
? do_page_fault
? __task_pid_nr_ns
? perf_event_tid
? __perf_event_header__init_id
? validate_chain
? perf_output_sample
? trace_hardirqs_off
? page_fault
? irq_work_run
? update_process_times
? tick_sched_timer
? tick_sched_timer
? __run_hrtimer
? hrtimer_interrupt
? account_system_vtime
? smp_apic_timer_interrupt
? apic_timer_interrupt
...
Looking at assembly code, I found:
list = this_cpu_xchg(irq_work_list, NULL);
gives this wrong code : (gcc-4.1.2 cross compiler)
ffffffff810bc45e:
mov %gs:0xead0,%rax
cmpxchg %rax,%gs:0xead0
jne ffffffff810bc45e <irq_work_run+0x3e>
test %rax,%rax
je ffffffff810bc4aa <irq_work_run+0x8a>
Tell gcc we dirty eax/rax register in percpu_xchg_op()
Compiler must use another register to store pxo_new__
We also dont need to reload percpu value after a jump,
since a 'failed' cmpxchg already updated eax/rax
Wrong generated code was :
xor %rax,%rax /* load 0 into %rax */
1: mov %gs:0xead0,%rax
cmpxchg %rax,%gs:0xead0
jne 1b
test %rax,%rax
After patch :
xor %rdx,%rdx /* load 0 into %rdx */
mov %gs:0xead0,%rax
1: cmpxchg %rdx,%gs:0xead0
jne 1b:
test %rax,%rax
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
LKML-Reference: <1295973114.3588.312.camel@edumazet-laptop>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Diffstat (limited to 'arch/x86')
-rw-r--r-- | arch/x86/include/asm/percpu.h | 24 |
1 files changed, 12 insertions, 12 deletions
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h index 3788f4649db4..7e172955ee57 100644 --- a/arch/x86/include/asm/percpu.h +++ b/arch/x86/include/asm/percpu.h | |||
@@ -273,34 +273,34 @@ do { \ | |||
273 | typeof(var) pxo_new__ = (nval); \ | 273 | typeof(var) pxo_new__ = (nval); \ |
274 | switch (sizeof(var)) { \ | 274 | switch (sizeof(var)) { \ |
275 | case 1: \ | 275 | case 1: \ |
276 | asm("\n1:mov "__percpu_arg(1)",%%al" \ | 276 | asm("\n\tmov "__percpu_arg(1)",%%al" \ |
277 | "\n\tcmpxchgb %2, "__percpu_arg(1) \ | 277 | "\n1:\tcmpxchgb %2, "__percpu_arg(1) \ |
278 | "\n\tjnz 1b" \ | 278 | "\n\tjnz 1b" \ |
279 | : "=a" (pxo_ret__), "+m" (var) \ | 279 | : "=&a" (pxo_ret__), "+m" (var) \ |
280 | : "q" (pxo_new__) \ | 280 | : "q" (pxo_new__) \ |
281 | : "memory"); \ | 281 | : "memory"); \ |
282 | break; \ | 282 | break; \ |
283 | case 2: \ | 283 | case 2: \ |
284 | asm("\n1:mov "__percpu_arg(1)",%%ax" \ | 284 | asm("\n\tmov "__percpu_arg(1)",%%ax" \ |
285 | "\n\tcmpxchgw %2, "__percpu_arg(1) \ | 285 | "\n1:\tcmpxchgw %2, "__percpu_arg(1) \ |
286 | "\n\tjnz 1b" \ | 286 | "\n\tjnz 1b" \ |
287 | : "=a" (pxo_ret__), "+m" (var) \ | 287 | : "=&a" (pxo_ret__), "+m" (var) \ |
288 | : "r" (pxo_new__) \ | 288 | : "r" (pxo_new__) \ |
289 | : "memory"); \ | 289 | : "memory"); \ |
290 | break; \ | 290 | break; \ |
291 | case 4: \ | 291 | case 4: \ |
292 | asm("\n1:mov "__percpu_arg(1)",%%eax" \ | 292 | asm("\n\tmov "__percpu_arg(1)",%%eax" \ |
293 | "\n\tcmpxchgl %2, "__percpu_arg(1) \ | 293 | "\n1:\tcmpxchgl %2, "__percpu_arg(1) \ |
294 | "\n\tjnz 1b" \ | 294 | "\n\tjnz 1b" \ |
295 | : "=a" (pxo_ret__), "+m" (var) \ | 295 | : "=&a" (pxo_ret__), "+m" (var) \ |
296 | : "r" (pxo_new__) \ | 296 | : "r" (pxo_new__) \ |
297 | : "memory"); \ | 297 | : "memory"); \ |
298 | break; \ | 298 | break; \ |
299 | case 8: \ | 299 | case 8: \ |
300 | asm("\n1:mov "__percpu_arg(1)",%%rax" \ | 300 | asm("\n\tmov "__percpu_arg(1)",%%rax" \ |
301 | "\n\tcmpxchgq %2, "__percpu_arg(1) \ | 301 | "\n1:\tcmpxchgq %2, "__percpu_arg(1) \ |
302 | "\n\tjnz 1b" \ | 302 | "\n\tjnz 1b" \ |
303 | : "=a" (pxo_ret__), "+m" (var) \ | 303 | : "=&a" (pxo_ret__), "+m" (var) \ |
304 | : "r" (pxo_new__) \ | 304 | : "r" (pxo_new__) \ |
305 | : "memory"); \ | 305 | : "memory"); \ |
306 | break; \ | 306 | break; \ |