• Christian Borntraeger's avatar
    virtio: fix race in enable_cb · 4265f161
    Christian Borntraeger authored
    There is a race in virtio_net, dealing with disabling/enabling the callback.
    I saw the following oops:
    
    kernel BUG at /space/kvm/drivers/virtio/virtio_ring.c:218!
    illegal operation: 0001 [#1] SMP
    Modules linked in: sunrpc dm_mod
    CPU: 2 Not tainted 2.6.25-rc1zlive-host-10623-gd358142-dirty #99
    Process swapper (pid: 0, task: 000000000f85a610, ksp: 000000000f873c60)
    Krnl PSW : 0404300180000000 00000000002b81a6 (vring_disable_cb+0x16/0x20)
               R:0 T:1 IO:0 EX:0 Key:0 M:1 W:0 P:0 AS:0 CC:3 PM:0 EA:3
    Krnl GPRS: 0000000000000001 0000000000000001 0000000010005800 0000000000000001
               000000000f3a0900 000000000f85a610 0000000000000000 0000000000000000
               0000000000000000 000000000f870000 0000000000000000 0000000000001237
               000000000f3a0920 000000000010ff74 00000000002846f6 000000000fa0bcd8
    Krnl Code: 00000000002b819a: a7110001           tmll    %r1,1
               00000000002b819e: a7840004           brc     8,2b81a6
               00000000002b81a2: a7f40001           brc     15,2b81a4
              >00000000002b81a6: a51b0001           oill    %r1,1
               00000000002b81aa: 40102000           sth     %r1,0(%r2)
               00000000002b81ae: 07fe               bcr     15,%r14
               00000000002b81b0: eb7ff0380024       stmg    %r7,%r15,56(%r15)
               00000000002b81b6: a7f13e00           tmll    %r15,15872
    Call Trace:
    ([<000000000fa0bcd0>] 0xfa0bcd0)
     [<00000000002b8350>] vring_interrupt+0x5c/0x6c
     [<000000000010ab08>] do_extint+0xb8/0xf0
     [<0000000000110716>] ext_no_vtime+0x16/0x1a
     [<0000000000107e72>] cpu_idle+0x1c2/0x1e0
    
    The problem can be triggered with a high amount of host->guest traffic.
    I think its the following race:
    
    poll says netif_rx_complete
    poll calls enable_cb
    enable_cb opens the interrupt mask
    a new packet comes, an interrupt is triggered----\
    enable_cb sees that there is more work           |
    enable_cb disables the interrupt                 |
           .                                         V
           .                            interrupt is delivered
           .                            skb_recv_done does atomic napi test, ok
     some waiting                       disable_cb is called->check fails->bang!
           .
    poll would do napi check
    poll would do disable_cb
    
    The fix is to let enable_cb not disable the interrupt again, but expect the
    caller to do the cleanup if it returns false. In that case, the interrupt is
    only disabled, if the napi test_set_bit was successful.
    Signed-off-by: default avatarChristian Borntraeger <borntraeger@de.ibm.com>
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (cleaned up doco)
    4265f161
virtio_ring.c 8.11 KB