[Intel-gfx] [PATCH 31/38] drm/i915: Track the pinned kernel contexts on each engine

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-03-05 18:17:35)
> 
> On 05/03/2019 18:10, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-03-05 18:07:39)
> >>>        /*
> >>>         * Similarly the preempt context must always be available so that
> >>> -      * we can interrupt the engine at any time.
> >>> +      * we can interrupt the engine at any time. However, as preemption
> >>> +      * is optional, we allow it to fail.
> >>>         */
> >>> -     if (i915->preempt_context) {
> >>> -             ce = intel_context_pin(i915->preempt_context, engine);
> >>> -             if (IS_ERR(ce)) {
> >>> -                     ret = PTR_ERR(ce);
> >>> -                     goto err_unpin_kernel;
> >>> -             }
> >>> -     }
> >>> +     if (i915->preempt_context)
> >>> +             pin_context(i915->preempt_context, engine,
> >>> +                         &engine->preempt_context);
> >>
> >> You lost the failure path here. I suspect deliberately? But I am not
> >> convinced we want to silently lose preemption when keeping the failure
> >> path is so easy.
> > 
> > The failure path kills the module. Whereas we can quite happily survive
> > without preemption.
> 
> Yes it is hard to decide what is worse, modprobe failure which never 
> happens, or change in performance profile which also never happens. :)
> 
> For something so unlikely I'd rather see it fail than silently change 
> behaviour. Perhaps it has some relevance during development and platform 
> bringup if nowhere else.

I err on the opposite side, keep going at all costs so that the user
isn't scaring at a blank screen of doom. It's not exactly a silent
failure, we tell anyone who asks that preemption is enabled :)
-Chris