[Intel-gfx] [PATCH] drm/i915: Allow interrupts when taking the timeline->mutex

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-06-10 12:08:40)
> 
> On 10/06/2019 11:36, Chris Wilson wrote:
> > Before we commit ourselves to writing commands into the
> > ringbuffer and submitting the request, allow signals to interrupt
> > acquisition of the timeline mutex. We allow ourselves to be interrupted
> > at any time later if we need to block for space in the ring, anyway.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_context.h | 5 +++--
> >   drivers/gpu/drm/i915/i915_request.c     | 5 ++++-
> >   2 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> > index 63392c88cd98..6d5453ba2c1e 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_context.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> > @@ -113,10 +113,11 @@ static inline void intel_context_put(struct intel_context *ce)
> >       kref_put(&ce->ref, ce->ops->destroy);
> >   }
> >   
> > -static inline void intel_context_timeline_lock(struct intel_context *ce)
> > +static inline int __must_check
> > +intel_context_timeline_lock(struct intel_context *ce)
> >       __acquires(&ce->ring->timeline->mutex)
> >   {
> > -     mutex_lock(&ce->ring->timeline->mutex);
> > +     return mutex_lock_interruptible(&ce->ring->timeline->mutex);
> >   }
> >   
> >   static inline void intel_context_timeline_unlock(struct intel_context *ce)
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index da1e6984a8cc..e9b59eea4f10 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -784,8 +784,11 @@ struct i915_request *
> >   i915_request_create(struct intel_context *ce)
> >   {
> >       struct i915_request *rq;
> > +     int err;
> >   
> > -     intel_context_timeline_lock(ce);
> > +     err = intel_context_timeline_lock(ce);
> > +     if (err)
> > +             return ERR_PTR(err);
> >   
> >       /* Move our oldest request to the slab-cache (if not in use!) */
> >       rq = list_first_entry(&ce->ring->request_list, typeof(*rq), ring_link);
> > 
> 
> Patch triggered by a bug, issue, report?

None, just a thought occurred to me to check.
 
> We had this conundrum in a few of the other patches which added new 
> mutexes. Sometimes it is less important, but in this case I suppose it 
> is better to handle signals.

There are a few places where we should not, that's the challenge. :)
-Chris