[Intel-gfx] [PATCH 04/10] drm/i915: Use explicit flag to mark unreachable intel_context

[Tvrtko Ursulin]

On 18/03/2020 13:49, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-03-18 12:11:33)
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> I need to keep the GEM context around a bit longer so adding an explicit
>> flag for syncing execbuf with closed/abandonded contexts.
>>
>> v2:
>>   * Use already available context flags. (Chris)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_context.c    | 3 ++-
>>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
>>   drivers/gpu/drm/i915/gt/intel_context_types.h  | 1 +
>>   3 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> index 9afc60ab95e0..7c119a3a2cbd 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> @@ -579,7 +579,8 @@ static void engines_idle_release(struct i915_gem_context *ctx,
>>                  int err = 0;
>>   
>>                  /* serialises with execbuf */
>> -               RCU_INIT_POINTER(ce->gem_context, NULL);
>> +               set_bit(INTEL_CONTEXT_CLOSED, &ce->flags);
>> +
>>                  if (!intel_context_pin_if_active(ce))
>>                          continue;
>>   
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> index d3f4f28e9468..875da020d6c8 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> @@ -2316,7 +2316,7 @@ static void eb_request_add(struct i915_execbuffer *eb)
>>          prev = __i915_request_commit(rq);
>>   
>>          /* Check that the context wasn't destroyed before submission */
>> -       if (likely(rcu_access_pointer(eb->context->gem_context))) {
>> +       if (likely(!test_bit(INTEL_CONTEXT_CLOSED, &eb->context->flags))) {
>>                  attr = eb->gem_context->sched;
>>   
>>                  /*
>> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
>> index 0f3b68b95c56..d5925c25f109 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
>> @@ -66,6 +66,7 @@ struct intel_context {
>>   #define CONTEXT_BANNED                 4
>>   #define CONTEXT_FORCE_SINGLE_SUBMISSION        5
>>   #define CONTEXT_NOPREEMPT              6
>> +#define INTEL_CONTEXT_CLOSED           7
> 
> Trying to start a flame war? :)

No, but CONTEXT_ namespace is overloaded between here and GEM context. I 
propose we prefix one of them all with something.

> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> 
> With this flag, we can start banning contexts after a GPU hang on a
> closed context _again_. That might justify applying immediately.

Hm okay.

Regards,

Tvrtko