[Intel-gfx] [PATCH 05/22] drm/i915: Lock the gem_context->active_list while dropping the link

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-03-18 10:39:44)
> 
> On 18/03/2019 09:51, Chris Wilson wrote:
> > On unpinning the intel_context, we remove it from the active list
> > inside the GEM context. This list is supposed to be guarded by the GEM
> > context mutex, so remember to take it!
> 
> Fixes: ?

It is not broken yet :)

> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/intel_context.c         | 15 +++++++++++----
> >   drivers/gpu/drm/i915/intel_lrc.c             |  3 ---
> >   drivers/gpu/drm/i915/intel_ringbuffer.c      |  3 ---
> >   drivers/gpu/drm/i915/selftests/mock_engine.c |  2 --
> >   4 files changed, 11 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_context.c b/drivers/gpu/drm/i915/intel_context.c
> > index 5a16c9bb2778..0ab894a058f6 100644
> > --- a/drivers/gpu/drm/i915/intel_context.c
> > +++ b/drivers/gpu/drm/i915/intel_context.c
> > @@ -165,13 +165,13 @@ intel_context_pin(struct i915_gem_context *ctx,
> >               if (err)
> >                       goto err;
> >   
> > +             i915_gem_context_get(ctx);
> > +             GEM_BUG_ON(ce->gem_context != ctx);
> > +
> >               mutex_lock(&ctx->mutex);
> >               list_add(&ce->active_link, &ctx->active_engines);
> >               mutex_unlock(&ctx->mutex);
> >   
> > -             i915_gem_context_get(ctx);
> > -             GEM_BUG_ON(ce->gem_context != ctx);
> > -
> >               smp_mb__before_atomic(); /* flush pin before it is visible */
> >       }
> >   
> > @@ -194,9 +194,16 @@ void intel_context_unpin(struct intel_context *ce)
> >       /* We may be called from inside intel_context_pin() to evict another */
> >       mutex_lock_nested(&ce->pin_mutex, SINGLE_DEPTH_NESTING);
> 
> Hm is the nested annotation and comment correct?

Yes. pin -> vma bind -> evict -> retire requests -> unpin someone else.
-Chris