4 -stable kernel review cycles starting

Sorry for the delay.

Here goes:

| (gdb) target remote :1234
| Remote debugging using :1234
| [New Thread 1]
| 0xffffffff81702314 in _spin_lock (lock=0xffffffff81ab9e30)
|     at /scratch/kernel/2.6.32.16/arch/x86/include/asm/spinlock.h:65
| 65      /scratch/kernel/2.6.32.16/arch/x86/include/asm/spinlock.h: No such file or directory.
|         in /scratch/kernel/2.6.32.16/arch/x86/include/asm/spinlock.h
| (gdb) bt
| #0  0xffffffff81702314 in _spin_lock (lock=0xffffffff81ab9e30)
|     at /scratch/kernel/2.6.32.16/arch/x86/include/asm/spinlock.h:65
| #1  0xffffffff8107b214 in vprintk (
|     fmt=0xffffffff818a3ee8 "<1>BUG: unable to handle kernel ", 
|     args=0xffff88001f8f2c38) at kernel/printk.c:705
| #2  0xffffffff816ff387 in printk (fmt=0xffffffff81ab9e30 "")
|     at kernel/printk.c:595
| #3  0xffffffff8105b94a in no_context (regs=0xffff88001f8f2e98, error_code=3, 
|     address=18446744071586262656) at arch/x86/mm/fault.c:583
| #4  0xffffffff8105bb65 in __bad_area_nosemaphore (regs=0xffff88001f8f2e98, 
|     error_code=3, address=18446744071586262656, si_code=196609)
|     at arch/x86/mm/fault.c:741
| #5  0xffffffff8105bc4e in bad_area_nosemaphore (regs=0xffffffff81ab9e30, 
|     error_code=18446612132843695160, address=4294892530)
|     at arch/x86/mm/fault.c:748
| #6  0xffffffff8105c02f in do_page_fault (regs=0xffff88001f8f2e98, error_code=3)
|     at arch/x86/mm/fault.c:1061
| #7  0xffffffff81702765 in page_fault ()
| #8  0x000d3496569e13a8 in ?? ()
| #9  0x00000000b4b2c8e7 in ?? ()
| #10 0x00000003ede5a5a5 in ?? ()
| #11 0x0000000000000001 in per_cpu__irq_stack_union ()
| #12 0xffff880001c11e80 in ?? ()
| ---Type <return> to continue, or q <return> to quit---
| #13 0x0000000000000000 in ?? ()
| (gdb) 
| (gdb) 
| (gdb) 
| (gdb) bt full
| #0  0xffffffff81702314 in _spin_lock (lock=0xffffffff81ab9e30)
|     at /scratch/kernel/2.6.32.16/arch/x86/include/asm/spinlock.h:65
| No locals.
| #1  0xffffffff8107b214 in vprintk (
|     fmt=0xffffffff818a3ee8 "<1>BUG: unable to handle kernel ", 
|     args=0xffff88001f8f2c38) at kernel/printk.c:705
|         printed_len = 0
|         current_log_level = 4
|         flags = 70
|         p = <value optimized out>
| #2  0xffffffff816ff387 in printk (fmt=0xffffffff81ab9e30 "")
|     at kernel/printk.c:595
|         args = {{gp_offset = 8, fp_offset = 48, 
|     overflow_arg_area = 0xffff88001f8f2d18, 
|     reg_save_area = 0xffff88001f8f2c58}}
|         r = 256
| #3  0xffffffff8105b94a in no_context (regs=0xffff88001f8f2e98, error_code=3, 
|     address=18446744071586262656) at arch/x86/mm/fault.c:583
|         tsk = (struct task_struct *) 0xffff88001f908000
|         flags = 146
|         sig = <value optimized out>
| #4  0xffffffff8105bb65 in __bad_area_nosemaphore (regs=0xffff88001f8f2e98, 
|     error_code=3, address=18446744071586262656, si_code=196609)
| ---Type <return> to continue, or q <return> to quit---
|     at arch/x86/mm/fault.c:741
|         tsk = (struct task_struct *) 0xffff88001f908000
| #5  0xffffffff8105bc4e in bad_area_nosemaphore (regs=0xffffffff81ab9e30, 
|     error_code=18446612132843695160, address=4294892530)
|     at arch/x86/mm/fault.c:748
| No locals.
| #6  0xffffffff8105c02f in do_page_fault (regs=0xffff88001f8f2e98, error_code=3)
|     at arch/x86/mm/fault.c:1061
|         vma = <value optimized out>
|         tsk = (struct task_struct *) 0xffff88001f908000
|         address = 18446744071586262656
|         mm = (struct mm_struct *) 0x0
|         write = <value optimized out>
|         fault = 256
| #7  0xffffffff81702765 in page_fault ()
| No symbol table info available.
| #8  0x000d3496569e13a8 in ?? ()
| No symbol table info available.
| #9  0x00000000b4b2c8e7 in ?? ()
| No symbol table info available.
| #10 0x00000003ede5a5a5 in ?? ()
| No symbol table info available.
| #11 0x0000000000000001 in per_cpu__irq_stack_union ()
| ---Type <return> to continue, or q <return> to quit---
| No symbol table info available.
| #12 0xffff880001c11e80 in ?? ()
| No symbol table info available.
| #13 0x0000000000000000 in ?? ()
| No symbol table info available.
| (gdb) quit

So, I disabled PRINTK_TIME and tried again. This time I got:

VFS: Mounted root (ext3 filesystem) readonly on device 253:0. Freeing unused kernel memory: 632k freed
Write protecting the kernel read-only data: 9492k

