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

[Tvrtko Ursulin]

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.

Regards,

Tvrtko