| Message ID | 20200318121138.909-5-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Per client engine busyness | expand |
Quoting Tvrtko Ursulin (2020-03-18 12:11:33) > From: Tvrtko Ursulin <tvrtko.ursulin@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@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? :) Reviewed-by: Chris Wilson <chris@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. -Chris
On 18/03/2020 13:49, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2020-03-18 12:11:33) >> From: Tvrtko Ursulin <tvrtko.ursulin@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@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@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
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 u32 *lrc_reg_state; u64 lrc_desc;