[Intel-gfx] [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Tvrtko Ursulin]

On 28/10/16 12:30, Chris Wilson wrote:
> On Fri, Oct 28, 2016 at 12:26:09PM +0100, Chris Wilson wrote:
>> On Fri, Oct 28, 2016 at 11:49:34AM +0100, Tvrtko Ursulin wrote:
>>>
>>>
>>> On 28/10/16 11:42, Chris Wilson wrote:
>>>> On Fri, Oct 28, 2016 at 11:27:43AM +0100, Tvrtko Ursulin wrote:
>>>>>
>>>>>
>>>>> On 28/10/16 11:10, Chris Wilson wrote:
>>>>>> On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 27/10/16 17:10, Chris Wilson wrote:
>>>>>>>> The breadcrumbs are about to be used from within IRQ context sections,
>>>>>>>> therefore we need to employ the irqsafe spinlock variants.
>>>>>>>>
>>>>>>>> (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.)
>>>>>>
>>>>>> [snip]
>>>>>>
>>>>>>> Assuming I got the above right and you agree to change it:
>>>>>>
>>>>>> You made me go and reduce them to _bh as appropriate anyway!!!
>>>>>
>>>>> Hm, but can't enable signalling be called with irqs disabled when
>>>>> fences are exported?
>>>>
>>>> Yes, but that supercedes the spin_lock_bh, so we can just call
>>>> spin_lock() in enabling_signaling as we can assert that we will always
>>>> be called with irqs disabled here (due to spin_lock_irqsafe(fence->lock)
>>>> in the callpath).
>>>
>>> But as long as the b->lock is taken in the irqs disabled section
>>> somewhere, other callers like signaller thread, debugfs, etc, cannot
>>> just take it with _bh.
>>
>> Lockdep doesn't complain, if we take spin_lock(b->lock) under irq inside our
>> tasklet (enable_signaling)  so long as we use spin_lock_bh() elsewhere.
>
> The key part is that we never take the spin_lock in irq context, just I
> am introducing it into softirq context. Hence why, I think, lockdep is
> happy.

Hm, but when we export it external fence user could use it from their 
irq context? We export to userspace, some other driver imports and 
enables signalling from irq?

Regards,

Tvrtko