Re: [PATCH 06/18] drm/i915/breadcrumbs: Keep the fake irq armed across reset

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2018-04-23 14:03:02)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> > @@ -148,6 +149,12 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t)
>> >       if (!b->irq_armed)
>> >               return;
>> >  
>> > +     /* If the user has disabled the fake-irq, restore the hangchecking */
>> > +     if (!test_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings)) {
>> > +             mod_timer(&b->hangcheck, wait_timeout());
>> > +             return;
>> > +     }
>> > +
>> 
>> Looking at the cancel_fake_irq() now, which we still need to keep as
>> sync, I think there is race introduce now as this can queue itself.
>> 
>> I think we want to also change the cancel_fake_irq() to do
>> the bit clearing first, not last after the del_timer_syncs().
>
> I see what you mean, but I think we want
>
> 	del_timer_sync(&b->fake_irq); // may queue b->hangcheck
> 	del_timer_sync(&b->hangcheck);
> 	clear_bit(engine->id. missed_irq_rings);
>

Ok got it. Swapping the order makes the newly
queued hangcheck from fake_irq to be cancelled and
the clearing should be last.

But now as the cancel is done only in fini path,
consider adding assertion for !irq_armed in start of
cancel_fake_irq().

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx