On 15/03/2017 18:32, Chris Wilson wrote:
On Wed, Mar 15, 2017 at 06:20:16PM +0000, Tvrtko Ursulin wrote:

On 15/03/2017 14:01, Chris Wilson wrote:
Check that we have disabled irqs before we take the spin_lock around
reassigned the breadcrumbs.irq_wait.

Signed-off-by: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c 
b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 3f222dee4c25..35529b35a276 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -301,8 +301,11 @@ static inline void __intel_breadcrumbs_next(struct 
intel_engine_cs *engine,
{
        struct intel_breadcrumbs *b = &engine->breadcrumbs;

+       GEM_BUG_ON(!irqs_disabled());
+
        spin_lock(&b->irq_lock);
        GEM_BUG_ON(!b->irq_armed);
+       GEM_BUG_ON(!b->irq_wait);
        b->irq_wait = to_wait(next);
        spin_unlock(&b->irq_lock);

@@ -395,8 +398,10 @@ static bool __intel_engine_add_wait(struct intel_engine_cs 
*engine,
        }

        if (first) {
-               spin_lock(&b->irq_lock);
                GEM_BUG_ON(rb_first(&b->waiters) != &wait->node);
+               GEM_BUG_ON(!irqs_disabled());
+
+               spin_lock(&b->irq_lock);
                b->irq_wait = wait;
                /* After assigning ourselves as the new bottom-half, we must
                 * perform a cursory check to prevent a missed interrupt.


A single GEM_BUG_ON(!irqs_disabled()) at the top of
__intel_engine_add_wait might be more logical?

I wanted to associate it with b->irq_lock, that was my thinking.
b->rb_lock also sadly has to be irqsafe.

That makes sense yes.

__intel_breadcrumbs_next() also serves remove_wait, did you mean to
remove the assert there as well?

Yes, I was thinking only one would do it.

We can safely ignore this patch, it should be catered by lockdep fairly
well, I was just being paranoid and going through the possible causes
and documenting my progress.

This also makes sense. I think it's the best option.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to