From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mika Kuoppala Subject: Re: [PATCH] drm/i915: Reorder semaphore deadlock check Date: Fri, 06 Jun 2014 13:04:12 +0300 Message-ID: <871tv2qrqb.fsf@gaia.fi.intel.com> References: <1401963909-27142-1-git-send-email-chris@chris-wilson.co.uk> <1402046549-32522-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: In-Reply-To: <1402046549-32522-1-git-send-email-chris@chris-wilson.co.uk> Sender: stable-owner@vger.kernel.org To: intel-gfx@lists.freedesktop.org Cc: Chris Wilson , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org Chris Wilson writes: > If a semaphore is waiting on another ring, which in turn happens to be > waiting on the first ring, but that second semaphore has been signalled, > we will be able to kick the second ring and so can treat the first ring > as a valid WAIT and not as HUNG. > > v2: Be paranoid and cap the potential recursion depth whilst visiting > the semaphore signallers. (Mika) > > References: https://bugs.freedesktop.org/show_bug.cgi?id=54226 > References: https://bugs.freedesktop.org/show_bug.cgi?id=75502 > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: stable@vger.kernel.org > --- Reviewed-by: Mika Kuoppala > drivers/gpu/drm/i915/i915_irq.c | 18 ++++++++++++++---- > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- > 2 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index a702303eab08..2974698f8f27 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2831,10 +2831,14 @@ static int semaphore_passed(struct intel_engine_cs *ring) > struct intel_engine_cs *signaller; > u32 seqno, ctl; > > - ring->hangcheck.deadlock = true; > + ring->hangcheck.deadlock++; > > signaller = semaphore_waits_for(ring, &seqno); > - if (signaller == NULL || signaller->hangcheck.deadlock) > + if (signaller == NULL) > + return -1; > + > + /* Prevent pathological recursion due to driver bugs */ > + if (signaller->hangcheck.deadlock >= I915_NUM_RINGS) > return -1; > > /* cursory check for an unkickable deadlock */ > @@ -2842,7 +2846,13 @@ static int semaphore_passed(struct intel_engine_cs *ring) > if (ctl & RING_WAIT_SEMAPHORE && semaphore_passed(signaller) < 0) > return -1; > > - return i915_seqno_passed(signaller->get_seqno(signaller, false), seqno); > + if (i915_seqno_passed(signaller->get_seqno(signaller, false), seqno)) > + return 1; > + > + if (signaller->hangcheck.deadlock) > + return -1; > + > + return 0; > } > > static void semaphore_clear_deadlocks(struct drm_i915_private *dev_priv) > @@ -2851,7 +2861,7 @@ static void semaphore_clear_deadlocks(struct drm_i915_private *dev_priv) > int i; > > for_each_ring(ring, dev_priv, i) > - ring->hangcheck.deadlock = false; > + ring->hangcheck.deadlock = 0; > } > > static enum intel_ring_hangcheck_action > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 24357c597b54..31c2aab8ad2c 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -55,7 +55,7 @@ struct intel_ring_hangcheck { > u32 seqno; > int score; > enum intel_ring_hangcheck_action action; > - bool deadlock; > + int deadlock; > }; > > struct intel_ringbuffer { > -- > 2.0.0