| Message ID | 20180625094842.8499-13-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Rather than have multiple locked instructions inside the notify_ring() > irq handler, move them inside the spinlock and reduce their intrinsic > locking. > Less is better, could note in commit message that we omit the non wait ones. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_irq.c | 4 ++-- > drivers/gpu/drm/i915/i915_request.c | 4 ++-- > drivers/gpu/drm/i915/intel_breadcrumbs.c | 11 +++++++---- > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- > 4 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 53dad48f92ce..6730c1a7f135 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1153,8 +1153,6 @@ static void notify_ring(struct intel_engine_cs *engine) > if (unlikely(!engine->breadcrumbs.irq_armed)) > return; > > - atomic_inc(&engine->irq_count); > - > rcu_read_lock(); > > spin_lock(&engine->breadcrumbs.irq_lock); > @@ -1189,6 +1187,8 @@ static void notify_ring(struct intel_engine_cs *engine) > tsk = wait->tsk; > } > } > + > + engine->breadcrumbs.irq_count++; > } else { > if (engine->breadcrumbs.irq_armed) > __intel_engine_disarm_breadcrumbs(engine); > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 11f175554da8..696125663105 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1161,7 +1161,7 @@ static bool __i915_spin_request(const struct i915_request *rq, > * takes to sleep on a request, on the order of a microsecond. > */ > > - irq = atomic_read(&engine->irq_count); > + irq = READ_ONCE(engine->breadcrumbs.irq_count); > timeout_us += local_clock_us(&cpu); > do { > if (i915_seqno_passed(intel_engine_get_seqno(engine), seqno)) > @@ -1173,7 +1173,7 @@ static bool __i915_spin_request(const struct i915_request *rq, > * assume we won't see one in the near future but require > * the engine->seqno_barrier() to fixup coherency. > */ > - if (atomic_read(&engine->irq_count) != irq) > + if (READ_ONCE(engine->breadcrumbs.irq_count) != irq) > break; > > if (signal_pending_state(state, current)) > diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c > index 86a987b8ac66..1db6ba7d926e 100644 > --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c > +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c > @@ -98,12 +98,14 @@ static void intel_breadcrumbs_hangcheck(struct timer_list *t) > struct intel_engine_cs *engine = > from_timer(engine, t, breadcrumbs.hangcheck); > struct intel_breadcrumbs *b = &engine->breadcrumbs; > + unsigned int irq_count; > > if (!b->irq_armed) > return; > > - if (b->hangcheck_interrupts != atomic_read(&engine->irq_count)) { > - b->hangcheck_interrupts = atomic_read(&engine->irq_count); > + irq_count = READ_ONCE(b->irq_count); > + if (b->hangcheck_interrupts != irq_count) { > + b->hangcheck_interrupts = irq_count; > mod_timer(&b->hangcheck, wait_timeout()); > return; > } > @@ -272,13 +274,14 @@ static bool use_fake_irq(const struct intel_breadcrumbs *b) > if (!test_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings)) > return false; > > - /* Only start with the heavy weight fake irq timer if we have not > + /* > + * Only start with the heavy weight fake irq timer if we have not > * seen any interrupts since enabling it the first time. If the > * interrupts are still arriving, it means we made a mistake in our > * engine->seqno_barrier(), a timing error that should be transient > * and unlikely to reoccur. > */ > - return atomic_read(&engine->irq_count) == b->hangcheck_interrupts; > + return READ_ONCE(b->irq_count) == b->hangcheck_interrupts; > } > > static void enable_fake_irq(struct intel_breadcrumbs *b) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 8dd34b9dc18a..33602eb1c77f 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -356,7 +356,6 @@ struct intel_engine_cs { > struct drm_i915_gem_object *default_state; > void *pinned_default_state; > > - atomic_t irq_count; > unsigned long irq_posted; > #define ENGINE_IRQ_BREADCRUMB 0 > > @@ -390,6 +389,7 @@ struct intel_engine_cs { > > unsigned int hangcheck_interrupts; > unsigned int irq_enabled; > + unsigned int irq_count; > > bool irq_armed : 1; > I915_SELFTEST_DECLARE(bool mock : 1); > -- > 2.18.0
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 53dad48f92ce..6730c1a7f135 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1153,8 +1153,6 @@ static void notify_ring(struct intel_engine_cs *engine) if (unlikely(!engine->breadcrumbs.irq_armed)) return; - atomic_inc(&engine->irq_count); - rcu_read_lock(); spin_lock(&engine->breadcrumbs.irq_lock); @@ -1189,6 +1187,8 @@ static void notify_ring(struct intel_engine_cs *engine) tsk = wait->tsk; } } + + engine->breadcrumbs.irq_count++; } else { if (engine->breadcrumbs.irq_armed) __intel_engine_disarm_breadcrumbs(engine); diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 11f175554da8..696125663105 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1161,7 +1161,7 @@ static bool __i915_spin_request(const struct i915_request *rq, * takes to sleep on a request, on the order of a microsecond. */ - irq = atomic_read(&engine->irq_count); + irq = READ_ONCE(engine->breadcrumbs.irq_count); timeout_us += local_clock_us(&cpu); do { if (i915_seqno_passed(intel_engine_get_seqno(engine), seqno)) @@ -1173,7 +1173,7 @@ static bool __i915_spin_request(const struct i915_request *rq, * assume we won't see one in the near future but require * the engine->seqno_barrier() to fixup coherency. */ - if (atomic_read(&engine->irq_count) != irq) + if (READ_ONCE(engine->breadcrumbs.irq_count) != irq) break; if (signal_pending_state(state, current)) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 86a987b8ac66..1db6ba7d926e 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -98,12 +98,14 @@ static void intel_breadcrumbs_hangcheck(struct timer_list *t) struct intel_engine_cs *engine = from_timer(engine, t, breadcrumbs.hangcheck); struct intel_breadcrumbs *b = &engine->breadcrumbs; + unsigned int irq_count; if (!b->irq_armed) return; - if (b->hangcheck_interrupts != atomic_read(&engine->irq_count)) { - b->hangcheck_interrupts = atomic_read(&engine->irq_count); + irq_count = READ_ONCE(b->irq_count); + if (b->hangcheck_interrupts != irq_count) { + b->hangcheck_interrupts = irq_count; mod_timer(&b->hangcheck, wait_timeout()); return; } @@ -272,13 +274,14 @@ static bool use_fake_irq(const struct intel_breadcrumbs *b) if (!test_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings)) return false; - /* Only start with the heavy weight fake irq timer if we have not + /* + * Only start with the heavy weight fake irq timer if we have not * seen any interrupts since enabling it the first time. If the * interrupts are still arriving, it means we made a mistake in our * engine->seqno_barrier(), a timing error that should be transient * and unlikely to reoccur. */ - return atomic_read(&engine->irq_count) == b->hangcheck_interrupts; + return READ_ONCE(b->irq_count) == b->hangcheck_interrupts; } static void enable_fake_irq(struct intel_breadcrumbs *b) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 8dd34b9dc18a..33602eb1c77f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -356,7 +356,6 @@ struct intel_engine_cs { struct drm_i915_gem_object *default_state; void *pinned_default_state; - atomic_t irq_count; unsigned long irq_posted; #define ENGINE_IRQ_BREADCRUMB 0 @@ -390,6 +389,7 @@ struct intel_engine_cs { unsigned int hangcheck_interrupts; unsigned int irq_enabled; + unsigned int irq_count; bool irq_armed : 1; I915_SELFTEST_DECLARE(bool mock : 1);
Rather than have multiple locked instructions inside the notify_ring() irq handler, move them inside the spinlock and reduce their intrinsic locking. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/i915/i915_request.c | 4 ++-- drivers/gpu/drm/i915/intel_breadcrumbs.c | 11 +++++++---- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 4 files changed, 12 insertions(+), 9 deletions(-)