[Intel-gfx] [PATCH 01/22] drm/i915/gem: Consolidate ctx->engines[] release

[Chris Wilson]

Quoting Mika Kuoppala (2020-03-02 18:29:09)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> > @@ -491,30 +491,104 @@ static void kill_engines(struct i915_gem_engines *engines)
> >  static void kill_stale_engines(struct i915_gem_context *ctx)
> >  {
> >       struct i915_gem_engines *pos, *next;
> > -     unsigned long flags;
> >  
> > -     spin_lock_irqsave(&ctx->stale.lock, flags);
> > +     spin_lock_irq(&ctx->stale.lock);
> 
> Reader ponders why it was with flags on the first place.

Hindsight is 20/20.

> > +     GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> >       list_for_each_entry_safe(pos, next, &ctx->stale.engines, link) {
> > -             if (!i915_sw_fence_await(&pos->fence))
> > +             if (!i915_sw_fence_await(&pos->fence)) {
> > +                     list_del_init(&pos->link);
> 
> Is this for making the bed for kill path list_empty?

It's acknowledging that if we are racing in FENCE_COMPLETE, we don't
need to kill it.

> > +static void engines_idle_release(struct i915_gem_context *ctx,
> > +                              struct i915_gem_engines *engines)
> > +{
> > +     struct i915_gem_engines_iter it;
> > +     struct intel_context *ce;
> > +
> > +     i915_sw_fence_init(&engines->fence, engines_notify);
> > +     INIT_LIST_HEAD(&engines->link);
> > +
> > +     engines->ctx = i915_gem_context_get(ctx);
> > +
> > +     for_each_gem_engine(ce, engines, it) {
> > +             struct dma_fence *fence;
> > +             int err = 0;
> > +
> > +             /* serialises with execbuf */
> > +             RCU_INIT_POINTER(ce->gem_context, NULL);
> > +             if (!intel_context_pin_if_active(ce))
> > +                     continue;
> 
> Ok so this should ensure the lifetime past the execbufs.
> 
> > +
> > +             fence = i915_active_fence_get(&ce->timeline->last_request);

Note it's this fence_get + ce->gem_context that combine to give us the
execbuf closure guard.

> > +             if (fence) {
> > +                     err = i915_sw_fence_await_dma_fence(&engines->fence,
> > +                                                         fence, 0,
> > +                                                         GFP_KERNEL);
> > +                     dma_fence_put(fence);
> > +             }
> > +             intel_context_unpin(ce);
> > +             if (err < 0)
> > +                     goto kill;
> > +     }
-Chris