[Intel-gfx] [PATCH] drm/i915: Don't disable interrupts independently of the lock

[Chris Wilson]

Quoting Sebastian Andrzej Siewior (2019-10-10 17:06:40)
> The locks (active.lock and rq->lock) need to be taken with disabled
> interrupts. This is done in i915_request_retire() by disabling the
> interrupts independently of the locks itself.
> While local_irq_disable()+spin_lock() equals spin_lock_irq() on vanilla
> it does not on PREEMPT_RT. Also, it is not obvious if there is a special reason
> to why the interrupts are disabled independently of the lock.
> 
> Enable/disable interrupts as part of the locking instruction.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
> ---
>  drivers/gpu/drm/i915/i915_request.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -251,15 +251,13 @@ static bool i915_request_retire(struct i
>                 active->retire(active, rq);
>         }
>  
> -       local_irq_disable();
> -
>         /*
>          * We only loosely track inflight requests across preemption,
>          * and so we may find ourselves attempting to retire a _completed_
>          * request that we have removed from the HW and put back on a run
>          * queue.
>          */
> -       spin_lock(&rq->engine->active.lock);
> +       spin_lock_irq(&rq->engine->active.lock);
>         list_del(&rq->sched.link);
>         spin_unlock(&rq->engine->active.lock);
>  
> @@ -278,9 +276,7 @@ static bool i915_request_retire(struct i
>                 __notify_execute_cb(rq);
>         }
>         GEM_BUG_ON(!list_empty(&rq->execute_cb));
> -       spin_unlock(&rq->lock);
> -
> -       local_irq_enable();
> +       spin_unlock_irq(&rq->lock);

Nothing screams about the imbalance? irq off from one lock to the other?
-Chris