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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2019-10-10 19:11:27 [+0100], Chris Wilson wrote:
> > --- 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?

There is no imbalance, is there? Interrupts are disabled as part of
acquiring the first lock and enabled again as part of releasing the
second lock.
It may not look beautiful. 

I'm just not sure if this

|         spin_lock_irq(&rq->engine->active.lock);
|         list_del(&rq->sched.link);
|         spin_unlock_irq(&rq->engine->active.lock);
| 
|         spin_lock_irq(&rq->lock);
|         i915_request_mark_complete(rq);
…
|         spin_unlock_irq(&rq->lock);

has been avoided because an interrupt here could change something or if
this is just an optimisation.

> -Chris

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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux