* [PATCH] drm/i915: Reorder semaphore deadlock check
@ 2014-06-05 10:25 Chris Wilson
2014-06-06 9:22 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2014-06-05 10:25 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson, stable
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.
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 <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a702303eab08..41eeeb60c60f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2834,7 +2834,7 @@ static int semaphore_passed(struct intel_engine_cs *ring)
ring->hangcheck.deadlock = true;
signaller = semaphore_waits_for(ring, &seqno);
- if (signaller == NULL || signaller->hangcheck.deadlock)
+ if (signaller == NULL)
return -1;
/* cursory check for an unkickable deadlock */
@@ -2842,7 +2842,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)
--
2.0.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] drm/i915: Reorder semaphore deadlock check
2014-06-05 10:25 [PATCH] drm/i915: Reorder semaphore deadlock check Chris Wilson
@ 2014-06-06 9:22 ` Chris Wilson
2014-06-06 10:04 ` Mika Kuoppala
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2014-06-06 9:22 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson, Mika Kuoppala, stable
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 <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: stable@vger.kernel.org
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Reorder semaphore deadlock check
2014-06-06 9:22 ` Chris Wilson
@ 2014-06-06 10:04 ` Mika Kuoppala
2014-06-10 16:36 ` [Intel-gfx] " Jani Nikula
0 siblings, 1 reply; 4+ messages in thread
From: Mika Kuoppala @ 2014-06-06 10:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson, stable
Chris Wilson <chris@chris-wilson.co.uk> 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 <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Reorder semaphore deadlock check
2014-06-06 10:04 ` Mika Kuoppala
@ 2014-06-10 16:36 ` Jani Nikula
0 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2014-06-10 16:36 UTC (permalink / raw)
To: Mika Kuoppala, Chris Wilson, intel-gfx; +Cc: stable
On Fri, 06 Jun 2014, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Chris Wilson <chris@chris-wilson.co.uk> 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 <chris@chris-wilson.co.uk>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Cc: stable@vger.kernel.org
>> ---
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Pushed to -fixes, thanks for the patch and review.
BR,
Jani.
>
>> 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-06-10 16:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-05 10:25 [PATCH] drm/i915: Reorder semaphore deadlock check Chris Wilson
2014-06-06 9:22 ` Chris Wilson
2014-06-06 10:04 ` Mika Kuoppala
2014-06-10 16:36 ` [Intel-gfx] " Jani Nikula
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.