BUG: unable to handle kernel paging request at ffffffff81712e80
IP: [<ffffffff81054f73>] pvclock_clocksource_read+0x83/0xc0
PGD 1003067 PUD 1007063 PMD 7e8b1063 PTE 8000000001712161
Oops: 0003 [#1] SMP 
last sysfs file: 
CPU 0 
Pid: 1, comm: swapper Not tainted 2.6.32.16-dsa-amd64 #2 
RIP: 0010:[<ffffffff81054f73>]  [<ffffffff81054f73>] pvclock_clocksource_read+0x83/0xc0
RSP: 0018:ffff88007fb47880  EFLAGS: 00010283
RAX: 000d362a57085b85 RBX: ffff880001c11e80 RCX: 000d362a573c6400
RDX: 000d362a57085b85 RSI: 000000000001c410 RDI: ffff880001c11e80
RBP: ffff88007fb478c0 R08: 000000000001a9f2 R09: 6db6db6db6db6db7
R10: ffff88007f15a0a0 R11: ffff88007f15a040 R12: 0000000000000001
R13: 0000001112bb6a47 R14: 00000000b4b2c8e7 R15: 000d362a573c2e34
FS:  0000000000000000(0000) GS:ffff880001c00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: ffffffff81712e80 CR3: 0000000001001000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process swapper (pid: 1, threadinfo ffff88007fb46000, task ffff88007fb50000)
Stack:
 ffff88007fb47900 ffffffff0001a9f2 ffff88007e884150 ffffffff81975c00
<0> 00000000000001b6 ffffffffb3c3c368 ffff88007fb29078 0000000000000000
<0> ffff88007fb478d0 ffffffff8105423c ffff88007fb478e0 ffffffff81054249
Call Trace:
 [<ffffffff8105423c>] kvm_clock_read+0x1c/0x20
 [<ffffffff81054249>] kvm_clock_get_cycles+0x9/0x10
 [<ffffffff81099101>] ktime_get_ts+0x71/0xe0
 [<ffffffff810add4d>] __delayacct_blkio_start+0x1d/0x20
 [<ffffffff8170032a>] io_schedule+0x5a/0xd0
 [<ffffffff81117bdd>] sync_buffer+0x3d/0x50
 [<ffffffff817008ca>] __wait_on_bit+0x5a/0x90
 [<ffffffff81117ba0>] ? sync_buffer+0x0/0x50
 [<ffffffff81117ba0>] ? sync_buffer+0x0/0x50
 [<ffffffff81700974>] out_of_line_wait_on_bit+0x74/0x90
 [<ffffffff81090b30>] ? wake_bit_function+0x0/0x40
 [<ffffffff81117b46>] __wait_on_buffer+0x26/0x30
 [<ffffffff8116331d>] ext3_find_entry+0x19d/0x6b0
 [<ffffffff81060e90>] ? do_flush_tlb_all+0x0/0x70
 [<ffffffff8116418d>] ext3_lookup+0x4d/0x130
 [<ffffffff81105203>] ? d_alloc+0x123/0x1b0
 [<ffffffff810f9fe6>] do_lookup+0x206/0x260
 [<ffffffff810fbe6b>] __link_path_walk+0x19b/0x1000
 [<ffffffff81060eb8>] ? do_flush_tlb_all+0x28/0x70
 [<ffffffff8107fece>] ? on_each_cpu+0x2e/0x50
 [<ffffffff810fcf5e>] path_walk+0x6e/0xe0
 [<ffffffff810fd0eb>] do_path_lookup+0x5b/0x60
 [<ffffffff810fe015>] do_filp_open+0xf5/0xb10
 [<ffffffff810a4026>] ? smp_call_function_many+0x1e6/0x230
 [<ffffffff8105cad0>] ? __cpa_flush_range+0x0/0x30
 [<ffffffff8105caf5>] ? __cpa_flush_range+0x25/0x30
 [<ffffffff8105db73>] ? change_page_attr_set_clr+0x273/0x410
 [<ffffffff81108a5a>] ? alloc_fd+0x4a/0x140
 [<ffffffff810eed7f>] do_sys_open+0x7f/0x140
 [<ffffffff810eee6b>] sys_open+0x1b/0x20
 [<ffffffff810091ef>] init_post+0x3f/0x1b0
 [<ffffffff81a106e5>] kernel_init+0x1a2/0x1ba
 [<ffffffff810373ca>] child_rip+0xa/0x20
 [<ffffffff81a10543>] ? kernel_init+0x0/0x1ba
 [<ffffffff810373c0>] ? child_rip+0x0/0x20
Code: 78 4f 44 89 e1 48 d3 e0 44 89 f2 48 f7 e2 48 0f ac d0 20 4a 8d 0c 38 44 39 03 75 a4 48 8b 15 15 df 6b 00 48 39 ca 77 17 48 89 d0 <f0> 48 0f b1 0d 04 df 6b 00 48 39 c1 48 89 c2 75 e7 48 89 ca 48 
RIP  [<ffffffff81054f73>] pvclock_clocksource_read+0x83/0xc0
 RSP <ffff88007fb47880>
CR2: ffffffff81712e80
---[ end trace 38f25e02ae34d4c1 ]---
BUG: unable to handle kernel paging request at ffffffff81712e80
IP: [<ffffffff81054f73>] pvclock_clocksource_read+0x83/0xc0
PGD 1003067 PUD 1007063 PMD 7e8b1063 PTE 8000000001712161
Oops: 0003 [#2] SMP 
last sysfs file: 
CPU 0 
Pid: 1, comm: swapper Tainted: G      D    2.6.32.16-dsa-amd64 #2 
RIP: 0010:[<ffffffff81054f73>]  [<ffffffff81054f73>] pvclock_clocksource_read+0x83/0xc0
RSP: 0018:ffff880001c03e78  EFLAGS: 00010097
RAX: 000d362a57085b85 RBX: ffff880001c11e80 RCX: 000d362a5b7e8c49
RDX: 000d362a57085b85 RSI: 0000000000000000 RDI: ffff880001c11e80
RBP: ffff880001c03eb8 R08: 000000000001e50c R09: 0000000000000001
R10: ffffffff81ac2360 R11: 0000000000000021 R12: 0000000000000001
R13: 000000111eca7f9f R14: 00000000b4b2c8e7 R15: 000d362a5b7dc64c
FS:  0000000000000000(0000) GS:ffff880001c00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: ffffffff81712e80 CR3: 0000000001001000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process swapper (pid: 1, threadinfo ffff88007fb46000, task ffff88007fb50000)
Stack:
 0000000000000282 000000000001e50c 0000000000000001 ffffffff81975c00
<0> 0000000000000000 0000000023493c1b 00000000000001b6 0000000000000000
<0> ffff880001c03ec8 ffffffff8105423c ffff880001c03ed8 ffffffff81054249
Call Trace:
 <IRQ> 
 [<ffffffff8105423c>] kvm_clock_read+0x1c/0x20
 [<ffffffff81054249>] kvm_clock_get_cycles+0x9/0x10
 [<ffffffff810991da>] ktime_get+0x6a/0xf0
 [<ffffffff81093cd4>] hrtimer_interrupt+0x54/0x1f0
 [<ffffffff810374cc>] ? call_softirq+0x1c/0x30
 [<ffffffff8104e138>] smp_apic_timer_interrupt+0x68/0xa0
 [<ffffffff81036eb3>] apic_timer_interrupt+0x13/0x20
 <EOI> 
 [<ffffffff810a5bb4>] ? acct_collect+0x164/0x1b0
 [<ffffffff810a5af5>] ? acct_collect+0xa5/0x1b0
 [<ffffffff8107e5bc>] ? do_exit+0x6ec/0x7a0
 [<ffffffff8103a943>] ? oops_end+0xa3/0xb0
 [<ffffffff8105b8a8>] ? no_context+0xe8/0x260
 [<ffffffff810c442d>] ? zone_statistics+0x7d/0xa0
 [<ffffffff8105bb65>] ? __bad_area_nosemaphore+0x145/0x220
 [<ffffffff810b2a01>] ? mempool_alloc_slab+0x11/0x20
 [<ffffffff810b2b30>] ? mempool_alloc+0x60/0x130
 [<ffffffff8105bc4e>] ? bad_area_nosemaphore+0xe/0x10
 [<ffffffff8105c02f>] ? do_page_fault+0x1cf/0x280
 [<ffffffff81702765>] ? page_fault+0x25/0x30
 [<ffffffff81054f73>] ? pvclock_clocksource_read+0x83/0xc0
 [<ffffffff8105423c>] ? kvm_clock_read+0x1c/0x20
 [<ffffffff81054249>] ? kvm_clock_get_cycles+0x9/0x10
 [<ffffffff81099101>] ? ktime_get_ts+0x71/0xe0
 [<ffffffff810add4d>] ? __delayacct_blkio_start+0x1d/0x20
 [<ffffffff8170032a>] ? io_schedule+0x5a/0xd0
 [<ffffffff81117bdd>] ? sync_buffer+0x3d/0x50
 [<ffffffff817008ca>] ? __wait_on_bit+0x5a/0x90
 [<ffffffff81117ba0>] ? sync_buffer+0x0/0x50
 [<ffffffff81117ba0>] ? sync_buffer+0x0/0x50
 [<ffffffff81700974>] ? out_of_line_wait_on_bit+0x74/0x90
 [<ffffffff81090b30>] ? wake_bit_function+0x0/0x40
 [<ffffffff81117b46>] ? __wait_on_buffer+0x26/0x30
 [<ffffffff8116331d>] ? ext3_find_entry+0x19d/0x6b0
 [<ffffffff81060e90>] ? do_flush_tlb_all+0x0/0x70
 [<ffffffff8116418d>] ? ext3_lookup+0x4d/0x130
 [<ffffffff81105203>] ? d_alloc+0x123/0x1b0
 [<ffffffff810f9fe6>] ? do_lookup+0x206/0x260
 [<ffffffff810fbe6b>] ? __link_path_walk+0x19b/0x1000
 [<ffffffff81060eb8>] ? do_flush_tlb_all+0x28/0x70
 [<ffffffff8107fece>] ? on_each_cpu+0x2e/0x50
 [<ffffffff810fcf5e>] ? path_walk+0x6e/0xe0
 [<ffffffff810fd0eb>] ? do_path_lookup+0x5b/0x60
 [<ffffffff810fe015>] ? do_filp_open+0xf5/0xb10
 [<ffffffff810a4026>] ? smp_call_function_many+0x1e6/0x230
 [<ffffffff8105cad0>] ? __cpa_flush_range+0x0/0x30
 [<ffffffff8105caf5>] ? __cpa_flush_range+0x25/0x30
 [<ffffffff8105db73>] ? change_page_attr_set_clr+0x273/0x410
 [<ffffffff81108a5a>] ? alloc_fd+0x4a/0x140
 [<ffffffff810eed7f>] ? do_sys_open+0x7f/0x140
 [<ffffffff810eee6b>] ? sys_open+0x1b/0x20
 [<ffffffff810091ef>] ? init_post+0x3f/0x1b0
 [<ffffffff81a106e5>] ? kernel_init+0x1a2/0x1ba
 [<ffffffff810373ca>] ? child_rip+0xa/0x20
 [<ffffffff81a10543>] ? kernel_init+0x0/0x1ba
 [<ffffffff810373c0>] ? child_rip+0x0/0x20
Code: 78 4f 44 89 e1 48 d3 e0 44 89 f2 48 f7 e2 48 0f ac d0 20 4a 8d 0c 38 44 39 03 75 a4 48 8b 15 15 df 6b 00 48 39 ca 77 17 48 89 d0 <f0> 48 0f b1 0d 04 df 6b 00 48 39 c1 48 89 c2 75 e7 48 89 ca 48 
RIP  [<ffffffff81054f73>] pvclock_clocksource_read+0x83/0xc0
 RSP <ffff880001c03e78>
CR2: ffffffff81712e80
---[ end trace 38f25e02ae34d4c2 ]---
Kernel panic - not syncing: Fatal exception in interrupt
Pid: 1, comm: swapper Tainted: G      D    2.6.32.16-dsa-amd64 #2
Call Trace:
 <IRQ>  [<ffffffff816ff24f>] panic+0xa0/0x171
 [<ffffffff81095095>] ? up+0x35/0x50
 [<ffffffff8107ae7d>] ? release_console_sem+0x1cd/0x220
 [<ffffffff8107b155>] ? console_unblank+0x75/0x90
 [<ffffffff8103a93b>] oops_end+0x9b/0xb0
 [<ffffffff8105b8a8>] no_context+0xe8/0x260
 [<ffffffff8105bb65>] __bad_area_nosemaphore+0x145/0x220
 [<ffffffff810b2a22>] ? mempool_free_slab+0x12/0x20
 [<ffffffff810b2aba>] ? mempool_free+0x8a/0xa0
 [<ffffffff813ec9cb>] ? mix_pool_bytes_extract+0x14b/0x160
 [<ffffffff8105bc4e>] bad_area_nosemaphore+0xe/0x10
 [<ffffffff8105c02f>] do_page_fault+0x1cf/0x280
 [<ffffffff81702765>] page_fault+0x25/0x30
 [<ffffffff81054f73>] ? pvclock_clocksource_read+0x83/0xc0
 [<ffffffff81054f3e>] ? pvclock_clocksource_read+0x4e/0xc0
 [<ffffffff8105423c>] kvm_clock_read+0x1c/0x20
 [<ffffffff81054249>] kvm_clock_get_cycles+0x9/0x10
 [<ffffffff810991da>] ktime_get+0x6a/0xf0
 [<ffffffff81093cd4>] hrtimer_interrupt+0x54/0x1f0
 [<ffffffff810374cc>] ? call_softirq+0x1c/0x30
 [<ffffffff8104e138>] smp_apic_timer_interrupt+0x68/0xa0
 [<ffffffff81036eb3>] apic_timer_interrupt+0x13/0x20
 <EOI>  [<ffffffff810a5bb4>] ? acct_collect+0x164/0x1b0
 [<ffffffff810a5af5>] ? acct_collect+0xa5/0x1b0
 [<ffffffff8107e5bc>] ? do_exit+0x6ec/0x7a0
 [<ffffffff8103a943>] ? oops_end+0xa3/0xb0
 [<ffffffff8105b8a8>] ? no_context+0xe8/0x260
 [<ffffffff810c442d>] ? zone_statistics+0x7d/0xa0
 [<ffffffff8105bb65>] ? __bad_area_nosemaphore+0x145/0x220
 [<ffffffff810b2a01>] ? mempool_alloc_slab+0x11/0x20
 [<ffffffff810b2b30>] ? mempool_alloc+0x60/0x130
 [<ffffffff8105bc4e>] ? bad_area_nosemaphore+0xe/0x10
 [<ffffffff8105c02f>] ? do_page_fault+0x1cf/0x280
 [<ffffffff81702765>] ? page_fault+0x25/0x30
 [<ffffffff81054f73>] ? pvclock_clocksource_read+0x83/0xc0
 [<ffffffff8105423c>] ? kvm_clock_read+0x1c/0x20
 [<ffffffff81054249>] ? kvm_clock_get_cycles+0x9/0x10
 [<ffffffff81099101>] ? ktime_get_ts+0x71/0xe0
 [<ffffffff810add4d>] ? __delayacct_blkio_start+0x1d/0x20
 [<ffffffff8170032a>] ? io_schedule+0x5a/0xd0
 [<ffffffff81117bdd>] ? sync_buffer+0x3d/0x50
 [<ffffffff817008ca>] ? __wait_on_bit+0x5a/0x90
 [<ffffffff81117ba0>] ? sync_buffer+0x0/0x50
 [<ffffffff81117ba0>] ? sync_buffer+0x0/0x50
 [<ffffffff81700974>] ? out_of_line_wait_on_bit+0x74/0x90
 [<ffffffff81090b30>] ? wake_bit_function+0x0/0x40
 [<ffffffff81117b46>] ? __wait_on_buffer+0x26/0x30
 [<ffffffff8116331d>] ? ext3_find_entry+0x19d/0x6b0
 [<ffffffff81060e90>] ? do_flush_tlb_all+0x0/0x70
 [<ffffffff8116418d>] ? ext3_lookup+0x4d/0x130
 [<ffffffff81105203>] ? d_alloc+0x123/0x1b0
 [<ffffffff810f9fe6>] ? do_lookup+0x206/0x260
 [<ffffffff810fbe6b>] ? __link_path_walk+0x19b/0x1000
 [<ffffffff81060eb8>] ? do_flush_tlb_all+0x28/0x70
 [<ffffffff8107fece>] ? on_each_cpu+0x2e/0x50
 [<ffffffff810fcf5e>] ? path_walk+0x6e/0xe0
 [<ffffffff810fd0eb>] ? do_path_lookup+0x5b/0x60
 [<ffffffff810fe015>] ? do_filp_open+0xf5/0xb10
 [<ffffffff810a4026>] ? smp_call_function_many+0x1e6/0x230
 [<ffffffff8105cad0>] ? __cpa_flush_range+0x0/0x30
 [<ffffffff8105caf5>] ? __cpa_flush_range+0x25/0x30
 [<ffffffff8105db73>] ? change_page_attr_set_clr+0x273/0x410
 [<ffffffff81108a5a>] ? alloc_fd+0x4a/0x140
 [<ffffffff810eed7f>] ? do_sys_open+0x7f/0x140
 [<ffffffff810eee6b>] ? sys_open+0x1b/0x20
 [<ffffffff810091ef>] ? init_post+0x3f/0x1b0
 [<ffffffff81a106e5>] ? kernel_init+0x1a2/0x1ba
 [<ffffffff810373ca>] ? child_rip+0xa/0x20
 [<ffffffff81a10543>] ? kernel_init+0x0/0x1ba
 [<ffffffff810373c0>] ? child_rip+0x0/0x20

[Also available at http://asteria.noreply.org/~weasel/volatile/2010-07-13-CQL4Lj0xCNo/typescript.zoe.kvmclock.panic ]

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

A good trace is worth it.

Read-only pte

Write fault.

--> lock cmpxchg %rcx,0x6bdf04(%rip)

[...]

So it looks like last_value was placed in a read only section. Please post your System.map somewhere.

error compiling committee.c: too many arguments to function

weasel@intrepid:~$ publish System.map
http://asteria.noreply.org/~weasel/volatile/2010-07-13-mbm2xEdd8Q4/System.map
weasel@intrepid:~$ grep -i last_value System.map
ffffffff81712e80 r last_value
ffffffff81b05240 b last_value.26163

Cheers,
Peter

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

"r" = "read only"

How does it look in 'nm arch/x86/kernel/pvclock.o'?

The same:

[git|v2.6.32.16] weasel@thelma:/scratch/kernel/2.6.32.16$ nm arch/x86/kernel/pvclock.o 0000000000000000 r last_value
U native_read_tsc
0000000000000040 T pvclock_clocksource_read
0000000000000100 T pvclock_read_wallclock
0000000000000000 T pvclock_tsc_khz
U set_normalized_timespec

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

But it's

Doesn't make any sense.

Let's try to see if the toolchain is confused:

- rename last_value to some random name
- drop the 'static' qualifier
- drop the '= ATOMIC64_INIT(0)'
- all of the above

(better start with the last).

No need to rebuild everything, just look at the output of nm pvclock.o.

My bet is that dropping 'static' will fix it. We may have the wrong constraints on atomic64_cmpxchg64(), so the compiler thinks we never change last_value.

error compiling committee.c: too many arguments to function

The constraints are there, but maybe the toolchain is confused.

error compiling committee.c: too many arguments to function

No they aren't, as Linus just pointed out. His patch should fix the problem.

error compiling committee.c: too many arguments to function

Linus's patch touches __xchg() whereas we're using __cmpxchg() in this particular case I think.

At least, applying it to my 2.6.32.16 tree didn't help, last_value was still read-only. Or I backported it wrong.

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

No, you didn't back-port it wrong. I just didn't fix the right part. I thought the PV code used xchg, not cmpxchg, so I only patched that. But cmpxchg has the exact same issue.

Does this fix it?

Again: UNTESTED.

Linus

Btw, this second patch was a bit more aggressive than the first one, and actually removes the "memory" clobber entirely, and the fake cast of the target type.

That shouldn't matter _except_ if people actually use cmpxchg to implement their own locking, since now the compiler could potentially move unrelated memory references around the lock. Of course, if you use cmpxchg to implement your own locking, you're probably doing something wrong anyway (ie you'll get the wrong memory barriers on various architectures), so it should all be fine.

But I thought I'd mention it. And I don't really know how much gcc moves memory accesses around a "asm volatile" - the gcc docs are historically very vague ("volatile asms aren't moved around 'significantly'", whatever 'significant' means)

And btw, none of the above should be taken to mean that I have tested the patch or found it to be otherwise good. It might be totally broken for other reasons. Caveat emptor.

Linus

There are some places which rely on xchg/cmpxchg being a barrier in arch-specific code. For example, the Xen code uses as part of the shared memory protocol with the hypervisor.

"asm volatile"'s only real meaning is that it will not get elided if it appears its output is unused (assuming it is reachable at all). I don't think you can consider it having any meaningful effects on ordering.

J

Actually, I believe volatile operations (including asm volatile) are strictly ordered *with respect to other volatile operations*. As such I would think we'd want to keep the "memory" clobber here, to make it strictly ordered with regards to *all* memory operations.

As for the concept in this patch, it's obviously the right thing, but as Linus said elsewhere it's incomplete.

Conceptually:

Unfortunately I can't write the productization patch right now since I have a plane to catch in about an hour, but if noone beats me to it I'll to it Thursday.

-hpa

The documentation makes no reference to that property; in fact it suggests it is outright not true:

Note that even a volatile `asm' instruction can be moved relative to other
code, including across jump instructions. For example, on many targets there is a system register which can be set to control the rounding mode of floating point operations. You might try setting it with a volatile `asm', like this PowerPC example:

asm volatile("mtfsf 255,%0" : : "f" (fpenv)); sum = x + y;

This will not work reliably, as the compiler may move the addition back before the volatile asm'. To make it work you need to add an artificial dependency to the asm' referencing a variable in the code you don't want moved, for example:

asm volatile ("mtfsf 255,%1" : "=X"(sum): "f"(fpenv)); sum = x + y;

Similarly, you can't expect a sequence of volatile `asm' instructions to remain perfectly consecutive.
[...]
An asm' instruction without any output operands will be treated identically to a volatile asm' instruction.

That would keep its overall effect consistent.

J

The gcc documentation wrt inline asm's is totally worthless. Don't even bother quoting it - because the gcc people themselves have never cared. If the docs ever end up not matching what they want to do, they will just change the documentation.

In other words, at least historically the docs are not in any way meaningful. They are not a "these are the semantics we guarantee", they are just random noise. As I mentioned, the docs historically just said something like "will not be moved significantly", and apparently they've been changed to be something else.

The only thing that has ever been meaningful is "this works". And, of course, that has changed over time too, including actual recommendations on how to make something work (as mentioned, iirc "+" was only valid on register constraints, and "+m" used to not be allowed _or_ recommended, these days it's the only way to do certain things).

It's irritating, because in other circumstances, gcc people take the reverse approach, and consider paper documentation more important than actual implementation issues. So sometimes they say "hey, this is the spec", but when it comes to their own docs, the answer has historically been "we'll just change the spec".

Linus

Sure, I completely agree. At the moment the docs say "asm volatile guarantees nothing", and we can work with that. So long as we don't expect asm volatile to mean anything more (ie, magic semantics involving reordering), everyone is happy.

BTW, gcc 2.95's docs do mention "asm volatile" having an effect on ordering, which is probably where the notion came from: "If you write an asm' instruction with no outputs, GNU CC [...] not delete the instruction or move it outside of loops. [...] you should write the volatile' keyword to prevent future versions of GNU CC from moving the instruction around within a core region". Lucky we never relied on that, right? Right?

J

Except we do.

If gcc ever starts reordering volatile operations, including "asm volatile", the kernel will break, and will be unfixable. Just about every single driver will break. All over the kernel we're explicitly or implicitly making the assumption that volatile operations are strictly ordered by the compiler with respect to each other.

-hpa

Can you give an example? All the cases I've seen rely on the ordering properties of "memory" clobbers, which is sound. (And volatile variables are a completely unrelated issue, of course.)

J

I/O ports, for example.

-hpa

Yes, it looks like they should have memory barriers if we want them to be ordered with respect to normal writes; afaict "asm volatile" has never had strict ordering wrt memory ops.

Anything else?

J

[Adding H.J. to the Cc: list]

Noone has talked about strict ordering between volatiles and (non-volatile) memory ops in general. I have been talking about volatile to volatile ordering, and I thought I'd been very clear about that.

H.J., we're having a debate about the actual semantics of "volatile", especially "asm volatile" in gcc. In particular, I believe that volatile operations should not be possible to reorder with regards to each other, and the kernel depends on that fact.

-hpa

P.S: gcc 4.4 seems to handle "const volatile" incorrectly, probably by applying CSE to those values.

OK.

I think we should consider that deprecated and rely on dependencies and clobbers.

J

I don't think that's practical in general. If the compiler is *that broken*, I don't see how it is usable at all.

As Linus indicated, I don't think we can assume the gcc documentation to accurately reflect the intent of the gcc team, mostly because I think it often lags way behind what they're thinking.

-hpa

Well, over the years, gcc has explicitly changed the docs to weaken the meaning of "asm volatile" from "not being moved ``significantly''" to "no real guarantees about movement at all".

I don't see why its so broken. There are lots of mechanisms to control asm ordering; we don't need "asm volatile" as well. But we do need it to mean "still emit an apparently dead asm with no outputs (or unused outputs)".

They can still be omitted if the whole basic block is dead code, and they can be duplicated by things like inlining and loop unrolling. But presumably they can never be evaluated more times than the source code says they should be, so to that extent they're like volatile variables.

I think the use of the "volatile" keyword here is a red herring. Just because the gcc devs decided to use it as a qualifier for asm statements doesn't mean that one should read anything other than a vague, vague relationship to volatile variables (and extra-vague given how weakly defined *they* are).

I can get not trusting gcc to do what the documentation says it should do, but relying on it to do things that the documentation definitely says it doesn't seems foolhardy.

I'm finding it a bit surreal to be arguing on the side of "don't trust gcc" vs you and Linus on the "gcc developers are arbitrary, capricious and untrustworthy, but we can rely on this piece of undocumented behaviour" side.

J

That makes no sense. According to that logic, "asm volatile" has no semantic meaning at ALL. That's just crazy talk.

The sane compiler semantics for "asm volatile" is that it acts as a volatile memory access. That's what the naming implies, and it has valid semantics that also happen to match the historical semantics. It means that it cannot be removed or duplicated, and it cannot be re-ordered wrt other volatile accesses (whether "asm volatile" or a traditional C volatile memory access).

I agree that we could just add memory clobbers to them all, but my objection to that is that it just makes the whole keyword totally pointless.

Linus

There are some discussions on:

http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02001.html
http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00001.html

Are they related?

H.J.

Not directly as far as I can tell.

The issue is if gcc can ever reorder, duplicate or elide a volatile operation (either asm volatile or a volatile-annotated memory reference.) In my (and Linus') opinion, this would be an incredibly serious bug.

The gcc 4.4 issue, which is separate from the definition issue, is that it seems to assume that it can elide references to "const volatile" objects. "const volatile" should mean a value that could change at any time but which is a bug to write to -- for example a readonly hardware register.

-hpa

Is there a gcc bug for this?

H.J.

Are you asking for a bug report against the documentation? We're not sure what the semantics intended by the gcc team to be, which I guess is a documentation bug.

-hpa

Documentation bug is also a bug :-).

H.J.

The question is "what are the real ordering semantics of asm volatile"? What ordering is enforced between other asm volatiles? What ordering is enforced between asm volatiles and regular memory accesses? asm volatile and other code?

The documentation discusses this to some extent, but mostly says "there are no ordering guarantees". Older versions of gcc - 2.95, for example
- are more explicit, saying that "asm volatiles" won't be moved out of their basic block (I think that's how I parse it, anyway).

Linux relies on "asm volatile" being ordered at least with respect to other asm volatiles. Is this reasonable now? Will gcc break this at some point in the future? If we can't rely on "asm volatile" ordering semantics, what other mechanism can we use (for example, to order clts with respect to FPU-using code)?

Thanks,
J

I looked over the text, and I think you're confusing static reordering and duplication with execution reordering (gcc is indeed free to move around and even replicate asm volatile statements). One thing is that the docs makes it perfectly clear that asm volatile is not ordered with respect to *non*-volatile operations (*all* the examples are volatile-nonvolatile.) It says that asm volatile statements may not be "consecutive", i.e. gcc may generate code in between them, but all of this is well known and understood.

The other thing that the documentation *does* specifically make clear is that an "asm volatile" has an implicit dependency on all memory (as an input, as opposed to an output/clobber.)

I actually found the following statement in the gcc documentation, although it is in the section on C++ (6.1 in my version); the text, though, makes it clear that it applies to both C and C++:

Both the C and C++ standard have the concept of volatile objects. These are normally accessed by pointers and used for accessing hardware. The standards encourage compilers to refrain from optimizations concerning accesses to volatile objects. The C standard leaves it implementation defined as to what constitutes a volatile access. The C++ standard omits to specify this, except to say that C++ should behave in a similar manner to C with respect to volatiles, where possible. The minimum either standard specifies is that at a sequence point all previous accesses to volatile objects have stabilized and no subsequent accesses have occurred. Thus an implementation is free to reorder and

                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

combine volatile accesses which occur between sequence points, but ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot do so for accesses across a sequence point. The use of ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ volatiles does not allow you to violate the restriction on updating objects multiple times within a sequence point.

[My emphasis, obviously.]

I think that pretty much settles the matter, since any statement (including an asm statement) inherently has a sequence point before or after it. However, I will file a gcc bug report to clarify.

-hpa

For those that are interested:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44943

-hpa

1. set up a mapping
2. invlpg or set cr3
3. use the mapping

Moving the invlpg will break your code.

I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

invlpg uses memory clobbers. All the crX ops seem to use a __force_order variable to sequence them - but it looks like it's done precisely backwards and it's barking mad to do allow write_crX to be reordered with respect to memory ops.

Hm, looks like glommer added it surreptitiously while unifying system_32.h and system_64.h (system_32.h relied on asm volatile not being reordered; system_64.h used memory clobbers).
J

clts() has no memory clobber; it is used to serialize execution of code within kernel_fpu_begin() / kernel_fpu_end() blocks.

If the code within is reordered before the clts(), we've corrupted guest FPU state.

That's the kind of bug I think Linus is talking about. We've been expecting volatile to work that way for over a decade, by my recollection, and if it doesn't, there is going to be a lot of broken code.

Shouldn't we at least get a compiler switch to force the volatile behavior? I'd suggest it default to conservative.

Hmm, well, despite that not being quite correct (if guest has used FPU, we save it, which has a memory clobber), it seems to be the case that a reordering of the clts() among the other volatile asm statements would be a very bad thing - you'd get kernel FPU exceptions.

And among asm volatiles, clts() is fairly unique in not having any clobbers or dependencies at all, so it could happen.

Zach

Hm, that's awkward - you'd really need some way of specifying ordering with respect to the FPU state, and "memory" would only be an approximate proxy of that. And not a good one, since gcc wouldn't regard register-only fpu ops as being affected by a memory clobber.

J

At this point, it looks like there is no reason to be alarmed. The documentation actually contains a statement about volatiles not being mutually reordered across sequence points, and since asm is a statement (rather than an expression) it is always surrounded by sequence points.
I have filed a gcc ticket to ask for clarification.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

invlpg, in the general case, definitely needs a memory clobber even if volatiles are ordered, since it needs to be ordered with regards to non-volatile memory operations.

Note that memory clobbers don't by themselves enforce ordering since they don't prevent the ordering of memory clobbers with respect to each other.

-hpa

Yes. I'd say write_crX should need that too, since they can they can have a variety global effects on how memory addressing works (from tlb flush to switching pagetable formats).

Hm. Well we do *definitely* rely on that. I guess technically we'd need at least a memory input on the asm to be ordered with respect to a memory clobber. But that doesn't seem to have been an issue (or perhaps its mixed up with the secret semantics of asm volatile).

J

It appears to, thanks a lot.

[git|v2.6.32.16] weasel@thelma:/scratch/kernel/2.6.32.16$ nm arch/x86/kernel/pvclock.o 0000000000000000 b last_value
..

And it did boot too.

Without the cast gcc spews a fair amount of warnings. About four every time it hits the include file.

Just for completeness' sake I attached the patch on top of 2.6.32.16 that I built with.

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

index 52de72e..f0551c5 100644
@@ -26,26 +26,26 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
 	switch (size) {
 	case 1:
 		asm volatile("xchgb %b0,%1"
-			     : "=q" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=q" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	case 2:
 		asm volatile("xchgw %w0,%1"
-			     : "=r" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=r" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	case 4:
 		asm volatile("xchgl %k0,%1"
-			     : "=r" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=r" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	case 8:
 		asm volatile("xchgq %0,%1"
-			     : "=r" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=r" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	}

@@ -66,28 +66,24 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
 	unsigned long prev;
 	switch (size) {
 	case 1:
-		asm volatile(LOCK_PREFIX "cmpxchgb %b1,%2"
-			     : "=a"(prev)
-			     : "q"(new), "m"(*__xg(ptr)), "0"(old)
-			     : "memory");
+		asm volatile(LOCK_PREFIX "cmpxchgb %b2,%1"
+			     : "=a"(prev), "+m" (*__xg(ptr))
+			     : "q"(new), "0"(old));
 		return prev;
 	case 2:
-		asm volatile(LOCK_PREFIX "cmpxchgw %w1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
-			     : "memory");
+		asm volatile(LOCK_PREFIX "cmpxchgw %w2,%1"
+			     : "=a"(prev), "+m" (*__xg(ptr))
+			     : "r"(new), "0"(old));
 		return prev;
 	case 4:
-		asm volatile(LOCK_PREFIX "cmpxchgl %k1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
-			     : "memory");
+		asm volatile(LOCK_PREFIX "cmpxchgl %k2,%1"
+			     : "=a"(prev), "+m" (*__xg(ptr))
+			     : "r"(new), "0"(old));
 		return prev;
 	case 8:
-		asm volatile(LOCK_PREFIX "cmpxchgq %1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
-			     : "memory");
+		asm volatile(LOCK_PREFIX "cmpxchgq %2,%1"
+			     : "=a"(prev), "+m" (*__xg(ptr))
+			     : "r"(new), "0"(old));
 		return prev;
 	}
 	return old;

0000000000000000 r some_random_name

0000000000000000 B last_value

0000000000000000 b last_value

0000000000000000 B some_random_name

make -f scripts/Makefile.build obj=arch/x86/kernel arch/x86/kernel/pvclock.o
gcc -Wp,-MD,arch/x86/kernel/.pvclock.o.d -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.3.2/include -Iinclude -I/scratch/kernel/2.6.32.16/arch/x86/include -include include/linux/autoconf.h -D__KERNEL__ -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -fno-delete-null-pointer-checks -O2 -m64 -mtune=generic -mno-red-zone -mcmodel=kernel -funit-at-a-time -maccumulate-outgoing-args -fstack-protector -DCONFIG_AS_CFI=1 -DCONFIG_AS_CFI_SIGNAL_FRAME=1 -pipe -Wno-sign-compare -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -fno-omit-frame-pointer -fno-optimize-sibling-calls -g -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(pvclock)" -D"KBUILD_MODNAME=KBUILD_STR(pvclock)" -c -o arch/x86/kernel/pvclock.o arch/x86/kernel/pvclock.c

[git|v2.6.32.16] weasel@thelma:/scratch/kernel/2.6.32.16$ gcc --version gcc (Debian 4.3.2-1.1) 4.3.2
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Using -O0 also results in a writeable last_value (b).

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

I bet it is the same. And I have a suspicion: because the only write access to that variable is in an asm that uses the "memory" clobber to say it wrote to it (rather than say it writes to it directly), and because the variable is marked 'static', gcc decides that nothing ever writes to it in that compilation unit, and it can be made read-only.

Look at our definition for "xchg()" in arch/x86/include/asm/cmpxchg_64.h. It boils down to

                asm volatile("xchgq %0,%1"                              \
                             : "=r" (__x)                               \
                             : "m" (*__xg(ptr)), "0" (__x)              \
                             : "memory");                               \

for the 8-byte case (which is obviously what atomic64_xchg() uses). And the _reason_ we do that thing where we use a memory _input_ and then a clobber is that older versions of gcc did not accept the thing we _want_ to use, namely using "+m" to say that we actually change the memory. So the above is "wrong", but has historical reasons - and it's apparently never been changed.

However, the "+m" was fixed, and we use it elsewhere, so I think the "m" plus memory clobber is now purely historical. Does a patch something like the appended fix it? I also suspect we should look at some other uses in this area. The atomic64_64.h file uses "=m" and "m", which looks like another legacy thing (again, "+m" historically wasn't allowed, and then later became the 'correct' way to do things).

NOTE NOTE NOTE! This is UNTESTED and INCOMPLETE. We should do cmpxchg too, and the 32-bit versions. I'm adding Ingo and Peter to the cc.

Linus

Well, current upstream uses "m":

and works; I also failed to reproduce with 2.6.32.16. So I expect some toolchain involvement.

Peter, what gcc are you using?

I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Yes, I'd expect current 2.6.34 to also show the same problem.

And yes, I do believe it's likely a newer gcc that triggers it too, otherwise we'd have seen more reports of it.

Linus

Did anyone get around to doing a proper fix for this? I don't see one in tip.git or linux-2.6.git. And it needs to get back into stable...

Thanks,
J

I'll go ahead and write it up.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Okay, I've stared at this issue for most of the day, and I have a fix for the immediate issue.

However, I also am convinced that the whole scheme with the __xg() macro is just broken. It was introduced in 1.3.11 when xchg and cmpxchg where neither "asm volatile" nor had "memory" clobbers as an apparent way to keep gcc from moving memory references around it, but since the array type is "unsigned long" rather than a char type, it probably doesn't even work right, and with asm volatile/memory clobber it should not be necessary.

At this point I'd like to push an urgent patch to fix the immediate issue, and a cleanup patch getting rid of __xg() for .36.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Commit-ID:  113fc5a6e8c2288619ff7e8187a6f556b7e0d372
Gitweb:     http://git.kernel.org/tip/113fc5a6e8c2288619ff7e8187a6f556b7e0d372
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Tue, 27 Jul 2010 17:01:49 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Tue, 27 Jul 2010 17:14:02 -0700

x86: Add memory modify constraints to xchg() and cmpxchg()

xchg() and cmpxchg() modify their memory operands, not merely read them. For some versions of gcc the "memory" clobber has apparently dealt with the situation, but not for all.

LKML-Reference: <[email protected]>

---
 arch/x86/include/asm/cmpxchg_32.h |   68 ++++++++++++++++++------------------
 arch/x86/include/asm/cmpxchg_64.h |   40 +++++++++++-----------
 2 files changed, 54 insertions(+), 54 deletions(-)

index 8859e12..c1cf59d 100644
@@ -27,20 +27,20 @@ struct __xchg_dummy {
 	switch (size) {							\
 	case 1:								\
 		asm volatile("xchgb %b0,%1"				\
-			     : "=q" (__x)				\
-			     : "m" (*__xg(ptr)), "0" (__x)		\
+			     : "=q" (__x), "+m" (*__xg(ptr))		\
+			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
 	case 2:								\
 		asm volatile("xchgw %w0,%1"				\
-			     : "=r" (__x)				\
-			     : "m" (*__xg(ptr)), "0" (__x)		\
+			     : "=r" (__x), "+m" (*__xg(ptr))		\
+			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
 	case 4:								\
 		asm volatile("xchgl %0,%1"				\
-			     : "=r" (__x)				\
-			     : "m" (*__xg(ptr)), "0" (__x)		\
+			     : "=r" (__x), "+m" (*__xg(ptr))		\
+			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
 	default:							\

@@ -70,14 +70,14 @@ static inline void __set_64bit(unsigned long long *ptr,
 			       unsigned int low, unsigned int high)
 {
 	asm volatile("\n1:\t"
-		     "movl (%0), %%eax\n\t"
-		     "movl 4(%0), %%edx\n\t"
-		     LOCK_PREFIX "cmpxchg8b (%0)\n\t"
+		     "movl (%1), %%eax\n\t"
+		     "movl 4(%1), %%edx\n\t"
+		     LOCK_PREFIX "cmpxchg8b (%1)\n\t"
 		     "jnz 1b"
-		     : /* no outputs */
-		     : "D"(ptr),
-		       "b"(low),
-		       "c"(high)
+		     : "=m" (*ptr)
+		     : "D" (ptr),
+		       "b" (low),
+		       "c" (high)
 		     : "ax", "dx", "memory");
 }
 

@@ -121,21 +121,21 @@ extern void __cmpxchg_wrong_size(void);
 	__typeof__(*(ptr)) __new = (new);				\
 	switch (size) {							\
 	case 1:								\
-		asm volatile(lock "cmpxchgb %b1,%2"			\
-			     : "=a"(__ret)				\
-			     : "q"(__new), "m"(*__xg(ptr)), "0"(__old)	\
+		asm volatile(lock "cmpxchgb %b2,%1"			\
+			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "q" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	case 2:								\
-		asm volatile(lock "cmpxchgw %w1,%2"			\
-			     : "=a"(__ret)				\
-			     : "r"(__new), "m"(*__xg(ptr)), "0"(__old)	\
+		asm volatile(lock "cmpxchgw %w2,%1"			\
+			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	case 4:								\
-		asm volatile(lock "cmpxchgl %1,%2"			\
-			     : "=a"(__ret)				\
-			     : "r"(__new), "m"(*__xg(ptr)), "0"(__old)	\
+		asm volatile(lock "cmpxchgl %2,%1"			\
+			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	default:							\

@@ -180,12 +180,12 @@ static inline unsigned long long __cmpxchg64(volatile void *ptr,
 					     unsigned long long new)
 {
 	unsigned long long prev;
-	asm volatile(LOCK_PREFIX "cmpxchg8b %3"
-		     : "=A"(prev)
-		     : "b"((unsigned long)new),
-		       "c"((unsigned long)(new >> 32)),
-		       "m"(*__xg(ptr)),
-		       "0"(old)
+	asm volatile(LOCK_PREFIX "cmpxchg8b %1"
+		     : "=A" (prev),
+		       "+m" (*__xg(ptr))
+		     : "b" ((unsigned long)new),
+		       "c" ((unsigned long)(new >> 32)),
+		       "0" (old)
 		     : "memory");
 	return prev;
 }

@@ -195,12 +195,12 @@ static inline unsigned long long __cmpxchg64_local(volatile void *ptr,
 						   unsigned long long new)
 {
 	unsigned long long prev;
-	asm volatile("cmpxchg8b %3"
-		     : "=A"(prev)
-		     : "b"((unsigned long)new),
-		       "c"((unsigned long)(new >> 32)),
-		       "m"(*__xg(ptr)),
-		       "0"(old)
+	asm volatile("cmpxchg8b %1"
+		     : "=A" (prev),
+		       "+m" (*__xg(ptr))
+		     : "b" ((unsigned long)new),
+		       "c" ((unsigned long)(new >> 32)),
+		       "0" (old)
 		     : "memory");
 	return prev;
 }

index 485ae41..b92f147 100644
@@ -26,26 +26,26 @@ extern void __cmpxchg_wrong_size(void);
 	switch (size) {							\
 	case 1:								\
 		asm volatile("xchgb %b0,%1"				\
-			     : "=q" (__x)				\
-			     : "m" (*__xg(ptr)), "0" (__x)		\
+			     : "=q" (__x), "+m" (*__xg(ptr))		\
+			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
 	case 2:								\
 		asm volatile("xchgw %w0,%1"				\
-			     : "=r" (__x)				\
-			     : "m" (*__xg(ptr)), "0" (__x)		\
+			     : "=r" (__x), "+m" (*__xg(ptr))		\
+			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
 	case 4:								\
 		asm volatile("xchgl %k0,%1"				\
-			     : "=r" (__x)				\
-			     : "m" (*__xg(ptr)), "0" (__x)		\
+			     : "=r" (__x), "+m" (*__xg(ptr))		\
+			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
 	case 8:								\
 		asm volatile("xchgq %0,%1"				\
-			     : "=r" (__x)				\
-			     : "m" (*__xg(ptr)), "0" (__x)		\
+			     : "=r" (__x), "+m" (*__xg(ptr))		\
+			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
 	default:							\

@@ -71,27 +71,27 @@ extern void __cmpxchg_wrong_size(void);
 	__typeof__(*(ptr)) __new = (new);				\
 	switch (size) {							\
 	case 1:								\
-		asm volatile(lock "cmpxchgb %b1,%2"			\
-			     : "=a"(__ret)				\
-			     : "q"(__new), "m"(*__xg(ptr)), "0"(__old)	\
+		asm volatile(lock "cmpxchgb %b2,%1"			\
+			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "q" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	case 2:								\
-		asm volatile(lock "cmpxchgw %w1,%2"			\
-			     : "=a"(__ret)				\
-			     : "r"(__new), "m"(*__xg(ptr)), "0"(__old)	\
+		asm volatile(lock "cmpxchgw %w2,%1"			\
+			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	case 4:								\
-		asm volatile(lock "cmpxchgl %k1,%2"			\
-			     : "=a"(__ret)				\
-			     : "r"(__new), "m"(*__xg(ptr)), "0"(__old)	\
+		asm volatile(lock "cmpxchgl %k2,%1"			\
+			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	case 8:								\
-		asm volatile(lock "cmpxchgq %1,%2"			\
-			     : "=a"(__ret)				\
-			     : "r"(__new), "m"(*__xg(ptr)), "0"(__old)	\
+		asm volatile(lock "cmpxchgq %2,%1"			\
+			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	default:							\

Ack. I assume this doesn't really change the code generated? At least not with a gcc that honors the whole memory clobber thing properly?

I also suspect that we can/should get rid of the __xg() thing - it was there just to make sure gcc didn't see the memory read as a single word and tried to optimize it. With the "+m" it probably doesn't matter any more (don't know if it ever did)

Linus

I didn't do a binary A:B comparison, on the assumption it would preturb the code too much to be equal, but I guess it would be a good idea to at least try.

"+m" would still see as a single word, but the __xg() thing should be obsoleted by asm volatile + "memory", neither of which were in the 1.3.11 version.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

For what it's worth, it fairly heavily preturbs code around __set_64bit(), which implies it actually does something useful in that case. The rest of the code looks similar enough.

-hpa

Commit-ID:  4532b305e8f0c238dd73048068ff8a6dd1380291
Gitweb:     http://git.kernel.org/tip/4532b305e8f0c238dd73048068ff8a6dd1380291
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Wed, 28 Jul 2010 15:18:35 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Wed, 28 Jul 2010 15:24:09 -0700

x86, asm: Clean up and simplify <asm/cmpxchg.h>

Remove the __xg() hack to create a memory barrier near xchg and cmpxchg; it has been there since 1.3.11 but should not be necessary with "asm volatile" and a "memory" clobber, neither of which were there in the original implementation.

However, we *should* make this a volatile reference.

LKML-Reference: <[email protected]>

---
 arch/x86/include/asm/cmpxchg_32.h |   75 ++++++++++++++++++++----------------
 arch/x86/include/asm/cmpxchg_64.h |   61 ++++++++++++++++++++---------
 2 files changed, 84 insertions(+), 52 deletions(-)

index 20955ea..f5bd1fd 100644
@@ -11,38 +11,42 @@
 extern void __xchg_wrong_size(void);
 
 /*
- * Note: no "lock" prefix even on SMP: xchg always implies lock anyway
- * Note 2: xchg has side effect, so that attribute volatile is necessary,
- *	  but generally the primitive is invalid, *ptr is output argument. --ANK
+ * Note: no "lock" prefix even on SMP: xchg always implies lock anyway.
+ * Since this is generally used to protect other memory information, we
+ * use "asm volatile" and "memory" clobbers to prevent gcc from moving
+ * information around.
  */
-
-struct __xchg_dummy {
-	unsigned long a[100];
-};
-#define __xg(x) ((struct __xchg_dummy *)(x))
-
 #define __xchg(x, ptr, size)						\
 ({									\
 	__typeof(*(ptr)) __x = (x);					\
 	switch (size) {							\
 	case 1:								\
-		asm volatile("xchgb %b0,%1"				\
-			     : "=q" (__x), "+m" (*__xg(ptr))		\
+	{								\
+		volatile u8 *__ptr = (volatile u8 *)(ptr);		\
+		asm volatile("xchgb %0,%1"				\
+			     : "=q" (__x), "+m" (*__ptr)		\
 			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
+	}								\
 	case 2:								\
-		asm volatile("xchgw %w0,%1"				\
-			     : "=r" (__x), "+m" (*__xg(ptr))		\
+	{								\
+		volatile u16 *__ptr = (volatile u16 *)(ptr);		\
+		asm volatile("xchgw %0,%1"				\
+			     : "=r" (__x), "+m" (*__ptr)		\
 			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
+	}								\
 	case 4:								\
+	{								\
+		volatile u32 *__ptr = (volatile u32 *)(ptr);		\
 		asm volatile("xchgl %0,%1"				\
-			     : "=r" (__x), "+m" (*__xg(ptr))		\
+			     : "=r" (__x), "+m" (*__ptr)		\
 			     : "0" (__x)				\
 			     : "memory");				\
 		break;							\
+	}								\
 	default:							\
 		__xchg_wrong_size();					\
 	}								\

@@ -94,23 +98,32 @@ extern void __cmpxchg_wrong_size(void);
 	__typeof__(*(ptr)) __new = (new);				\
 	switch (size) {							\
 	case 1:								\
-		asm volatile(lock "cmpxchgb %b2,%1"			\
-			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+	{								\
+		volatile u8 *__ptr = (volatile u8 *)(ptr);		\
+		asm volatile(lock "cmpxchgb %2,%1"			\
+			     : "=a" (__ret), "+m" (*__ptr)		\
 			     : "q" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
+	}								\
 	case 2:								\
-		asm volatile(lock "cmpxchgw %w2,%1"			\
-			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+	{								\
+		volatile u16 *__ptr = (volatile u16 *)(ptr);		\
+		asm volatile(lock "cmpxchgw %2,%1"			\
+			     : "=a" (__ret), "+m" (*__ptr)		\
 			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
+	}								\
 	case 4:								\
+	{								\
+		volatile u32 *__ptr = (volatile u32 *)(ptr);		\
 		asm volatile(lock "cmpxchgl %2,%1"			\
-			     : "=a" (__ret), "+m" (*__xg(ptr))		\
+			     : "=a" (__ret), "+m" (*__ptr)		\
 			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
+	}								\
 	default:							\
 		__cmpxchg_wrong_size();					\
 	}								\

@@ -148,31 +161,27 @@ extern void __cmpxchg_wrong_size(void);
 					       (unsigned long long)(n)))
 #endif
 
-static inline unsigned long long __cmpxchg64(volatile void *ptr,
-					     unsigned long long old,
-					     unsigned long long new)
+static inline u64 __cmpxchg64(volatile u64 *ptr, u64 old, u64 new)
 {
-	unsigned long long prev;
+	u64 prev;
 	asm volatile(LOCK_PREFIX "cmpxchg8b %1"
 		     : "=A" (prev),
-		       "+m" (*__xg(ptr))
-		     : "b" ((unsigned long)new),
-		       "c" ((unsigned long)(new >> 32)),
+		       "+m" (*ptr)
+		     : "b" ((u32)new),
+		       "c" ((u32)(new >> 32)),
 		       "0" (old)
 		     : "memory");
 	return prev;
 }
 
-static inline unsigned long long __cmpxchg64_local(volatile void *ptr,
-						   unsigned long long old,
-						   unsigned long long new)
+static inline u64 __cmpxchg64_local(volatile u64 *ptr, u64 old, u64 new)
 {
-	unsigned long long prev;
+	u64 prev;
 	asm volatile("cmpxchg8b %1"
 		     : "=A" (prev),
-		       "+m" (*__xg(ptr))
-		     : "b" ((unsigned long)new),
-		       "c" ((unsigned long)(new >> 32)),
+		       "+m" (*ptr)
+		     : "b" ((u32)new),
+		       "c" ((u32)(new >> 32)),
 		       "0" (old)
 		     : "memory");
 	return prev;

Commit-ID:  90c8f92f5c807807ca74d5f2f313794925174e6b
Gitweb:     http://git.kernel.org/tip/90c8f92f5c807807ca74d5f2f313794925174e6b
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Wed, 28 Jul 2010 16:53:49 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Wed, 28 Jul 2010 16:53:49 -0700

x86, asm: Move cmpxchg emulation code to arch/x86/lib

Move cmpxchg emulation code from arch/x86/kernel/cpu (which is otherwise CPU identification) to arch/x86/lib, where other emulation code lives already.

LKML-Reference: <[email protected]>

---
 arch/x86/kernel/cpu/Makefile           |    2 +-
 arch/x86/lib/Makefile                  |    1 +
 arch/x86/{kernel/cpu => lib}/cmpxchg.c |    0
 3 files changed, 2 insertions(+), 1 deletions(-)

index 3a785da..c47c439 100644
@@ -16,7 +16,7 @@ obj-y			:= intel_cacheinfo.o addon_cpuid_features.o
 obj-y			+= proc.o capflags.o powerflags.o common.o
 obj-y			+= vmware.o hypervisor.o sched.o mshyperv.o
 
-obj-$(CONFIG_X86_32)	+= bugs.o cmpxchg.o
+obj-$(CONFIG_X86_32)	+= bugs.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
 
 obj-$(CONFIG_CPU_SUP_INTEL)		+= intel.o

index f871e04..e10cf07 100644
@@ -30,6 +30,7 @@ ifeq ($(CONFIG_X86_32),y)
         lib-y += checksum_32.o
         lib-y += strstr_32.o
         lib-y += semaphore_32.o string_32.o
+        lib-y += cmpxchg.o
 ifneq ($(CONFIG_X86_CMPXCHG64),y)
         lib-y += cmpxchg8b_emu.o atomic64_386_32.o
 endif

similarity index 100%
rename from arch/x86/kernel/cpu/cmpxchg.c
rename to arch/x86/lib/cmpxchg.c

Commit-ID:  a378d9338e8dde78314b3a6ae003de351936c729
Gitweb:     http://git.kernel.org/tip/a378d9338e8dde78314b3a6ae003de351936c729
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Wed, 28 Jul 2010 17:05:11 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Wed, 28 Jul 2010 17:05:11 -0700

x86, asm: Merge cmpxchg_486_u64() and cmpxchg8b_emu()

We have two functions for doing exactly the same thing -- emulating cmpxchg8b on 486 and older hardware -- with different calling conventions, and yet doing the same thing. Drop the C version and use the assembly version, via alternatives, for both the local and non-local versions of cmpxchg8b.

LKML-Reference: <[email protected]>

---
 arch/x86/include/asm/cmpxchg_32.h |   30 ++++++++++++++----------------
 arch/x86/lib/cmpxchg.c            |   18 ------------------
 2 files changed, 14 insertions(+), 34 deletions(-)

index f5bd1fd..284a6e8 100644
@@ -246,8 +246,6 @@ static inline unsigned long cmpxchg_386(volatile void *ptr, unsigned long old,
  * to simulate the cmpxchg8b on the 80386 and 80486 CPU.
  */
 
-extern unsigned long long cmpxchg_486_u64(volatile void *, u64, u64);
-
 #define cmpxchg64(ptr, o, n)					\
 ({								\
 	__typeof__(*(ptr)) __ret;				\

@@ -265,20 +263,20 @@ extern unsigned long long cmpxchg_486_u64(volatile void *, u64, u64);
 	__ret; })
 
 
-
-#define cmpxchg64_local(ptr, o, n)					\
-({									\
-	__typeof__(*(ptr)) __ret;					\
-	if (likely(boot_cpu_data.x86 > 4))				\
-		__ret = (__typeof__(*(ptr)))__cmpxchg64_local((ptr),	\
-				(unsigned long long)(o),		\
-				(unsigned long long)(n));		\
-	else								\
-		__ret = (__typeof__(*(ptr)))cmpxchg_486_u64((ptr),	\
-				(unsigned long long)(o),		\
-				(unsigned long long)(n));		\
-	__ret;								\
-})
+#define cmpxchg64_local(ptr, o, n)				\
+({								\
+	__typeof__(*(ptr)) __ret;				\
+	__typeof__(*(ptr)) __old = (o);				\
+	__typeof__(*(ptr)) __new = (n);				\
+	alternative_io("call cmpxchg8b_emu",			\
+		       "cmpxchg8b (%%esi)" ,			\
+		       X86_FEATURE_CX8,				\
+		       "=A" (__ret),				\
+		       "S" ((ptr)), "0" (__old),		\
+		       "b" ((unsigned int)__new),		\
+		       "c" ((unsigned int)(__new>>32))		\
+		       : "memory");				\
+	__ret; })
 
 #endif
 

index 2056ccf..5d619f6 100644
@@ -52,21 +52,3 @@ unsigned long cmpxchg_386_u32(volatile void *ptr, u32 old, u32 new)
 }
 EXPORT_SYMBOL(cmpxchg_386_u32);
 #endif
-
-#ifndef CONFIG_X86_CMPXCHG64
-unsigned long long cmpxchg_486_u64(volatile void *ptr, u64 old, u64 new)
-{
-	u64 prev;
-	unsigned long flags;
-
-	/* Poor man's cmpxchg8b for 386 and 486. Unsuitable for SMP */
-	local_irq_save(flags);
-	prev = *(u64 *)ptr;
-	if (prev == old)
-		*(u64 *)ptr = new;
-	local_irq_restore(flags);
-	return prev;
-}
-EXPORT_SYMBOL(cmpxchg_486_u64);
-#endif
-

Here's the 2.6.32 version for stable.

J

From: Jeremy Fitzhardinge<[email protected]>
Date: Tue, 27 Jul 2010 23:03:58 -0700
Subject: [PATCH] x86: Add memory modify constraints to xchg() and cmpxchg()

xchg() and cmpxchg() modify their memory operands, not merely read them. For some versions of gcc the "memory" clobber has apparently dealt with the situation, but not for all.

Also adds the missing 8-byte case for __sync_cmpxchg().

Based on HPA's patch.

index ee1931b..5af5051 100644
@@ -34,12 +34,12 @@ static inline void __set_64bit(unsigned long long *ptr,
 			       unsigned int low, unsigned int high)
 {
 	asm volatile("\n1:\t"
-		     "movl (%0), %%eax\n\t"
-		     "movl 4(%0), %%edx\n\t"
-		     LOCK_PREFIX "cmpxchg8b (%0)\n\t"
+		     "movl (%1), %%eax\n\t"
+		     "movl 4(%1), %%edx\n\t"
+		     LOCK_PREFIX "cmpxchg8b %0\n\t"
 		     "jnz 1b"
-		     : /* no outputs */
-		     : "D"(ptr),
+		     : "=m"(*ptr)
+		     : "D" (ptr),
 		       "b"(low),
 		       "c"(high)
 		     : "ax", "dx", "memory");

@@ -82,20 +82,20 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
 	switch (size) {
 	case 1:
 		asm volatile("xchgb %b0,%1"
-			     : "=q" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=q" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	case 2:
 		asm volatile("xchgw %w0,%1"
-			     : "=r" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=r" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	case 4:
 		asm volatile("xchgl %0,%1"
-			     : "=r" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=r" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	}

@@ -139,21 +139,21 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
 	unsigned long prev;
 	switch (size) {
 	case 1:
-		asm volatile(LOCK_PREFIX "cmpxchgb %b1,%2"
-			     : "=a"(prev)
-			     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile(LOCK_PREFIX "cmpxchgb %b2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "q"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 2:
-		asm volatile(LOCK_PREFIX "cmpxchgw %w1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile(LOCK_PREFIX "cmpxchgw %w2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 4:
-		asm volatile(LOCK_PREFIX "cmpxchgl %1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile(LOCK_PREFIX "cmpxchgl %2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	}

@@ -172,21 +172,21 @@ static inline unsigned long __sync_cmpxchg(volatile void *ptr,
 	unsigned long prev;
 	switch (size) {
 	case 1:
-		asm volatile("lock; cmpxchgb %b1,%2"
-			     : "=a"(prev)
-			     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("lock; cmpxchgb %b2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "q"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 2:
-		asm volatile("lock; cmpxchgw %w1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("lock; cmpxchgw %w2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 4:
-		asm volatile("lock; cmpxchgl %1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("lock; cmpxchgl %2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	}

@@ -200,21 +200,21 @@ static inline unsigned long __cmpxchg_local(volatile void *ptr,
 	unsigned long prev;
 	switch (size) {
 	case 1:
-		asm volatile("cmpxchgb %b1,%2"
-			     : "=a"(prev)
-			     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("cmpxchgb %b2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "q"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 2:
-		asm volatile("cmpxchgw %w1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("cmpxchgw %w2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 4:
-		asm volatile("cmpxchgl %1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("cmpxchgl %2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	}

@@ -226,11 +226,10 @@ static inline unsigned long long __cmpxchg64(volatile void *ptr,
 					     unsigned long long new)
 {
 	unsigned long long prev;
-	asm volatile(LOCK_PREFIX "cmpxchg8b %3"
-		     : "=A"(prev)
+	asm volatile(LOCK_PREFIX "cmpxchg8b %1"
+		     : "=A"(prev), "+m" (*__xg(ptr))
 		     : "b"((unsigned long)new),
 		       "c"((unsigned long)(new>>  32)),
-		       "m"(*__xg(ptr)),
 		       "0"(old)
 		     : "memory");
 	return prev;

@@ -241,11 +240,10 @@ static inline unsigned long long __cmpxchg64_local(volatile void *ptr,
 						   unsigned long long new)
 {
 	unsigned long long prev;
-	asm volatile("cmpxchg8b %3"
-		     : "=A"(prev)
+	asm volatile("cmpxchg8b %1"
+		     : "=A"(prev), "+m"(*__xg(ptr))
 		     : "b"((unsigned long)new),
 		       "c"((unsigned long)(new>>  32)),
-		       "m"(*__xg(ptr)),
 		       "0"(old)
 		     : "memory");
 	return prev;

Thanks, I'll queue it up when it hits Linus's tree.

greg k-h

Hello,

2.6.32.21 does not boot on my HP Compaq nx7300. I bisected it to this patch:

Here's a video of the crash: http://store.lisk.in/tmp/09092010019.mp4
With boot_delay=30, it sometimes just reboots and sometimes prints the trace. Without it, it always prints the trace. The crash occurs during the initialization of intel-agp (which explains why I can't reproduce it in qemu). More details in the video.

The system boots and works just fine with 2.6.32.21 with the patch reverted. v2.6.36-rc3-185-gd56557a works fine as well, perhaps there were other fixes that didn't get to stable (e.g. 4532b30).

I'm compiling the kernel with the latest gcc in Debian (-dumpversion says 4.4.5, dpkg reports 4:4.4.4-2). Shall I post dmesg/lspci as well?

Since I have an easy workaround and other things to do, I won't spend time trying to backport those other fixes myself, but if you're going to fix this, I will help with testing.

Regards,

Tomáš Janoušek, a.k.a. Liskni_si, http://work.lisk.in/

Could you give us your .config and also what version of gcc and binutils is on your system?

-hpa

Hello,

http://store.lisk.in/tmp/.config.2.6.32.21-debug

$ ld -V
GNU ld (GNU Binutils for Debian) 2.20.1-system.20100303

(that's 2.20.1-12 in Debian testing)

$ cc -dumpversion
4.4.5

(that's 4:4.4.4-2 in Debian testing)

$ cat /proc/version 
Linux version 2.6.36-rc3-lis-00187-ga784c32 ([email protected]) (gcc version
4.4.5 20100728 (prerelease) (Debian 4.4.4-8) ) #103 SMP Thu Sep 9 16:40:09
CEST 2010

Regards,

Tomáš Janoušek, a.k.a. Liskni_si, http://work.lisk.in/

I believe this needs 69309a05907546fb686b251d4ab041c26afe1e1d, which I think is being queued up for stable.

Could you try applying 69309a05907546fb686b251d4ab041c26afe1e1d and see if it solves your problem.

-hpa

Hello,

It does indeed. Thanks.

Tomáš Janoušek, a.k.a. Liskni_si, http://work.lisk.in/

Hm, that patch doesn't apply to the latest .32-stable tree. Can someone provide me with a backported version so I make sure I get it correct?

thanks,

greg k-h

Hello,

I'm attaching the one I used and tested.

Tomáš Janoušek, a.k.a. Liskni_si, http://work.lisk.in/

From 48a45b4cd701520435188fcf9ef0c04e73c79172 Mon Sep 17 00:00:00 2001
From: H. Peter Anvin <[email protected]>
Date: Tue, 27 Jul 2010 23:29:52 -0700
Subject: [PATCH] x86, asm: Clean up and simplify set_64bit()

Clean up and simplify set_64bit(). This code is quite old (1.3.11) and contains a fair bit of auxilliary machinery that current versions of gcc handle just fine automatically. Worse, the auxilliary machinery can actually cause an unnecessary spill to memory.

Furthermore, the loading of the old value inside the loop in the 32-bit case is unnecessary: if the value doesn't match, the CMPXCHG8B instruction will already have loaded the "new previous" value for us.

Clean up the comment, too, and remove page references to obsolete versions of the Intel SDM.

LKML-Reference: <tip-*@vger.kernel.org>

---
 arch/x86/include/asm/cmpxchg_32.h |   65 +++++++++++--------------------------
 arch/x86/include/asm/cmpxchg_64.h |    4 +--
 2 files changed, 20 insertions(+), 49 deletions(-)

index 5af5051..9873a5f 100644
@@ -17,60 +17,33 @@ struct __xchg_dummy {
 #define __xg(x) ((struct __xchg_dummy *)(x))
 
 /*
- * The semantics of XCHGCMP8B are a bit strange, this is why
- * there is a loop and the loading of %%eax and %%edx has to
- * be inside. This inlines well in most cases, the cached
- * cost is around ~38 cycles. (in the future we might want
- * to do an SIMD/3DNOW!/MMX/FPU 64-bit store here, but that
- * might have an implicit FPU-save as a cost, so it's not
- * clear which path to go.)
+ * CMPXCHG8B only writes to the target if we had the previous
+ * value in registers, otherwise it acts as a read and gives us the
+ * "new previous" value.  That is why there is a loop.  Preloading
+ * EDX:EAX is a performance optimization: in the common case it means
+ * we need only one locked operation.
  *
- * cmpxchg8b must be used with the lock prefix here to allow
- * the instruction to be executed atomically, see page 3-102
- * of the instruction set reference 24319102.pdf. We need
- * the reader side to see the coherent 64bit value.
+ * A SIMD/3DNOW!/MMX/FPU 64-bit store here would require at the very
+ * least an FPU save and/or %cr0.ts manipulation.
+ *
+ * cmpxchg8b must be used with the lock prefix here to allow the
+ * instruction to be executed atomically.  We need to have the reader
+ * side to see the coherent 64bit value.
  */
-static inline void __set_64bit(unsigned long long *ptr,
-			       unsigned int low, unsigned int high)
+static inline void set_64bit(volatile u64 *ptr, u64 value)
 {
+	u32 low  = value;
+	u32 high = value >> 32;
+	u64 prev = *ptr;
+
 	asm volatile("\n1:\t"
-		     "movl (%1), %%eax\n\t"
-		     "movl 4(%1), %%edx\n\t"
 		     LOCK_PREFIX "cmpxchg8b %0\n\t"
 		     "jnz 1b"
-		     : "=m"(*ptr)
-		     : "D" (ptr),
-		       "b"(low),
-		       "c"(high)
-		     : "ax", "dx", "memory");
-}
-
-static inline void __set_64bit_constant(unsigned long long *ptr,
-					unsigned long long value)
-{
-	__set_64bit(ptr, (unsigned int)value, (unsigned int)(value >> 32));
-}
-
-#define ll_low(x)	*(((unsigned int *)&(x)) + 0)
-#define ll_high(x)	*(((unsigned int *)&(x)) + 1)
-
-static inline void __set_64bit_var(unsigned long long *ptr,
-				   unsigned long long value)
-{
-	__set_64bit(ptr, ll_low(value), ll_high(value));
+		     : "=m" (*ptr), "+A" (prev)
+		     : "b" (low), "c" (high)
+		     : "memory");
 }
 
-#define set_64bit(ptr, value)			\
-	(__builtin_constant_p((value))		\
-	 ? __set_64bit_constant((ptr), (value))	\
-	 : __set_64bit_var((ptr), (value)))
-
-#define _set_64bit(ptr, value)						\
-	(__builtin_constant_p(value)					\
-	 ? __set_64bit(ptr, (unsigned int)(value),			\
-		       (unsigned int)((value) >> 32))			\
-	 : __set_64bit(ptr, ll_low((value)), ll_high((value))))
-
 /*
  * Note: no "lock" prefix even on SMP: xchg always implies lock anyway
  * Note 2: xchg has side effect, so that attribute volatile is necessary,

index 1871cb0..e8cb051 100644
@@ -8,13 +8,11 @@
 
 #define __xg(x) ((volatile long *)(x))
 
-static inline void set_64bit(volatile unsigned long *ptr, unsigned long val)
+static inline void set_64bit(volatile u64 *ptr, u64 val)
 {
 	*ptr = val;
 }
 
-#define _set_64bit set_64bit
-
 /*
  * Note: no "lock" prefix even on SMP: xchg always implies lock anyway
  * Note 2: xchg has side effect, so that attribute volatile is necessary,

1.7.1

That worked, thanks, I've queued it up now.

greg k-h

Thanks a lot everyone. This patch resolves the issue for me on top of 2.6.32.17.

(I had some whitespace issue with applying the patch, so I've attached the cleaned up version in case it wasn't just me.)

Cheers,
Peter

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

From f6fd342ddae3f7edecd0254e5c4e6795d4fba4a9 Mon Sep 17 00:00:00 2001
From: Jeremy Fitzhardinge <[email protected]>
Date: Tue, 27 Jul 2010 23:03:58 -0700
Subject: [PATCH] x86: Add memory modify constraints to xchg() and cmpxchg()

xchg() and cmpxchg() modify their memory operands, not merely read them. For some versions of gcc the "memory" clobber has apparently dealt with the situation, but not for all.

Also adds the missing 8-byte case for __sync_cmpxchg().

Based on HPA's patch.

---
 arch/x86/include/asm/cmpxchg_32.h |   86 +++++++++++++++++------------------
 arch/x86/include/asm/cmpxchg_64.h |   88 ++++++++++++++++++++-----------------
 2 files changed, 89 insertions(+), 85 deletions(-)

index ee1931b..5af5051 100644
@@ -34,12 +34,12 @@ static inline void __set_64bit(unsigned long long *ptr,
 			       unsigned int low, unsigned int high)
 {
 	asm volatile("\n1:\t"
-		     "movl (%0), %%eax\n\t"
-		     "movl 4(%0), %%edx\n\t"
-		     LOCK_PREFIX "cmpxchg8b (%0)\n\t"
+		     "movl (%1), %%eax\n\t"
+		     "movl 4(%1), %%edx\n\t"
+		     LOCK_PREFIX "cmpxchg8b %0\n\t"
 		     "jnz 1b"
-		     : /* no outputs */
-		     : "D"(ptr),
+		     : "=m"(*ptr)
+		     : "D" (ptr),
 		       "b"(low),
 		       "c"(high)
 		     : "ax", "dx", "memory");

@@ -82,20 +82,20 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr,
 	switch (size) {
 	case 1:
 		asm volatile("xchgb %b0,%1"
-			     : "=q" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=q" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	case 2:
 		asm volatile("xchgw %w0,%1"
-			     : "=r" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=r" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	case 4:
 		asm volatile("xchgl %0,%1"
-			     : "=r" (x)
-			     : "m" (*__xg(ptr)), "0" (x)
+			     : "=r" (x), "+m" (*__xg(ptr))
+			     : "0" (x)
 			     : "memory");
 		break;
 	}

@@ -139,21 +139,21 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
 	unsigned long prev;
 	switch (size) {
 	case 1:
-		asm volatile(LOCK_PREFIX "cmpxchgb %b1,%2"
-			     : "=a"(prev)
-			     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile(LOCK_PREFIX "cmpxchgb %b2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "q"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 2:
-		asm volatile(LOCK_PREFIX "cmpxchgw %w1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile(LOCK_PREFIX "cmpxchgw %w2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 4:
-		asm volatile(LOCK_PREFIX "cmpxchgl %1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile(LOCK_PREFIX "cmpxchgl %2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	}

@@ -172,21 +172,21 @@ static inline unsigned long __sync_cmpxchg(volatile void *ptr,
 	unsigned long prev;
 	switch (size) {
 	case 1:
-		asm volatile("lock; cmpxchgb %b1,%2"
-			     : "=a"(prev)
-			     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("lock; cmpxchgb %b2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "q"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 2:
-		asm volatile("lock; cmpxchgw %w1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("lock; cmpxchgw %w2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 4:
-		asm volatile("lock; cmpxchgl %1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("lock; cmpxchgl %2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	}

@@ -200,21 +200,21 @@ static inline unsigned long __cmpxchg_local(volatile void *ptr,
 	unsigned long prev;
 	switch (size) {
 	case 1:
-		asm volatile("cmpxchgb %b1,%2"
-			     : "=a"(prev)
-			     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("cmpxchgb %b2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "q"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 2:
-		asm volatile("cmpxchgw %w1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("cmpxchgw %w2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	case 4:
-		asm volatile("cmpxchgl %1,%2"
-			     : "=a"(prev)
-			     : "r"(new), "m"(*__xg(ptr)), "0"(old)
+		asm volatile("cmpxchgl %2,%1"
+			     : "=a"(prev), "+m"(*__xg(ptr))
+			     : "r"(new), "0"(old)
 			     : "memory");
 		return prev;
 	}

@@ -226,11 +226,10 @@ static inline unsigned long long __cmpxchg64(volatile void *ptr,
 					     unsigned long long new)
 {
 	unsigned long long prev;
-	asm volatile(LOCK_PREFIX "cmpxchg8b %3"
-		     : "=A"(prev)
+	asm volatile(LOCK_PREFIX "cmpxchg8b %1"
+		     : "=A"(prev), "+m" (*__xg(ptr))
 		     : "b"((unsigned long)new),
 		       "c"((unsigned long)(new >> 32)),
-		       "m"(*__xg(ptr)),
 		       "0"(old)
 		     : "memory");
 	return prev;

@@ -241,11 +240,10 @@ static inline unsigned long long __cmpxchg64_local(volatile void *ptr,
 						   unsigned long long new)
 {
 	unsigned long long prev;
-	asm volatile("cmpxchg8b %3"
-		     : "=A"(prev)
+	asm volatile("cmpxchg8b %1"
+		     : "=A"(prev), "+m"(*__xg(ptr))
 		     : "b"((unsigned long)new),
 		       "c"((unsigned long)(new >> 32)),
-		       "m"(*__xg(ptr)),
 		       "0"(old)
 		     : "memory");
 	return prev;

1.5.6.5

Thanks, I needed this version, it wasn't just you :)

greg k-h

gcc (Debian 4.3.2-1.1) 4.3.2

                           |  .''`.  ** Debian GNU/Linux **
      Peter Palfrader      | : :' :      The  universal
 http://www.palfrader.org/ | `. `'      Operating System
                           |   `-    http://www.debian.org/

The experimental Debian packages of 2.6.35-rc5 are built with gcc 4.4 and they don't have this bug - last_value is marked as 'b' in System.map, which I assume means BSS. So this may be specific to gcc 4.3.

Ben.

Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Avi Kivity <[email protected]>

The current lmsw implementation allows the guest to clear cr0.pe, contrary to the manual, which breaks EMM386.EXE.

Fix by ORing the old cr0.pe with lmsw's operand.

---
 arch/x86/kvm/x86.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -345,7 +345,7 @@ EXPORT_SYMBOL_GPL(kvm_set_cr0);
 
 void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw)
 {
-	kvm_set_cr0(vcpu, (vcpu->arch.cr0 & ~0x0ful) | (msw & 0x0f));
+	kvm_set_cr0(vcpu, (vcpu->arch.cr0 & ~0x0eul) | (msw & 0x0f));
 }
 EXPORT_SYMBOL_GPL(kvm_lmsw);
 

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Sheng Yang <[email protected]>

kvm_x86_ops->set_efer() would execute vcpu->arch.efer = efer, so the checking of LMA bit didn't work.

---
 arch/x86/kvm/x86.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -516,11 +516,11 @@ static void set_efer(struct kvm_vcpu *vc
 		}
 	}
 
-	kvm_x86_ops->set_efer(vcpu, efer);
-
 	efer &= ~EFER_LMA;
 	efer |= vcpu->arch.shadow_efer & EFER_LMA;
 
+	kvm_x86_ops->set_efer(vcpu, efer);
+
 	vcpu->arch.shadow_efer = efer;
 
 	vcpu->arch.mmu.base_role.nxe = (efer & EFER_NX) && !tdp_enabled;

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Avi Kivity <[email protected]>

When cr0.wp=0, we may shadow a gpte having u/s=1 and r/w=0 with an spte having u/s=0 and r/w=1. This allows excessive access if the guest sets cr0.wp=1 and accesses through this spte.

Fix by making cr0.wp part of the base role; we'll have different sptes for the two cases and the problem disappears.

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/mmu.c              |    3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

@@ -193,6 +193,7 @@ union kvm_mmu_page_role {
 		unsigned invalid:1;
 		unsigned cr4_pge:1;
 		unsigned nxe:1;
+		unsigned cr0_wp:1;
 	};
 };
 

@@ -221,7 +221,7 @@ void kvm_mmu_set_mask_ptes(u64 user_mask
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_set_mask_ptes);
 
-static int is_write_protection(struct kvm_vcpu *vcpu)
+static bool is_write_protection(struct kvm_vcpu *vcpu)
 {
 	return vcpu->arch.cr0 & X86_CR0_WP;
 }

@@ -2445,6 +2445,7 @@ static int init_kvm_softmmu(struct kvm_v
 		r = paging32_init_context(vcpu);
 
 	vcpu->arch.mmu.base_role.glevels = vcpu->arch.mmu.root_level;
+	vcpu->arch.mmu.base_role.cr0_wp = is_write_protection(vcpu);
 
 	return r;
 }

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Shane Wang <[email protected]>

Per document, for feature control MSR:

Bit 1 enables VMXON in SMX operation. If the bit is clear, execution
of VMXON in SMX operation causes a general-protection exception.
Bit 2 enables VMXON outside SMX operation. If the bit is clear, execution
of VMXON outside SMX operation causes a general-protection exception.

This patch is to enable this kind of check with SMX for VMXON in KVM.

---
 arch/x86/include/asm/msr-index.h |    5 +++--
 arch/x86/kernel/tboot.c          |    1 +
 arch/x86/kvm/vmx.c               |   32 +++++++++++++++++++++-----------
 include/linux/tboot.h            |    1 +
 4 files changed, 26 insertions(+), 13 deletions(-)

@@ -198,8 +198,9 @@
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
 
-#define FEATURE_CONTROL_LOCKED		(1<<0)
-#define FEATURE_CONTROL_VMXON_ENABLED	(1<<2)
+#define FEATURE_CONTROL_LOCKED				(1<<0)
+#define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
+#define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
 
 #define MSR_IA32_APICBASE		0x0000001b
 #define MSR_IA32_APICBASE_BSP		(1<<8)

@@ -46,6 +46,7 @@
 
 /* Global pointer to shared data; NULL means no measured launch. */
 struct tboot *tboot __read_mostly;
+EXPORT_SYMBOL(tboot);
 
 /* timeout for APs (in secs) to enter wait-for-SIPI state during shutdown */
 #define AP_WAIT_TIMEOUT		1

@@ -26,6 +26,7 @@
 #include <linux/sched.h>
 #include <linux/moduleparam.h>
 #include <linux/ftrace_event.h>
+#include <linux/tboot.h>
 #include "kvm_cache_regs.h"
 #include "x86.h"
 

@@ -1140,9 +1141,16 @@ static __init int vmx_disabled_by_bios(v
 	u64 msr;
 
 	rdmsrl(MSR_IA32_FEATURE_CONTROL, msr);
-	return (msr & (FEATURE_CONTROL_LOCKED |
-		       FEATURE_CONTROL_VMXON_ENABLED))
-	    == FEATURE_CONTROL_LOCKED;
+	if (msr & FEATURE_CONTROL_LOCKED) {
+		if (!(msr & FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX)
+			&& tboot_enabled())
+			return 1;
+		if (!(msr & FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX)
+			&& !tboot_enabled())
+			return 1;
+	}
+
+	return 0;
 	/* locked but not enabled */
 }
 

@@ -1150,18 +1158,20 @@ static void hardware_enable(void *garbag
 {
 	int cpu = raw_smp_processor_id();
 	u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
-	u64 old;
+	u64 old, test_bits;
 
 	INIT_LIST_HEAD(&per_cpu(vcpus_on_cpu, cpu));
 	rdmsrl(MSR_IA32_FEATURE_CONTROL, old);
-	if ((old & (FEATURE_CONTROL_LOCKED |
-		    FEATURE_CONTROL_VMXON_ENABLED))
-	    != (FEATURE_CONTROL_LOCKED |
-		FEATURE_CONTROL_VMXON_ENABLED))
+
+	test_bits = FEATURE_CONTROL_LOCKED;
+	test_bits |= FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
+	if (tboot_enabled())
+		test_bits |= FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX;
+
+	if ((old & test_bits) != test_bits) {
 		/* enable and lock */
-		wrmsrl(MSR_IA32_FEATURE_CONTROL, old |
-		       FEATURE_CONTROL_LOCKED |
-		       FEATURE_CONTROL_VMXON_ENABLED);
+		wrmsrl(MSR_IA32_FEATURE_CONTROL, old | test_bits);
+	}
 	write_cr4(read_cr4() | X86_CR4_VMXE); /* FIXME: not cpu hotplug safe */
 	asm volatile (ASM_VMX_VMXON_RAX
 		      : : "a"(&phys_addr), "m"(phys_addr)

@@ -150,6 +150,7 @@ extern int tboot_force_iommu(void);
 
 #else
 
+#define tboot_enabled()			0
 #define tboot_probe()			do { } while (0)
 #define tboot_shutdown(shutdown_type)	do { } while (0)
 #define tboot_sleep(sleep_state, pm1a_control, pm1b_control)	\

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Avi Kivity <[email protected]>

On svm, kvm_read_pdptr() may require reading guest memory, which can sleep.

Push the spinlock into mmu_alloc_roots(), and only take it after we've read the pdptr.

---
 arch/x86/kvm/mmu.c |    7 +++++++
 1 file changed, 7 insertions(+)

@@ -2091,11 +2091,13 @@ static int mmu_alloc_roots(struct kvm_vc
 			direct = 1;
 		if (mmu_check_root(vcpu, root_gfn))
 			return 1;
+		spin_lock(&vcpu->kvm->mmu_lock);
 		sp = kvm_mmu_get_page(vcpu, root_gfn, 0,
 				      PT64_ROOT_LEVEL, direct,
 				      ACC_ALL, NULL);
 		root = __pa(sp->spt);
 		++sp->root_count;
+		spin_unlock(&vcpu->kvm->mmu_lock);
 		vcpu->arch.mmu.root_hpa = root;
 		return 0;
 	}

@@ -2117,11 +2119,14 @@ static int mmu_alloc_roots(struct kvm_vc
 			root_gfn = 0;
 		if (mmu_check_root(vcpu, root_gfn))
 			return 1;
+		spin_lock(&vcpu->kvm->mmu_lock);
 		sp = kvm_mmu_get_page(vcpu, root_gfn, i << 30,
 				      PT32_ROOT_LEVEL, direct,
 				      ACC_ALL, NULL);
 		root = __pa(sp->spt);
 		++sp->root_count;
+		spin_unlock(&vcpu->kvm->mmu_lock);
+
 		vcpu->arch.mmu.pae_root[i] = root | PT_PRESENT_MASK;
 	}
 	vcpu->arch.mmu.root_hpa = __pa(vcpu->arch.mmu.pae_root);

@@ -2485,7 +2490,9 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
 		goto out;
 	spin_lock(&vcpu->kvm->mmu_lock);
 	kvm_mmu_free_some_pages(vcpu);
+	spin_unlock(&vcpu->kvm->mmu_lock);
 	r = mmu_alloc_roots(vcpu);
+	spin_lock(&vcpu->kvm->mmu_lock);
 	mmu_sync_roots(vcpu);
 	spin_unlock(&vcpu->kvm->mmu_lock);
 	if (r)

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Avi Kivity <[email protected]>

Wallclock writing uses an unprotected global variable to hold the version; this can cause one guest to interfere with another if both write their wallclock at the same time.

---
 arch/x86/kvm/x86.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

@@ -554,14 +554,22 @@ static int do_set_msr(struct kvm_vcpu *v
 
 static void kvm_write_wall_clock(struct kvm *kvm, gpa_t wall_clock)
 {
-	static int version;
+	int version;
+	int r;
 	struct pvclock_wall_clock wc;
 	struct timespec boot;
 
 	if (!wall_clock)
 		return;
 
-	version++;
+	r = kvm_read_guest(kvm, wall_clock, &version, sizeof(version));
+	if (r)
+		return;
+
+	if (version & 1)
+		++version;  /* first time write, random junk */
+
+	++version;
 
 	kvm_write_guest(kvm, wall_clock, &version, sizeof(version));
 

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Avi Kivity <[email protected]>

---
 arch/x86/kvm/x86.c |    6 ++++++
 1 file changed, 6 insertions(+)

@@ -1420,6 +1420,7 @@ static int kvm_vcpu_ioctl_get_cpuid2(str
 {
 	int r;
 
+	vcpu_load(vcpu);
 	r = -E2BIG;
 	if (cpuid->nent < vcpu->arch.cpuid_nent)
 		goto out;

@@ -1431,6 +1432,7 @@ static int kvm_vcpu_ioctl_get_cpuid2(str
 
 out:
 	cpuid->nent = vcpu->arch.cpuid_nent;
+	vcpu_put(vcpu);
 	return r;
 }
 

@@ -1680,6 +1682,7 @@ static int kvm_vcpu_ioctl_x86_setup_mce(
 	int r;
 	unsigned bank_num = mcg_cap & 0xff, bank;
 
+	vcpu_load(vcpu);
 	r = -EINVAL;
 	if (!bank_num || bank_num >= KVM_MAX_MCE_BANKS)
 		goto out;

@@ -1694,6 +1697,7 @@ static int kvm_vcpu_ioctl_x86_setup_mce(
 	for (bank = 0; bank < bank_num; bank++)
 		vcpu->arch.mce_banks[bank*4] = ~(u64)0;
 out:
+	vcpu_put(vcpu);
 	return r;
 }
 

@@ -1896,7 +1900,9 @@ long kvm_arch_vcpu_ioctl(struct file *fi
 		r = -EFAULT;
 		if (copy_from_user(&mce, argp, sizeof mce))
 			goto out;
+		vcpu_load(vcpu);
 		r = kvm_vcpu_ioctl_x86_set_mce(vcpu, &mce);
+		vcpu_put(vcpu);
 		break;
 	}
 	default:

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Roedel, Joerg <[email protected]>

This patch fixes a bug in the KVM efer-msr write path. If a guest writes to a reserved efer bit the set_efer function injects the #GP directly. The architecture dependent wrmsr function does not see this, assumes success and advances the rip. This results in a #GP in the guest with the wrong rip. This patch fixes this by reporting efer write errors back to the architectural wrmsr function.

---
 arch/x86/kvm/x86.c |   31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

@@ -483,37 +483,29 @@ static u32 emulated_msrs[] = {
 	MSR_IA32_MISC_ENABLE,
 };
 
-static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
+static int set_efer(struct kvm_vcpu *vcpu, u64 efer)
 {
-	if (efer & efer_reserved_bits) {
-		kvm_inject_gp(vcpu, 0);
-		return;
-	}
+	if (efer & efer_reserved_bits)
+		return 1;
 
 	if (is_paging(vcpu)
-	    && (vcpu->arch.shadow_efer & EFER_LME) != (efer & EFER_LME)) {
-		kvm_inject_gp(vcpu, 0);
-		return;
-	}
+	    && (vcpu->arch.shadow_efer & EFER_LME) != (efer & EFER_LME))
+		return 1;
 
 	if (efer & EFER_FFXSR) {
 		struct kvm_cpuid_entry2 *feat;
 
 		feat = kvm_find_cpuid_entry(vcpu, 0x80000001, 0);
-		if (!feat || !(feat->edx & bit(X86_FEATURE_FXSR_OPT))) {
-			kvm_inject_gp(vcpu, 0);
-			return;
-		}
+		if (!feat || !(feat->edx & bit(X86_FEATURE_FXSR_OPT)))
+			return 1;
 	}
 
 	if (efer & EFER_SVME) {
 		struct kvm_cpuid_entry2 *feat;
 
 		feat = kvm_find_cpuid_entry(vcpu, 0x80000001, 0);
-		if (!feat || !(feat->ecx & bit(X86_FEATURE_SVM))) {
-			kvm_inject_gp(vcpu, 0);
-			return;
-		}
+		if (!feat || !(feat->ecx & bit(X86_FEATURE_SVM)))
+			return 1;
 	}
 
 	efer &= ~EFER_LMA;

@@ -525,6 +517,8 @@ static void set_efer(struct kvm_vcpu *vc
 
 	vcpu->arch.mmu.base_role.nxe = (efer & EFER_NX) && !tdp_enabled;
 	kvm_mmu_reset_context(vcpu);
+
+	return 0;
 }
 
 void kvm_enable_efer_bits(u64 mask)

@@ -827,8 +821,7 @@ int kvm_set_msr_common(struct kvm_vcpu *
 {
 	switch (msr) {
 	case MSR_EFER:
-		set_efer(vcpu, data);
-		break;
+		return set_efer(vcpu, data);
 	case MSR_K7_HWCR:
 		data &= ~(u64)0x40;	/* ignore flush filter disable */
 		if (data != 0) {

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Yin Kangkai <[email protected]>

commit 765f8361902d015c864d5e62019b2f139452d7ef upstream.

jbd-debug and jbd2-debug is currently read-only (S_IRUGO), which is not correct. Make it writable so that we can start debuging.

---
 fs/jbd/journal.c  |    2 +-
 fs/jbd2/journal.c |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

@@ -1913,7 +1913,7 @@ static void __init jbd_create_debugfs_en
 {
 	jbd_debugfs_dir = debugfs_create_dir("jbd", NULL);
 	if (jbd_debugfs_dir)
-		jbd_debug = debugfs_create_u8("jbd-debug", S_IRUGO,
+		jbd_debug = debugfs_create_u8("jbd-debug", S_IRUGO | S_IWUSR,
 					       jbd_debugfs_dir,
 					       &journal_enable_debug);
 }

@@ -2115,7 +2115,8 @@ static void __init jbd2_create_debugfs_e
 {
 	jbd2_debugfs_dir = debugfs_create_dir("jbd2", NULL);
 	if (jbd2_debugfs_dir)
-		jbd2_debug = debugfs_create_u8(JBD2_DEBUG_NAME, S_IRUGO,
+		jbd2_debug = debugfs_create_u8(JBD2_DEBUG_NAME,
+					       S_IRUGO | S_IWUSR,
 					       jbd2_debugfs_dir,
 					       &jbd2_journal_enable_debug);
 }

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Helge Deller <[email protected]>

commit 550f0d922286556c7ea43974bb7921effb5a5278 upstream.

Clear the floating point exception flag before returning to user space. This is needed, else the libc trampoline handler may hit the same SIGFPE again while building up a trampoline to a signal handler.

Fixes debian bug #559406.

---
 arch/parisc/math-emu/decode_exc.c |    1 +
 1 file changed, 1 insertion(+)

@@ -342,6 +342,7 @@ decode_fpu(unsigned int Fpu_register[],
 		return SIGNALCODE(SIGFPE, FPE_FLTINV);
 	  case DIVISIONBYZEROEXCEPTION:
 		update_trap_counts(Fpu_register, aflags, bflags, trap_counts);
+		Clear_excp_register(exception_index);
 	  	return SIGNALCODE(SIGFPE, FPE_FLTDIV);
 	  case INEXACTEXCEPTION:
 		update_trap_counts(Fpu_register, aflags, bflags, trap_counts);

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Mikulas Patocka <[email protected]>

commit 102c6ddb1d081a6a1fede38c43a42c9811313ec7 upstream.

Removed unnecessary 'and' masking: The right shift discards the lower bits so there is no need to clear them.

(A later patch needs this change to support a 32-bit chunk_mask.)

---
 drivers/md/dm-exception-store.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -162,7 +162,7 @@ static inline sector_t get_dev_size(stru
 static inline chunk_t sector_to_chunk(struct dm_exception_store *store,
 				      sector_t sector)
 {
-	return (sector & ~store->chunk_mask) >> store->chunk_shift;
+	return sector >> store->chunk_shift;
 }
 
 int dm_exception_store_type_register(struct dm_exception_store_type *type);

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Dan Carpenter <[email protected]>

commit 4d09ec0f705cf88a12add029c058b53f288cfaa2 upstream.

We were using the wrong variable here so the error codes weren't being returned properly. The original code returns -ENOKEY.

---
 security/keys/process_keys.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

@@ -509,7 +509,7 @@ try_again:
 
 			ret = install_thread_keyring();
 			if (ret < 0) {
-				key = ERR_PTR(ret);
+				key_ref = ERR_PTR(ret);
 				goto error;
 			}
 			goto reget_creds;

@@ -527,7 +527,7 @@ try_again:
 
 			ret = install_process_keyring();
 			if (ret < 0) {
-				key = ERR_PTR(ret);
+				key_ref = ERR_PTR(ret);
 				goto error;
 			}
 			goto reget_creds;

@@ -586,7 +586,7 @@ try_again:
 
 	case KEY_SPEC_GROUP_KEYRING:
 		/* group keyrings are not yet supported */
-		key = ERR_PTR(-EINVAL);
+		key_ref = ERR_PTR(-EINVAL);
 		goto error;
 
 	case KEY_SPEC_REQKEY_AUTH_KEY:

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Toshiyuki Okajima <[email protected]>

commit cea7daa3589d6b550546a8c8963599f7c1a3ae5c upstream.

find_keyring_by_name() can gain access to a keyring that has had its reference count reduced to zero, and is thus ready to be freed. This then allows the dead keyring to be brought back into use whilst it is being destroyed.

The following timeline illustrates the process:

|(cleaner)                           (user)
|
| free_user(user)                    sys_keyctl()
|  |                                  |
|  key_put(user->session_keyring)     keyctl_get_keyring_ID()
|  ||	//=> keyring->usage = 0        |
|  |schedule_work(&key_cleanup_task)   lookup_user_key()
|  ||                                   |
|  kmem_cache_free(,user)               |
|  .                                    |[KEY_SPEC_USER_KEYRING]
|  .                                    install_user_keyrings()
|  .                                    ||
| key_cleanup() [<= worker_thread()]    ||
|  |                                    ||
|  [spin_lock(&key_serial_lock)]        |[mutex_lock(&key_user_keyr..mutex)]
|  |                                    ||
|  atomic_read() == 0                   ||
|  |{ rb_ease(&key->serial_node,) }     ||
|  |                                    ||
|  [spin_unlock(&key_serial_lock)]      |find_keyring_by_name()
|  |                                    |||
|  keyring_destroy(keyring)             ||[read_lock(&keyring_name_lock)]
|  ||                                   |||
|  |[write_lock(&keyring_name_lock)]    ||atomic_inc(&keyring->usage)
|  |.                                   ||| *** GET freeing keyring ***
|  |.                                   ||[read_unlock(&keyring_name_lock)]
|  ||                                   ||
|  |list_del()                          |[mutex_unlock(&key_user_k..mutex)]
|  ||                                   |
|  |[write_unlock(&keyring_name_lock)]  ** INVALID keyring is returned **
|  |                                    .
|  kmem_cache_free(,keyring)            .
|                                       .
|                                       atomic_dec(&keyring->usage)

v *** DESTROYED ***
TIME

If CONFIG_SLUB_DEBUG=y then we may see the following message generated:

	=============================================================================
	BUG key_jar: Poison overwritten
	-----------------------------------------------------------------------------

	INFO: 0xffff880197a7e200-0xffff880197a7e200. First byte 0x6a instead of 0x6b
	INFO: Allocated in key_alloc+0x10b/0x35f age=25 cpu=1 pid=5086
	INFO: Freed in key_cleanup+0xd0/0xd5 age=12 cpu=1 pid=10
	INFO: Slab 0xffffea000592cb90 objects=16 used=2 fp=0xffff880197a7e200 flags=0x200000000000c3
	INFO: Object 0xffff880197a7e200 @offset=512 fp=0xffff880197a7e300

	Bytes b4 0xffff880197a7e1f0:  5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
	  Object 0xffff880197a7e200:  6a 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b jkkkkkkkkkkkkkkk

Alternatively, we may see a system panic happen, such as:

	BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
	IP: [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9
	PGD 6b2b4067 PUD 6a80d067 PMD 0
	Oops: 0000 [#1] SMP
	last sysfs file: /sys/kernel/kexec_crash_loaded
	CPU 1
	...
	Pid: 31245, comm: su Not tainted 2.6.34-rc5-nofixed-nodebug #2 D2089/PRIMERGY
	RIP: 0010:[<ffffffff810e61a3>]  [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9
	RSP: 0018:ffff88006af3bd98  EFLAGS: 00010002
	RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffff88007d19900b
	RDX: 0000000100000000 RSI: 00000000000080d0 RDI: ffffffff81828430
	RBP: ffffffff81828430 R08: ffff88000a293750 R09: 0000000000000000
	R10: 0000000000000001 R11: 0000000000100000 R12: 00000000000080d0
	R13: 00000000000080d0 R14: 0000000000000296 R15: ffffffff810f20ce
	FS:  00007f97116bc700(0000) GS:ffff88000a280000(0000) knlGS:0000000000000000
	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
	CR2: 0000000000000001 CR3: 000000006a91c000 CR4: 00000000000006e0
	DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
	DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
	Process su (pid: 31245, threadinfo ffff88006af3a000, task ffff8800374414c0)
	Stack:
	 0000000512e0958e 0000000000008000 ffff880037f8d180 0000000000000001
	 0000000000000000 0000000000008001 ffff88007d199000 ffffffff810f20ce
	 0000000000008000 ffff88006af3be48 0000000000000024 ffffffff810face3
	Call Trace:
	 [<ffffffff810f20ce>] ? get_empty_filp+0x70/0x12f
	 [<ffffffff810face3>] ? do_filp_open+0x145/0x590
	 [<ffffffff810ce208>] ? tlb_finish_mmu+0x2a/0x33
	 [<ffffffff810ce43c>] ? unmap_region+0xd3/0xe2
	 [<ffffffff810e4393>] ? virt_to_head_page+0x9/0x2d
	 [<ffffffff81103916>] ? alloc_fd+0x69/0x10e
	 [<ffffffff810ef4ed>] ? do_sys_open+0x56/0xfc
	 [<ffffffff81008a02>] ? system_call_fastpath+0x16/0x1b
	Code: 0f 1f 44 00 00 49 89 c6 fa 66 0f 1f 44 00 00 65 4c 8b 04 25 60 e8 00 00 48 8b 45 00 49 01 c0 49 8b 18 48 85 db 74 0d 48 63 45 18 <48> 8b 04 03 49 89 00 eb 14 4c 89 f9 83 ca ff 44 89 e6 48 89 ef
	RIP  [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9

This problem is that find_keyring_by_name does not confirm that the keyring is valid before accepting it.

Skipping keyrings that have been reduced to a zero count seems the way to go. To this end, use atomic_inc_not_zero() to increment the usage count and skip the candidate keyring if that returns false.

The following script _may_ cause the bug to happen, but there's no guarantee as the window of opportunity is small:

	#!/bin/sh
	LOOP=100000
	USER=dummy_user
	/bin/su -c "exit;" $USER || { /usr/sbin/adduser -m $USER; add=1; }
	for ((i=0; i<LOOP; i++))
	do
		/bin/su -c "echo '$i' > /dev/null" $USER
	done
	(( add == 1 )) && /usr/sbin/userdel -r $USER
	exit

Note that the nominated user must not be in use.

An alternative way of testing this may be:

	for ((i=0; i<100000; i++))
	do
		keyctl session foo /bin/true || break
	done >&/dev/null

as that uses a keyring named "foo" rather than relying on the user and user-session named keyrings.

---
 security/keys/keyring.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

@@ -524,9 +524,8 @@ struct key *find_keyring_by_name(const c
 	struct key *keyring;
 	int bucket;
 
-	keyring = ERR_PTR(-EINVAL);
 	if (!name)
-		goto error;
+		return ERR_PTR(-EINVAL);
 
 	bucket = keyring_hash(name);
 

@@ -553,17 +552,18 @@ struct key *find_keyring_by_name(const c
 					   KEY_SEARCH) < 0)
 				continue;
 
-			/* we've got a match */
-			atomic_inc(&keyring->usage);
-			read_unlock(&keyring_name_lock);
-			goto error;
+			/* we've got a match but we might end up racing with
+			 * key_cleanup() if the keyring is currently 'dead'
+			 * (ie. it has a zero usage count) */
+			if (!atomic_inc_not_zero(&keyring->usage))
+				continue;
+			goto out;
 		}
 	}
 
-	read_unlock(&keyring_name_lock);
 	keyring = ERR_PTR(-ENOKEY);
-
- error:
+out:
+	read_unlock(&keyring_name_lock);
 	return keyring;
 
 } /* end find_keyring_by_name() */

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Ben Hutchings <[email protected]>

commit 6377a7ae1ab82859edccdbc8eaea63782efb134d upstream.

On specific platforms, MSI is unreliable on some of the QLA24xx chips, resulting in fatal I/O errors under load, as reported in <http://bugs.debian.org/572322>
and by some RHEL customers.

---
 drivers/scsi/qla2xxx/qla_isr.c |   28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

@@ -2240,30 +2240,28 @@ qla2x00_request_irqs(struct qla_hw_data
 
 	/* If possible, enable MSI-X. */
 	if (!IS_QLA2432(ha) && !IS_QLA2532(ha) &&
-	    !IS_QLA8432(ha) && !IS_QLA8001(ha))
-		goto skip_msix;
+		!IS_QLA8432(ha) && !IS_QLA8001(ha))
+		goto skip_msi;
+
+	if (ha->pdev->subsystem_vendor == PCI_VENDOR_ID_HP &&
+		(ha->pdev->subsystem_device == 0x7040 ||
+		ha->pdev->subsystem_device == 0x7041 ||
+		ha->pdev->subsystem_device == 0x1705)) {
+		DEBUG2(qla_printk(KERN_WARNING, ha,
+			"MSI-X: Unsupported ISP2432 SSVID/SSDID (0x%X,0x%X).\n",
+			ha->pdev->subsystem_vendor,
+			ha->pdev->subsystem_device));
+		goto skip_msi;
+	}
 
 	if (IS_QLA2432(ha) && (ha->pdev->revision < QLA_MSIX_CHIP_REV_24XX ||
 		!QLA_MSIX_FW_MODE_1(ha->fw_attributes))) {
 		DEBUG2(qla_printk(KERN_WARNING, ha,
 		"MSI-X: Unsupported ISP2432 (0x%X, 0x%X).\n",
 			ha->pdev->revision, ha->fw_attributes));
-
 		goto skip_msix;
 	}
 
-	if (ha->pdev->subsystem_vendor == PCI_VENDOR_ID_HP &&
-	    (ha->pdev->subsystem_device == 0x7040 ||
-		ha->pdev->subsystem_device == 0x7041 ||
-		ha->pdev->subsystem_device == 0x1705)) {
-		DEBUG2(qla_printk(KERN_WARNING, ha,
-		    "MSI-X: Unsupported ISP2432 SSVID/SSDID (0x%X, 0x%X).\n",
-		    ha->pdev->subsystem_vendor,
-		    ha->pdev->subsystem_device));
-
-		goto skip_msi;
-	}
-
 	ret = qla24xx_enable_msix(ha, rsp);
 	if (!ret) {
 		DEBUG2(qla_printk(KERN_INFO, ha,

2.6.32-stable review patch. If anyone has any objections, please let us know.

------------------

From: Wei Yongjun <[email protected]>

commit 2e3219b5c8a2e44e0b83ae6e04f52f20a82ac0f2 upstream.

commit 5fa782c2f5ef6c2e4f04d3e228412c9b4a4c8809
sctp: Fix skb_over_panic resulting from multiple invalid \

    parameter errors (CVE-2010-1173) (v4)

cause 'error cause' never be add the the ERROR chunk due to some typo when check valid length in sctp_init_cause_fixed().

---
 net/sctp/sm_make_chunk.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -140,7 +140,7 @@ int sctp_init_cause_fixed(struct sctp_ch
 	len = sizeof(sctp_errhdr_t) + paylen;
 	err.length  = htons(len);
 
-	if (skb_tailroom(chunk->skb) >  len)
+	if (skb_tailroom(chunk->skb) < len)
 		return -ENOSPC;
 	chunk->subh.err_hdr = sctp_addto_chunk_fixed(chunk,
 						     sizeof(sctp_errhdr_t),

@@ -1412,7 +1412,7 @@ void *sctp_addto_chunk(struct sctp_chunk
 void *sctp_addto_chunk_fixed(struct sctp_chunk *chunk,
 			     int len, const void *data)
 {
-	if (skb_tailroom(chunk->skb) > len)
+	if (skb_tailroom(chunk->skb) >= len)
 		return sctp_addto_chunk(chunk, len, data);
 	else
 		return NULL;

This is the start of the stable review cycle for the 2.6.34.1 release. There are 200 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know. If anyone is a maintainer of the proper subsystem, and wants to add a Signed-off-by: line to the patch, please respond with it.

Responses should be made by Sat, July 3, 17:00:00 UTC UTC. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

        kernel.org/pub/linux/kernel/v2.6/stable-review/patch-2.6.34.1-rc1.gz

and the diffstat can be found below.

thanks,

greg k-h

 Documentation/hwmon/ltc4245                     |    4 +-
 Makefile                                        |    2 +-
 arch/arm/common/sa1111.c                        |    9 +-
 arch/arm/mach-mx2/devices.c                     |    4 +-
 arch/arm/mm/cache-v7.S                          |    4 +
 arch/arm/mm/copypage-feroceon.c                 |    4 +-
 arch/arm/mm/copypage-v4wb.c                     |    4 +-
 arch/arm/mm/copypage-v4wt.c                     |    4 +-
 arch/arm/mm/copypage-xsc3.c                     |    4 +-
 arch/arm/mm/fault.c                             |    3 +
 arch/arm/mm/init.c                              |    6 +-
 arch/arm/vfp/vfphw.S                            |    2 +-
 arch/blackfin/include/asm/cache.h               |    2 +
 arch/frv/include/asm/cache.h                    |    2 +
 arch/m68k/include/asm/cache.h                   |    2 +
 arch/mn10300/include/asm/cache.h                |    2 +
 arch/parisc/math-emu/decode_exc.c               |    1 +
 arch/powerpc/kernel/asm-offsets.c               |    8 +
 arch/powerpc/kernel/head_fsl_booke.S            |   13 +-
 arch/powerpc/kvm/book3s.c                       |   10 +
 arch/powerpc/kvm/booke.c                        |   15 +-
 arch/powerpc/kvm/powerpc.c                      |    3 +-
 arch/powerpc/lib/string.S                       |    4 +-
 arch/powerpc/mm/fsl_booke_mmu.c                 |   25 +--
 arch/powerpc/mm/mmu_decl.h                      |   10 +-
 arch/powerpc/mm/pgtable_32.c                    |    8 +
 arch/powerpc/mm/pgtable_64.c                    |    8 +
 arch/powerpc/mm/tlb_nohash_low.S                |   28 ++
 arch/powerpc/oprofile/op_model_cell.c           |    2 +-
 arch/powerpc/platforms/pseries/hotplug-cpu.c    |   30 +--
 arch/powerpc/platforms/pseries/plpar_wrappers.h |    8 +
 arch/powerpc/platforms/pseries/smp.c            |   28 ++
 arch/s390/kvm/kvm-s390.c                        |    4 +-
 arch/x86/include/asm/kvm_host.h                 |    3 +
 arch/x86/include/asm/msr-index.h                |    5 +-
 arch/x86/kernel/amd_iommu.c                     |   16 +-
 arch/x86/kernel/amd_iommu_init.c                |   20 ++-
 arch/x86/kernel/cpu/perf_event.c                |    5 +-
 arch/x86/kernel/pvclock.c                       |   24 ++
 arch/x86/kernel/setup.c                         |   11 +
 arch/x86/kernel/tboot.c                         |    1 +
 arch/x86/kvm/mmu.c                              |   10 +-
 arch/x86/kvm/svm.c                              |  163 ++++++++-----
 arch/x86/kvm/vmx.c                              |   38 ++-
 arch/x86/kvm/x86.c                              |   58 +++--
 arch/x86/oprofile/nmi_int.c                     |   10 +-
 arch/x86/xen/suspend.c                          |    4 +-
 arch/xtensa/include/asm/cache.h                 |    1 +
 block/cfq-iosched.c                             |   20 +-
 drivers/acpi/video_detect.c                     |    2 +-
 drivers/ata/libata-core.c                       |    7 +-
 drivers/ata/libata-sff.c                        |    2 +-
 drivers/ata/sata_nv.c                           |    5 +-
 drivers/ata/sata_via.c                          |   13 +
 drivers/base/cpu.c                              |    2 +-
 drivers/char/ipmi/ipmi_si_intf.c                |   11 +-
 drivers/clocksource/sh_cmt.c                    |   21 +-
 drivers/clocksource/sh_tmu.c                    |   20 +-
 drivers/firewire/core-card.c                    |   24 ++-
 drivers/gpu/drm/drm_edid.c                      |    2 +-
 drivers/gpu/drm/i915/i915_gem.c                 |   19 ++
 drivers/gpu/drm/i915/intel_display.c            |    6 -
 drivers/gpu/drm/i915/intel_dp.c                 |   10 -
 drivers/gpu/drm/radeon/radeon.h                 |    1 +
 drivers/gpu/drm/radeon/radeon_agp.c             |    5 +
 drivers/gpu/drm/radeon/radeon_atombios.c        |    7 +-
 drivers/gpu/drm/radeon/radeon_device.c          |    2 +
 drivers/gpu/drm/radeon/radeon_display.c         |    5 +-
 drivers/gpu/drm/radeon/radeon_state.c           |    5 +-
 drivers/hid/hid-core.c                          |    1 +
 drivers/hid/hid-gyration.c                      |    1 +
 drivers/hid/hid-ids.h                           |    1 +
 drivers/hwmon/ltc4245.c                         |   18 +-
 drivers/md/linear.c                             |    1 +
 drivers/md/md.c                                 |   99 +++++++-
 drivers/md/md.h                                 |    1 +
 drivers/md/raid1.c                              |    9 +-
 drivers/md/raid10.c                             |    2 +-
 drivers/md/raid5.c                              |    7 +-
 drivers/media/video/uvc/uvc_ctrl.c              |    2 +
 drivers/misc/vmware_balloon.c                   |   18 ++-
 drivers/net/arcnet/com20020-pci.c               |    4 +-
 drivers/net/can/sja1000/sja1000.c               |   20 ++-
 drivers/net/can/sja1000/sja1000.h               |    1 +
 drivers/net/mlx4/icm.c                          |    3 +-
 drivers/net/wireless/ath/ar9170/hw.h            |    1 +
 drivers/net/wireless/ath/ar9170/main.c          |    2 +-
 drivers/net/wireless/ath/ar9170/usb.c           |   10 +
 drivers/net/wireless/ath/ath5k/base.c           |   11 +-
 drivers/net/wireless/ath/ath9k/hw.c             |    4 +-
 drivers/net/wireless/ath/ath9k/init.c           |    3 +
 drivers/net/wireless/iwlwifi/iwl-agn-rs.c       |   10 +-
 drivers/net/wireless/iwlwifi/iwl-scan.c         |   21 ++-
 drivers/net/wireless/iwlwifi/iwl-tx.c           |    2 +
 drivers/net/wireless/p54/p54usb.c               |    1 +
 drivers/net/wireless/rtl818x/rtl8180_dev.c      |    1 +
 drivers/net/wireless/wl12xx/wl1251_sdio.c       |    1 +
 drivers/oprofile/cpu_buffer.c                   |   63 +----
 drivers/pci/quirks.c                            |   13 +-
 drivers/pcmcia/ds.c                             |    1 +
 drivers/pcmcia/yenta_socket.c                   |    5 +-
 drivers/platform/x86/Kconfig                    |    1 +
 drivers/rtc/rtc-cmos.c                          |    5 +-
 drivers/rtc/rtc-s3c.c                           |    5 +-
 drivers/scsi/libsas/sas_ata.c                   |    5 +-
 drivers/scsi/libsas/sas_scsi_host.c             |   16 +-
 drivers/serial/68328serial.c                    |    2 +-
 drivers/staging/batman-adv/proc.c               |   51 +++--
 drivers/staging/batman-adv/vis.c                |  235 ++++++++++++------
 drivers/staging/batman-adv/vis.h                |    8 +-
 drivers/staging/comedi/drivers/ni_mio_cs.c      |    2 +-
 drivers/staging/rt2860/usb_main_dev.c           |    1 +
 drivers/staging/vt6655/device_main.c            |    2 +
 drivers/usb/class/cdc-acm.c                     |    6 +-
 drivers/usb/core/driver.c                       |    2 +
 drivers/usb/core/hcd.c                          |  169 ++++++++-----
 drivers/usb/core/message.c                      |   45 +---
 drivers/usb/core/urb.c                          |    9 +-
 drivers/usb/core/usb.c                          |    4 +
 drivers/usb/gadget/fsl_udc_core.c               |    2 +-
 drivers/usb/host/ehci-au1xxx.c                  |   16 +-
 drivers/usb/host/ehci-fsl.c                     |    2 +
 drivers/usb/host/ehci-hub.c                     |  158 ++++++++++---
 drivers/usb/host/ehci-pci.c                     |   15 +-
 drivers/usb/host/ehci.h                         |   10 +
 drivers/usb/host/fhci.h                         |    9 +-
 drivers/usb/host/whci/qset.c                    |    2 +-
 drivers/usb/host/xhci-pci.c                     |    2 +-
 drivers/usb/host/xhci-ring.c                    |   39 +++-
 drivers/usb/host/xhci.c                         |   54 ++++-
 drivers/usb/mon/mon_bin.c                       |    2 +-
 drivers/usb/mon/mon_text.c                      |    4 +-
 drivers/usb/serial/cp210x.c                     |   11 +
 drivers/usb/serial/cypress_m8.c                 |    2 +-
 drivers/usb/serial/digi_acceleport.c            |    4 +-
 drivers/usb/serial/ftdi_sio.c                   |    4 +
 drivers/usb/serial/ir-usb.c                     |    2 +
 drivers/usb/serial/kl5kusb105.c                 |    1 +
 drivers/usb/serial/kobil_sct.c                  |    3 +-
 drivers/usb/serial/mos7840.c                    |    1 -
 drivers/usb/serial/option.c                     |  180 ++++++++++++++
 drivers/usb/serial/qcaux.c                      |    5 +
 drivers/usb/serial/spcp8x5.c                    |    4 +-
 drivers/usb/serial/visor.c                      |    1 +
 drivers/usb/storage/unusual_devs.h              |   15 ++
 drivers/vhost/net.c                             |    2 +-
 drivers/video/arcfb.c                           |    8 +-
 drivers/video/hgafb.c                           |   10 +-
 drivers/video/vfb.c                             |    4 +-
 drivers/video/vga16fb.c                         |   10 +-
 drivers/video/w100fb.c                          |   10 +-
 drivers/xen/xenbus/xenbus_xs.c                  |    2 +-
 fs/aio.c                                        |   65 +++--
 fs/btrfs/acl.c                                  |    3 +
 fs/cifs/cifsproto.h                             |    6 +-
 fs/cifs/dir.c                                   |   20 +-
 fs/cifs/file.c                                  |   11 +-
 fs/compat.c                                     |  132 ++++++----
 fs/dcache.c                                     |    1 +
 fs/exofs/dir.c                                  |    2 +-
 fs/ext4/move_extent.c                           |    3 +
 fs/ext4/resize.c                                |    3 +-
 fs/fs-writeback.c                               |   14 +-
 fs/gfs2/file.c                                  |    7 +
 fs/libfs.c                                      |    3 +-
 fs/namei.c                                      |    2 +-
 fs/nfs/write.c                                  |   20 ++-
 fs/nfsd/nfssvc.c                                |    2 +-
 fs/nfsd/vfs.c                                   |   11 +-
 fs/nfsd/vfs.h                                   |    1 +
 include/linux/aio.h                             |    5 +
 include/linux/compat.h                          |    4 +
 include/linux/fs.h                              |    2 +-
 include/linux/miscdevice.h                      |    7 +-
 include/linux/pci_ids.h                         |    1 +
 include/linux/perf_event.h                      |    5 +-
 include/linux/slub_def.h                        |   11 +-
 include/linux/swap.h                            |   10 -
 include/linux/tboot.h                           |    1 +
 include/linux/usb.h                             |    9 +
 include/trace/events/signal.h                   |    3 +-
 kernel/cgroup.c                                 |    2 +-
 kernel/compat.c                                 |   25 +-
 kernel/mutex.c                                  |    7 +
 kernel/perf_event.c                             |  299 +++++++++++++---------
 kernel/posix-timers.c                           |   11 +-
 kernel/signal.c                                 |    6 +-
 lib/idr.c                                       |    5 +-
 mm/filemap.c                                    |   10 +-
 mm/slub.c                                       |   33 +--
 net/mac80211/Kconfig                            |    9 +
 net/mac80211/cfg.c                              |    7 +-
 net/mac80211/mlme.c                             |   40 +++-
 net/mac80211/rx.c                               |    3 +-
 net/mac80211/tx.c                               |    5 +-
 scripts/kconfig/Makefile                        |    2 +-
 sound/core/pcm_lib.c                            |   13 +-
 sound/core/pcm_native.c                         |   39 +---
 sound/pci/hda/hda_intel.c                       |    9 +-
 sound/pci/hda/patch_realtek.c                   |    1 +
 sound/pci/hda/patch_sigmatel.c                  |    2 +-
 sound/soc/codecs/wm8350.c                       |    4 +-
 sound/soc/codecs/wm8400.c                       |   18 +-
 sound/soc/codecs/wm8990.c                       |   18 +-
 sound/soc/imx/imx-pcm-dma-mx2.c                 |    7 +-
 tools/perf/bench/mem-memcpy.c                   |    2 +-
 tools/perf/bench/sched-messaging.c              |    4 +-
 tools/perf/builtin-annotate.c                   |    8 +-
 tools/perf/builtin-buildid-cache.c              |    2 +-
 tools/perf/builtin-buildid-list.c               |    4 +-
 tools/perf/builtin-diff.c                       |    4 +-
 tools/perf/builtin-help.c                       |    2 +-
 tools/perf/builtin-lock.c                       |    2 +-
 tools/perf/builtin-probe.c                      |    2 +-
 tools/perf/builtin-record.c                     |   24 +-
 tools/perf/builtin-report.c                     |    6 +-
 tools/perf/builtin-sched.c                      |    6 +-
 tools/perf/builtin-stat.c                       |   10 +-
 tools/perf/builtin-timechart.c                  |    2 +-
 tools/perf/builtin-top.c                        |   47 +++--
 tools/perf/builtin-trace.c                      |    2 +-
 tools/perf/util/debug.c                         |    2 +-
 tools/perf/util/debug.h                         |    3 +-
 tools/perf/util/parse-options.c                 |    6 +
 tools/perf/util/parse-options.h                 |    4 +-
 tools/perf/util/symbol.c                        |   25 +-
 tools/perf/util/symbol.h                        |   15 ++
 tools/perf/util/trace-event-parse.c             |    2 +-
 tools/perf/util/trace-event.h                   |    3 +-
 229 files changed, 2312 insertions(+), 1134 deletions(-)

2.6.34-stable review patch. If anyone has any objections, please let me know.

------------------

From: Robert Richter <[email protected]>

commit 2623a1d55a6260c855e1f6d1895900b50b40a896 upstream.

This fixes a NULL pointer dereference that is triggered when taking a cpu offline after oprofile was initialized, e.g.:

 $ opcontrol --init
 $ opcontrol --start-daemon
 $ opcontrol --shutdown
 $ opcontrol --deinit
 $ echo 0 > /sys/devices/system/cpu/cpu1/online

See the crash dump below. Though the counter has been disabled the cpu notifier is still active and trying to use already freed counter data.

This fix is for linux-stable. To proper fix this, the hotplug code must be rewritten. Thus I will leave a WARN_ON_ONCE() message with this patch.

BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff8132ad57>] op_amd_stop+0x2d/0x8e
PGD 0
Oops: 0000 [#1] SMP
last sysfs file: /sys/devices/system/cpu/cpu1/online
CPU 1
Modules linked in:

Pid: 0, comm: swapper Not tainted 2.6.34-rc5-oprofile-x86_64-standard-00210-g8c00f06 #16 Anaheim/Anaheim

RIP: 0010:[<ffffffff8132ad57>]  [<ffffffff8132ad57>] op_amd_stop+0x2d/0x8e
RSP: 0018:ffff880001843f28  EFLAGS: 00010006
RAX: 0000000000000000 RBX: 0000000000000000 RCX: dead000000200200
RDX: ffff880001843f68 RSI: dead000000100100 RDI: 0000000000000000
RBP: ffff880001843f48 R08: 0000000000000000 R09: ffff880001843f08
R10: ffffffff8102c9a5 R11: ffff88000184ea80 R12: 0000000000000000
R13: ffff88000184f6c0 R14: 0000000000000000 R15: 0000000000000000
FS:  00007fec6a92e6f0(0000) GS:ffff880001840000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000000 CR3: 000000000163b000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process swapper (pid: 0, threadinfo ffff88042fcd8000, task ffff88042fcd51d0)
Stack:
 ffff880001843f48 0000000000000001 ffff88042e9f7d38 ffff880001843f68
<0> ffff880001843f58 ffffffff8132a602 ffff880001843f98 ffffffff810521b3
<0> ffff880001843f68 ffff880001843f68 ffff880001843f88 ffff88042fcd9fd8
Call Trace:
 <IRQ>
 [<ffffffff8132a602>] nmi_cpu_stop+0x21/0x23
 [<ffffffff810521b3>] generic_smp_call_function_single_interrupt+0xdf/0x11b
 [<ffffffff8101804f>] smp_call_function_single_interrupt+0x22/0x31
 [<ffffffff810029f3>] call_function_single_interrupt+0x13/0x20
 <EOI>
 [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff81008701>] ? default_idle+0x22/0x37
 [<ffffffff8100896d>] c1e_idle+0xdf/0xe6
 [<ffffffff813f1170>] ? atomic_notifier_call_chain+0x13/0x15
 [<ffffffff810012fb>] cpu_idle+0x4b/0x7e
 [<ffffffff813e8a4e>] start_secondary+0x1ae/0x1b2
Code: 89 e5 41 55 49 89 fd 41 54 45 31 e4 53 31 db 48 83 ec 08 89 df e8 be f8 ff ff 48 98 48 83 3c c5 10 67 7a 81 00 74 1f 49 8b 45 08 <42> 8b 0c 20 0f 32 48 c1 e2 20 25 ff ff bf ff 48 09 d0 48 89 c2
RIP  [<ffffffff8132ad57>] op_amd_stop+0x2d/0x8e
 RSP <ffff880001843f28>
CR2: 0000000000000000
---[ end trace 679ac372d674b757 ]---
Kernel panic - not syncing: Fatal exception in interrupt
Pid: 0, comm: swapper Tainted: G      D    2.6.34-rc5-oprofile-x86_64-standard-00210-g8c00f06 #16
Call Trace:
 <IRQ>  [<ffffffff813ebd6a>] panic+0x9e/0x10c
 [<ffffffff810474b0>] ? up+0x34/0x39
 [<ffffffff81031ccc>] ? kmsg_dump+0x112/0x12c
 [<ffffffff813eeff1>] oops_end+0x81/0x8e
 [<ffffffff8101efee>] no_context+0x1f3/0x202
 [<ffffffff8101f1b7>] __bad_area_nosemaphore+0x1ba/0x1e0
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff810264dc>] ? activate_task+0x42/0x53
 [<ffffffff8102c967>] ? try_to_wake_up+0x272/0x284
 [<ffffffff8101f1eb>] bad_area_nosemaphore+0xe/0x10
 [<ffffffff813f0f3f>] do_page_fault+0x1c8/0x37c
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff813ee55f>] page_fault+0x1f/0x30
 [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff8132ad57>] ? op_amd_stop+0x2d/0x8e
 [<ffffffff8132ad46>] ? op_amd_stop+0x1c/0x8e
 [<ffffffff8132a602>] nmi_cpu_stop+0x21/0x23
 [<ffffffff810521b3>] generic_smp_call_function_single_interrupt+0xdf/0x11b
 [<ffffffff8101804f>] smp_call_function_single_interrupt+0x22/0x31
 [<ffffffff810029f3>] call_function_single_interrupt+0x13/0x20
 <EOI>  [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff81008701>] ? default_idle+0x22/0x37
 [<ffffffff8100896d>] c1e_idle+0xdf/0xe6
 [<ffffffff813f1170>] ? atomic_notifier_call_chain+0x13/0x15
 [<ffffffff810012fb>] cpu_idle+0x4b/0x7e
 [<ffffffff813e8a4e>] start_secondary+0x1ae/0x1b2
------------[ cut here ]------------
WARNING: at /local/rrichter/.source/linux/arch/x86/kernel/smp.c:118 native_smp_send_reschedule+0x27/0x53()
Hardware name: Anaheim
Modules linked in:
Pid: 0, comm: swapper Tainted: G      D    2.6.34-rc5-oprofile-x86_64-standard-00210-g8c00f06 #16
Call Trace:
 <IRQ>  [<ffffffff81017f32>] ? native_smp_send_reschedule+0x27/0x53
 [<ffffffff81030ee2>] warn_slowpath_common+0x77/0xa4
 [<ffffffff81030f1e>] warn_slowpath_null+0xf/0x11
 [<ffffffff81017f32>] native_smp_send_reschedule+0x27/0x53
 [<ffffffff8102634b>] resched_task+0x60/0x62
 [<ffffffff8102653a>] check_preempt_curr_idle+0x10/0x12
 [<ffffffff8102c8ea>] try_to_wake_up+0x1f5/0x284
 [<ffffffff8102c986>] default_wake_function+0xd/0xf
 [<ffffffff810a110d>] pollwake+0x57/0x5a
 [<ffffffff8102c979>] ? default_wake_function+0x0/0xf
 [<ffffffff81026be5>] __wake_up_common+0x46/0x75
 [<ffffffff81026ed0>] __wake_up+0x38/0x50
 [<ffffffff81031694>] printk_tick+0x39/0x3b
 [<ffffffff8103ac37>] update_process_times+0x3f/0x5c
 [<ffffffff8104dc63>] tick_periodic+0x5d/0x69
 [<ffffffff8104dc90>] tick_handle_periodic+0x21/0x71
 [<ffffffff81018fd0>] smp_apic_timer_interrupt+0x82/0x95
 [<ffffffff81002853>] apic_timer_interrupt+0x13/0x20
 [<ffffffff81030cb5>] ? panic_blink_one_second+0x0/0x7b
 [<ffffffff813ebdd6>] ? panic+0x10a/0x10c
 [<ffffffff810474b0>] ? up+0x34/0x39
 [<ffffffff81031ccc>] ? kmsg_dump+0x112/0x12c
 [<ffffffff813eeff1>] ? oops_end+0x81/0x8e
 [<ffffffff8101efee>] ? no_context+0x1f3/0x202
 [<ffffffff8101f1b7>] ? __bad_area_nosemaphore+0x1ba/0x1e0
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff810264dc>] ? activate_task+0x42/0x53
 [<ffffffff8102c967>] ? try_to_wake_up+0x272/0x284
 [<ffffffff8101f1eb>] ? bad_area_nosemaphore+0xe/0x10
 [<ffffffff813f0f3f>] ? do_page_fault+0x1c8/0x37c
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff813ee55f>] ? page_fault+0x1f/0x30
 [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff8132ad57>] ? op_amd_stop+0x2d/0x8e
 [<ffffffff8132ad46>] ? op_amd_stop+0x1c/0x8e
 [<ffffffff8132a602>] ? nmi_cpu_stop+0x21/0x23
 [<ffffffff810521b3>] ? generic_smp_call_function_single_interrupt+0xdf/0x11b
 [<ffffffff8101804f>] ? smp_call_function_single_interrupt+0x22/0x31
 [<ffffffff810029f3>] ? call_function_single_interrupt+0x13/0x20
 <EOI>  [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff81008701>] ? default_idle+0x22/0x37
 [<ffffffff8100896d>] ? c1e_idle+0xdf/0xe6
 [<ffffffff813f1170>] ? atomic_notifier_call_chain+0x13/0x15
 [<ffffffff810012fb>] ? cpu_idle+0x4b/0x7e
 [<ffffffff813e8a4e>] ? start_secondary+0x1ae/0x1b2
---[ end trace 679ac372d674b758 ]---

@@ -95,7 +95,10 @@ static void nmi_cpu_save_registers(struct op_msrs *msrs)
 static void nmi_cpu_start(void *dummy)
 {
 	struct op_msrs const *msrs = &__get_cpu_var(cpu_msrs);
-	model->start(msrs);
+	if (!msrs->controls)
+		WARN_ON_ONCE(1);
+	else
+		model->start(msrs);
 }
 
 static int nmi_start(void)

@@ -107,7 +110,10 @@ static int nmi_start(void)
 static void nmi_cpu_stop(void *dummy)
 {
 	struct op_msrs const *msrs = &__get_cpu_var(cpu_msrs);
-	model->stop(msrs);
+	if (!msrs->controls)
+		WARN_ON_ONCE(1);
+	else
+		model->stop(msrs);
 }
 
 static void nmi_stop(void)

2.6.34-stable review patch. If anyone has any objections, please let me know.

------------------

From: Andi Kleen <[email protected]>

commit cb6e943ccf19ab6d3189147e9d625a992e016084 upstream.

oprofile used a double buffer scheme for its cpu event buffer to avoid races on reading with the old locked ring buffer.

But that is obsolete now with the new ring buffer, so simply use a single buffer. This greatly simplifies the code and avoids a lot of sample drops on large runs, especially with call graph.

Based on suggestions from Steven Rostedt

For stable kernels from v2.6.32, but not earlier.

---
 drivers/oprofile/cpu_buffer.c |   63 ++++++++----------------------------------
 1 file changed, 13 insertions(+), 50 deletions(-)

@@ -30,23 +30,7 @@
 
 #define OP_BUFFER_FLAGS	0
 
-/*
- * Read and write access is using spin locking. Thus, writing to the
- * buffer by NMI handler (x86) could occur also during critical
- * sections when reading the buffer. To avoid this, there are 2
- * buffers for independent read and write access. Read access is in
- * process context only, write access only in the NMI handler. If the
- * read buffer runs empty, both buffers are swapped atomically. There
- * is potentially a small window during swapping where the buffers are
- * disabled and samples could be lost.
- *
- * Using 2 buffers is a little bit overhead, but the solution is clear
- * and does not require changes in the ring buffer implementation. It
- * can be changed to a single buffer solution when the ring buffer
- * access is implemented as non-locking atomic code.
- */
-static struct ring_buffer *op_ring_buffer_read;
-static struct ring_buffer *op_ring_buffer_write;
+static struct ring_buffer *op_ring_buffer;
 DEFINE_PER_CPU(struct oprofile_cpu_buffer, op_cpu_buffer);
 
 static void wq_sync_buffer(struct work_struct *work);

@@ -68,12 +52,9 @@ void oprofile_cpu_buffer_inc_smpl_lost(v
 
 void free_cpu_buffers(void)
 {
-	if (op_ring_buffer_read)
-		ring_buffer_free(op_ring_buffer_read);
-	op_ring_buffer_read = NULL;
-	if (op_ring_buffer_write)
-		ring_buffer_free(op_ring_buffer_write);
-	op_ring_buffer_write = NULL;
+	if (op_ring_buffer)
+		ring_buffer_free(op_ring_buffer);
+	op_ring_buffer = NULL;
 }
 
 #define RB_EVENT_HDR_SIZE 4

@@ -86,11 +67,8 @@ int alloc_cpu_buffers(void)
 	unsigned long byte_size = buffer_size * (sizeof(struct op_sample) +
 						 RB_EVENT_HDR_SIZE);
 
-	op_ring_buffer_read = ring_buffer_alloc(byte_size, OP_BUFFER_FLAGS);
-	if (!op_ring_buffer_read)
-		goto fail;
-	op_ring_buffer_write = ring_buffer_alloc(byte_size, OP_BUFFER_FLAGS);
-	if (!op_ring_buffer_write)
+	op_ring_buffer = ring_buffer_alloc(byte_size, OP_BUFFER_FLAGS);
+	if (!op_ring_buffer)
 		goto fail;
 
 	for_each_possible_cpu(i) {

@@ -162,16 +140,11 @@ struct op_sample
 *op_cpu_buffer_write_reserve(struct op_entry *entry, unsigned long size)
 {
 	entry->event = ring_buffer_lock_reserve
-		(op_ring_buffer_write, sizeof(struct op_sample) +
+		(op_ring_buffer, sizeof(struct op_sample) +
 		 size * sizeof(entry->sample->data[0]));
-	if (entry->event)
-		entry->sample = ring_buffer_event_data(entry->event);
-	else
-		entry->sample = NULL;
-
-	if (!entry->sample)
+	if (!entry->event)
 		return NULL;
-
+	entry->sample = ring_buffer_event_data(entry->event);
 	entry->size = size;
 	entry->data = entry->sample->data;
 

@@ -180,25 +153,16 @@ struct op_sample
 
 int op_cpu_buffer_write_commit(struct op_entry *entry)
 {
-	return ring_buffer_unlock_commit(op_ring_buffer_write, entry->event);
+	return ring_buffer_unlock_commit(op_ring_buffer, entry->event);
 }
 
 struct op_sample *op_cpu_buffer_read_entry(struct op_entry *entry, int cpu)
 {
 	struct ring_buffer_event *e;
-	e = ring_buffer_consume(op_ring_buffer_read, cpu, NULL);
-	if (e)
-		goto event;
-	if (ring_buffer_swap_cpu(op_ring_buffer_read,
-				 op_ring_buffer_write,
-				 cpu))
+	e = ring_buffer_consume(op_ring_buffer, cpu, NULL);
+	if (!e)
 		return NULL;
-	e = ring_buffer_consume(op_ring_buffer_read, cpu, NULL);
-	if (e)
-		goto event;
-	return NULL;
 
-event:
 	entry->event = e;
 	entry->sample = ring_buffer_event_data(e);
 	entry->size = (ring_buffer_event_length(e) - sizeof(struct op_sample))

@@ -209,8 +173,7 @@ event:
 
 unsigned long op_cpu_buffer_entries(int cpu)
 {
-	return ring_buffer_entries_cpu(op_ring_buffer_read, cpu)
-		+ ring_buffer_entries_cpu(op_ring_buffer_write, cpu);
+	return ring_buffer_entries_cpu(op_ring_buffer, cpu);
 }
 
 static int

2.6.34-stable review patch. If anyone has any objections, please let me know.

------------------

From: Peter Zijlstra <[email protected]>

commit 050735b08ca8a016bbace4445fa025b88fee770b upstream.

Both Stephane and Corey reported that PERF_FORMAT_GROUP didn't work as expected if the task the counters were attached to quit before the read() call.

The cause is that we unconditionally destroy the grouping when we remove counters from their context. Fix this by splitting off the group destroy from the list removal such that perf_event_remove_from_context() does not do this and change perf_event_release() to do so.

LKML-Reference: <1273571513.5605.3527.camel@twins>

---
 kernel/perf_event.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

@@ -315,8 +315,6 @@ list_add_event(struct perf_event *event,
 static void
 list_del_event(struct perf_event *event, struct perf_event_context *ctx)
 {
-	struct perf_event *sibling, *tmp;
-
 	if (list_empty(&event->group_entry))
 		return;
 	ctx->nr_events--;

@@ -340,6 +338,12 @@ list_del_event(struct perf_event *event,
 	 */
 	if (event->state > PERF_EVENT_STATE_OFF)
 		event->state = PERF_EVENT_STATE_OFF;
+}
+
+static void
+perf_destroy_group(struct perf_event *event, struct perf_event_context *ctx)
+{
+	struct perf_event *sibling, *tmp;
 
 	/*
 	 * If this was a group event with sibling events then

@@ -1856,9 +1860,18 @@ int perf_event_release_kernel(struct per
 {
 	struct perf_event_context *ctx = event->ctx;
 
+	/*
+	 * Remove from the PMU, can't get re-enabled since we got
+	 * here because the last ref went.
+	 */
+	perf_event_disable(event);
+
 	WARN_ON_ONCE(ctx->parent_ctx);
 	mutex_lock(&ctx->mutex);
-	perf_event_remove_from_context(event);
+	raw_spin_lock_irq(&ctx->lock);
+	list_del_event(event, ctx);
+	perf_destroy_group(event, ctx);
+	raw_spin_unlock_irq(&ctx->lock);
 	mutex_unlock(&ctx->mutex);
 
 	mutex_lock(&event->owner->perf_event_mutex);

2.6.34-stable review patch. If anyone has any objections, please let me know.

------------------

From: Arnaldo Carvalho de Melo <[email protected]>

commit b0a9ab62ab96e258a0ddd81d7fe2719c3db36006 upstream.

Before this patch this message would very briefly appear on the screen and then the screen would get updates only on the top, for number of interrupts received, etc, but no annotation would be performed:

[root@doppio linux-2.6-tip]# perf top -s n_tty_write > /tmp/bla
objdump: '[kernel.kallsyms]': No such file

Now this is what the user gets:

[root@doppio linux-2.6-tip]# perf top -s n_tty_write Can't annotate n_tty_write: No vmlinux file was found in the
path: [0] vmlinux
[1] /boot/vmlinux
[2] /boot/vmlinux-2.6.33-rc5
[3] /lib/modules/2.6.33-rc5/build/vmlinux
[4] /usr/lib/debug/lib/modules/2.6.33-rc5/vmlinux [root@doppio linux-2.6-tip]#

This bug was introduced when we added automatic search for vmlinux, before that time the user had to specify a vmlinux file.

LKML-Reference: <[email protected]>

---
 tools/perf/builtin-top.c |   33 +++++++++++++++++++++++++--------
 tools/perf/util/symbol.c |   25 ++++++++++++-------------
 tools/perf/util/symbol.h |   15 +++++++++++++++
 3 files changed, 52 insertions(+), 21 deletions(-)

@@ -169,7 +169,7 @@ static void sig_winch_handler(int sig __
 	update_print_entries(&winsize);
 }
 
-static void parse_source(struct sym_entry *syme)
+static int parse_source(struct sym_entry *syme)
 {
 	struct symbol *sym;
 	struct sym_entry_source *source;

@@ -180,12 +180,21 @@ static void parse_source(struct sym_entr
 	u64 len;
 
 	if (!syme)
-		return;
+		return -1;
+
+	sym = sym_entry__symbol(syme);
+	map = syme->map;
+
+	/*
+	 * We can't annotate with just /proc/kallsyms
+	 */
+	if (map->dso->origin == DSO__ORIG_KERNEL)
+		return -1;
 
 	if (syme->src == NULL) {
 		syme->src = zalloc(sizeof(*source));
 		if (syme->src == NULL)
-			return;
+			return -1;
 		pthread_mutex_init(&syme->src->lock, NULL);
 	}
 

@@ -195,9 +204,6 @@ static void parse_source(struct sym_entr
 		pthread_mutex_lock(&source->lock);
 		goto out_assign;
 	}
-
-	sym = sym_entry__symbol(syme);
-	map = syme->map;
 	path = map->dso->long_name;
 
 	len = sym->end - sym->start;

@@ -209,7 +215,7 @@ static void parse_source(struct sym_entr
 
 	file = popen(command, "r");
 	if (!file)
-		return;
+		return -1;
 
 	pthread_mutex_lock(&source->lock);
 	source->lines_tail = &source->lines;

@@ -245,6 +251,7 @@ static void parse_source(struct sym_entr
 out_assign:
 	sym_filter_entry = syme;
 	pthread_mutex_unlock(&source->lock);
+	return 0;
 }
 
 static void __zero_source_counters(struct sym_entry *syme)

@@ -990,7 +997,17 @@ static void event__process_sample(const
 	if (sym_filter_entry_sched) {
 		sym_filter_entry = sym_filter_entry_sched;
 		sym_filter_entry_sched = NULL;
-		parse_source(sym_filter_entry);
+		if (parse_source(sym_filter_entry) < 0) {
+			struct symbol *sym = sym_entry__symbol(sym_filter_entry);
+
+			pr_err("Can't annotate %s", sym->name);
+			if (sym_filter_entry->map->dso->origin == DSO__ORIG_KERNEL) {
+				pr_err(": No vmlinux file was found in the path:\n");
+				vmlinux_path__fprintf(stderr);
+			} else
+				pr_err(".\n");
+			exit(1);
+		}
 	}
 
 	syme = symbol__priv(al.sym);

@@ -18,18 +18,6 @@
 #define NT_GNU_BUILD_ID 3
 #endif
 
-enum dso_origin {
-	DSO__ORIG_KERNEL = 0,
-	DSO__ORIG_JAVA_JIT,
-	DSO__ORIG_BUILD_ID_CACHE,
-	DSO__ORIG_FEDORA,
-	DSO__ORIG_UBUNTU,
-	DSO__ORIG_BUILDID,
-	DSO__ORIG_DSO,
-	DSO__ORIG_KMODULE,
-	DSO__ORIG_NOT_FOUND,
-};
-
 static void dsos__add(struct list_head *head, struct dso *dso);
 static struct map *map__new2(u64 start, struct dso *dso, enum map_type type);
 static int dso__load_kernel_sym(struct dso *self, struct map *map,

@@ -1025,7 +1013,7 @@ static int dso__load_sym(struct dso *sel
 				}
 				curr_map->map_ip = identity__map_ip;
 				curr_map->unmap_ip = identity__map_ip;
-				curr_dso->origin = DSO__ORIG_KERNEL;
+				curr_dso->origin = self->origin;
 				map_groups__insert(kmap->kmaps, curr_map);
 				dsos__add(&dsos__kernel, curr_dso);
 				dso__set_loaded(curr_dso, map->type);

@@ -1895,6 +1883,17 @@ out_fail:
 	return -1;
 }
 
+size_t vmlinux_path__fprintf(FILE *fp)
+{
+	int i;
+	size_t printed = 0;
+
+	for (i = 0; i < vmlinux_path__nr_entries; ++i)
+		printed += fprintf(fp, "[%d] %s\n", i, vmlinux_path[i]);
+
+	return printed;
+}
+
 static int setup_list(struct strlist **list, const char *list_str,
 		      const char *list_name)
 {

@@ -150,6 +150,19 @@ size_t dsos__fprintf_buildid(FILE *fp, b
 
 size_t dso__fprintf_buildid(struct dso *self, FILE *fp);
 size_t dso__fprintf(struct dso *self, enum map_type type, FILE *fp);
+
+enum dso_origin {
+	DSO__ORIG_KERNEL = 0,
+	DSO__ORIG_JAVA_JIT,
+	DSO__ORIG_BUILD_ID_CACHE,
+	DSO__ORIG_FEDORA,
+	DSO__ORIG_UBUNTU,
+	DSO__ORIG_BUILDID,
+	DSO__ORIG_DSO,
+	DSO__ORIG_KMODULE,
+	DSO__ORIG_NOT_FOUND,
+};
+
 char dso__symtab_origin(const struct dso *self);
 void dso__set_long_name(struct dso *self, char *name);
 void dso__set_build_id(struct dso *self, void *build_id);