• Chris Wilson's avatar
    drm/i915: Convert breadcrumbs spinlock to be irqsafe · f6168e33
    Chris Wilson authored
    The breadcrumbs are about to be used from within IRQ context sections
    (e.g. nouveau signals a fence from an interrupt handler causing us to
    submit a new request) and/or from bottom-half tasklets (i.e.
    intel_lrc_irq_handler), therefore we need to employ the irqsafe spinlock
    variants.
    
    For example, deferring the request submission to the
    intel_lrc_irq_handler generates this trace:
    
    [   66.388639] =================================
    [   66.388650] [ INFO: inconsistent lock state ]
    [   66.388663] 4.9.0-rc2+ #56 Not tainted
    [   66.388672] ---------------------------------
    [   66.388682] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
    [   66.388695] swapper/1/0 [HC0[0]:SC1[1]:HE0:SE0] takes:
    [   66.388706]  (&(&b->lock)->rlock){+.?...} , at: [<ffffffff81401c88>] intel_engine_enable_signaling+0x78/0x150
    [   66.388761] {SOFTIRQ-ON-W} state was registered at:
    [   66.388772]   [   66.388783] [<ffffffff810bd842>] __lock_acquire+0x682/0x1870
    [   66.388795]   [   66.388803] [<ffffffff810bedbc>] lock_acquire+0x6c/0xb0
    [   66.388814]   [   66.388824] [<ffffffff8161753a>] _raw_spin_lock+0x2a/0x40
    [   66.388835]   [   66.388845] [<ffffffff81401e41>] intel_engine_reset_breadcrumbs+0x21/0xb0
    [   66.388857]   [   66.388866] [<ffffffff81403ae7>] gen8_init_common_ring+0x67/0x100
    [   66.388878]   [   66.388887] [<ffffffff81403b92>] gen8_init_render_ring+0x12/0x60
    [   66.388903]   [   66.388912] [<ffffffff813f8707>] i915_gem_init_hw+0xf7/0x2a0
    [   66.388927]   [   66.388936] [<ffffffff813f899b>] i915_gem_init+0xbb/0xf0
    [   66.388950]   [   66.388959] [<ffffffff813b4980>] i915_driver_load+0x7e0/0x1330
    [   66.388978]   [   66.388988] [<ffffffff813c09d8>] i915_pci_probe+0x28/0x40
    [   66.389003]   [   66.389013] [<ffffffff812fa0db>] pci_device_probe+0x8b/0xf0
    [   66.389028]   [   66.389037] [<ffffffff8147737e>] driver_probe_device+0x21e/0x430
    [   66.389056]   [   66.389065] [<ffffffff8147766e>] __driver_attach+0xde/0xe0
    [   66.389080]   [   66.389090] [<ffffffff814751ad>] bus_for_each_dev+0x5d/0x90
    [   66.389105]   [   66.389113] [<ffffffff81477799>] driver_attach+0x19/0x20
    [   66.389134]   [   66.389144] [<ffffffff81475ced>] bus_add_driver+0x15d/0x260
    [   66.389159]   [   66.389168] [<ffffffff81477e3b>] driver_register+0x5b/0xd0
    [   66.389183]   [   66.389281] [<ffffffff812fa19b>] __pci_register_driver+0x5b/0x60
    [   66.389301]   [   66.389312] [<ffffffff81aed333>] i915_init+0x3e/0x45
    [   66.389326]   [   66.389336] [<ffffffff81ac2ffa>] do_one_initcall+0x8b/0x118
    [   66.389350]   [   66.389359] [<ffffffff81ac323a>] kernel_init_freeable+0x1b3/0x23b
    [   66.389378]   [   66.389387] [<ffffffff8160fc39>] kernel_init+0x9/0x100
    [   66.389402]   [   66.389411] [<ffffffff816180e7>] ret_from_fork+0x27/0x40
    [   66.389426] irq event stamp: 315865
    [   66.389438] hardirqs last  enabled at (315864): [<ffffffff816178f1>] _raw_spin_unlock_irqrestore+0x31/0x50
    [   66.389469] hardirqs last disabled at (315865): [<ffffffff816176b3>] _raw_spin_lock_irqsave+0x13/0x50
    [   66.389499] softirqs last  enabled at (315818): [<ffffffff8107a04c>] _local_bh_enable+0x1c/0x50
    [   66.389530] softirqs last disabled at (315819): [<ffffffff8107a50e>] irq_exit+0xbe/0xd0
    [   66.389559]
    [   66.389559] other info that might help us debug this:
    [   66.389580]  Possible unsafe locking scenario:
    [   66.389580]
    [   66.389598]        CPU0
    [   66.389609]        ----
    [   66.389620]   lock(&(&b->lock)->rlock);
    [   66.389650]   <Interrupt>
    [   66.389661]     lock(&(&b->lock)->rlock);
    [   66.389690]
    [   66.389690]  *** DEADLOCK ***
    [   66.389690]
    [   66.389715] 2 locks held by swapper/1/0:
    [   66.389728]  #0: (&(&tl->lock)->rlock){..-...}, at: [<ffffffff81403e01>] intel_lrc_irq_handler+0x201/0x3c0
    [   66.389785]  #1: (&(&req->lock)->rlock/1){..-...}, at: [<ffffffff813fc0af>] __i915_gem_request_submit+0x8f/0x170
    [   66.389854]
    [   66.389854] stack backtrace:
    [   66.389959] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.9.0-rc2+ #56
    [   66.389976] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
    [   66.389999]  ffff88027fd03c58 ffffffff812beae5 ffff88027696e680 ffffffff822afe20
    [   66.390036]  ffff88027fd03ca8 ffffffff810bb420 0000000000000001 0000000000000000
    [   66.390070]  0000000000000000 0000000000000006 0000000000000004 ffff88027696ee10
    [   66.390104] Call Trace:
    [   66.390117]  <IRQ>
    [   66.390128]  [<ffffffff812beae5>] dump_stack+0x68/0x93
    [   66.390147]  [<ffffffff810bb420>] print_usage_bug+0x1d0/0x1e0
    [   66.390164]  [<ffffffff810bb8a0>] mark_lock+0x470/0x4f0
    [   66.390181]  [<ffffffff810ba9d0>] ? print_shortest_lock_dependencies+0x1b0/0x1b0
    [   66.390203]  [<ffffffff810bd75d>] __lock_acquire+0x59d/0x1870
    [   66.390221]  [<ffffffff810bedbc>] lock_acquire+0x6c/0xb0
    [   66.390237]  [<ffffffff810bedbc>] ? lock_acquire+0x6c/0xb0
    [   66.390255]  [<ffffffff81401c88>] ? intel_engine_enable_signaling+0x78/0x150
    [   66.390273]  [<ffffffff8161753a>] _raw_spin_lock+0x2a/0x40
    [   66.390291]  [<ffffffff81401c88>] ? intel_engine_enable_signaling+0x78/0x150
    [   66.390309]  [<ffffffff81401c88>] intel_engine_enable_signaling+0x78/0x150
    [   66.390327]  [<ffffffff813fc170>] __i915_gem_request_submit+0x150/0x170
    [   66.390345]  [<ffffffff81403e8b>] intel_lrc_irq_handler+0x28b/0x3c0
    [   66.390363]  [<ffffffff81079d97>] tasklet_action+0x57/0xc0
    [   66.390380]  [<ffffffff8107a249>] __do_softirq+0x119/0x240
    [   66.390396]  [<ffffffff8107a50e>] irq_exit+0xbe/0xd0
    [   66.390414]  [<ffffffff8101afd5>] do_IRQ+0x65/0x110
    [   66.390431]  [<ffffffff81618806>] common_interrupt+0x86/0x86
    [   66.390446]  <EOI>
    [   66.390457]  [<ffffffff814ec6d1>] ? cpuidle_enter_state+0x151/0x200
    [   66.390480]  [<ffffffff814ec7a2>] cpuidle_enter+0x12/0x20
    [   66.390498]  [<ffffffff810b639e>] call_cpuidle+0x1e/0x40
    [   66.390516]  [<ffffffff810b65ae>] cpu_startup_entry+0x10e/0x1f0
    [   66.390534]  [<ffffffff81036133>] start_secondary+0x103/0x130
    
    (This is split out of the defer global seqno allocation patch due to
    realisation that we need a more complete conversion if we want to defer
    request submission even further.)
    
    v2: lockdep was warning about mixed SOFTIRQ contexts not HARDIRQ
    contexts so we only need to use spin_lock_bh and not disable interrupts.
    
    v3: We need full irq protection as we may be called from a third party
    interrupt handler (via fences).
    Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
    Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
    Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
    Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
    Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
    Link: http://patchwork.freedesktop.org/patch/msgid/20161028125858.23563-32-chris@chris-wilson.co.uk
    f6168e33
intel_breadcrumbs.c 19.5 KB