* [RESEND 2/6] drm/i915/selftests: Exercise reset to break stuck GTT eviction
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
@ 2018-07-16 8:03 ` Chris Wilson
2018-07-16 9:32 ` Tvrtko Ursulin
2018-07-16 8:03 ` [RESEND 3/6] drm/i915/execlists: Always clear preempt status on cancelling all Chris Wilson
` (7 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2018-07-16 8:03 UTC (permalink / raw)
To: intel-gfx
We must be able to reset the GPU while we are waiting on it to perform
an eviction (unbinding an active vma). So attach a spinning request to a
target vma and try and it evict it from a thread to see if that blocks
indefinitely.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../gpu/drm/i915/selftests/intel_hangcheck.c | 153 +++++++++++++++++-
1 file changed, 151 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index 73462a65a330..344973664206 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -27,6 +27,7 @@
#include "../i915_selftest.h"
#include "i915_random.h"
#include "igt_flush_test.h"
+#include "igt_wedge_me.h"
#include "mock_context.h"
#include "mock_drm.h"
@@ -921,7 +922,7 @@ static u32 fake_hangcheck(struct i915_request *rq, u32 mask)
return reset_count;
}
-static int igt_wait_reset(void *arg)
+static int igt_reset_wait(void *arg)
{
struct drm_i915_private *i915 = arg;
struct i915_request *rq;
@@ -995,6 +996,152 @@ static int igt_wait_reset(void *arg)
return err;
}
+static int evict_vma(void *data)
+{
+ struct i915_vma *vma = data;
+ struct drm_i915_private *i915 = vma->vm->i915;
+ struct drm_mm_node evict = vma->node;
+ int err;
+
+ mutex_lock(&i915->drm.struct_mutex);
+ err = i915_gem_evict_for_node(vma->vm, &evict, 0);
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ return err;
+}
+
+static int __igt_reset_evict_vma(struct drm_i915_private *i915,
+ struct i915_address_space *vm)
+{
+ struct drm_i915_gem_object *obj;
+ struct task_struct *tsk = NULL;
+ struct i915_request *rq;
+ struct i915_vma *vma;
+ struct hang h;
+ int err;
+
+ if (!intel_engine_can_store_dword(i915->engine[RCS]))
+ return 0;
+
+ /* Check that we can recover an unbind stuck on a hanging request */
+
+ global_reset_lock(i915);
+
+ mutex_lock(&i915->drm.struct_mutex);
+ err = hang_init(&h, i915);
+ if (err)
+ goto unlock;
+
+ obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ goto fini;
+ }
+
+ vma = i915_vma_instance(obj, vm, NULL);
+ if (IS_ERR(vma)) {
+ err = PTR_ERR(vma);
+ goto out_obj;
+ }
+
+ rq = hang_create_request(&h, i915->engine[RCS]);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto out_obj;
+ }
+
+ err = i915_vma_pin(vma, 0, 0,
+ i915_vma_is_ggtt(vma) ? PIN_GLOBAL : PIN_USER);
+ if (err)
+ goto out_obj;
+
+ err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
+ i915_vma_unpin(vma);
+
+ i915_request_get(rq);
+ i915_request_add(rq);
+ if (err)
+ goto out_rq;
+
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ if (!wait_until_running(&h, rq)) {
+ struct drm_printer p = drm_info_printer(i915->drm.dev);
+
+ pr_err("%s: Failed to start request %x, at %x\n",
+ __func__, rq->fence.seqno, hws_seqno(&h, rq));
+ intel_engine_dump(rq->engine, &p, "%s\n", rq->engine->name);
+
+ i915_gem_set_wedged(i915);
+ goto out_reset;
+ }
+
+ tsk = kthread_run(evict_vma, vma, "igt/evict_vma");
+
+ if (wait_for(waitqueue_active(&rq->execute), 10)) {
+ struct drm_printer p = drm_info_printer(i915->drm.dev);
+
+ pr_err("igt/evict_vma kthread did not wait\n");
+ intel_engine_dump(rq->engine, &p, "%s\n", rq->engine->name);
+
+ i915_gem_set_wedged(i915);
+ goto out_reset;
+ }
+
+out_reset:
+ fake_hangcheck(rq, intel_engine_flag(rq->engine));
+
+ if (tsk) {
+ struct igt_wedge_me w;
+
+ igt_wedge_on_timeout(&w, i915, HZ / 5)
+ err = kthread_stop(tsk);
+ }
+
+ mutex_lock(&i915->drm.struct_mutex);
+out_rq:
+ i915_request_put(rq);
+out_obj:
+ i915_gem_object_put(obj);
+fini:
+ hang_fini(&h);
+unlock:
+ mutex_unlock(&i915->drm.struct_mutex);
+ global_reset_unlock(i915);
+
+ if (i915_terminally_wedged(&i915->gpu_error))
+ return -EIO;
+
+ return err;
+}
+
+static int igt_reset_evict_ggtt(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+
+ return __igt_reset_evict_vma(i915, &i915->ggtt.vm);
+}
+
+static int igt_reset_evict_ppgtt(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct i915_gem_context *ctx;
+ int err;
+
+ mutex_lock(&i915->drm.struct_mutex);
+ ctx = kernel_context(i915);
+ mutex_unlock(&i915->drm.struct_mutex);
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+
+ err = 0;
+ if (ctx->ppgtt)
+ err = __igt_reset_evict_vma(i915, &ctx->ppgtt->vm);
+
+ kernel_context_close(ctx);
+ return err;
+}
+
static int wait_for_others(struct drm_i915_private *i915,
struct intel_engine_cs *exclude)
{
@@ -1240,8 +1387,10 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
SUBTEST(igt_reset_idle_engine),
SUBTEST(igt_reset_active_engine),
SUBTEST(igt_reset_engines),
- SUBTEST(igt_wait_reset),
SUBTEST(igt_reset_queue),
+ SUBTEST(igt_reset_wait),
+ SUBTEST(igt_reset_evict_ggtt),
+ SUBTEST(igt_reset_evict_ppgtt),
SUBTEST(igt_handle_error),
};
bool saved_hangcheck;
--
2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RESEND 2/6] drm/i915/selftests: Exercise reset to break stuck GTT eviction
2018-07-16 8:03 ` [RESEND 2/6] drm/i915/selftests: Exercise reset to break stuck GTT eviction Chris Wilson
@ 2018-07-16 9:32 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-07-16 9:32 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 16/07/2018 09:03, Chris Wilson wrote:
> We must be able to reset the GPU while we are waiting on it to perform
> an eviction (unbinding an active vma). So attach a spinning request to a
> target vma and try and it evict it from a thread to see if that blocks
> indefinitely.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> .../gpu/drm/i915/selftests/intel_hangcheck.c | 153 +++++++++++++++++-
> 1 file changed, 151 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> index 73462a65a330..344973664206 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> @@ -27,6 +27,7 @@
> #include "../i915_selftest.h"
> #include "i915_random.h"
> #include "igt_flush_test.h"
> +#include "igt_wedge_me.h"
>
> #include "mock_context.h"
> #include "mock_drm.h"
> @@ -921,7 +922,7 @@ static u32 fake_hangcheck(struct i915_request *rq, u32 mask)
> return reset_count;
> }
>
> -static int igt_wait_reset(void *arg)
> +static int igt_reset_wait(void *arg)
> {
> struct drm_i915_private *i915 = arg;
> struct i915_request *rq;
> @@ -995,6 +996,152 @@ static int igt_wait_reset(void *arg)
> return err;
> }
>
> +static int evict_vma(void *data)
> +{
> + struct i915_vma *vma = data;
> + struct drm_i915_private *i915 = vma->vm->i915;
> + struct drm_mm_node evict = vma->node;
> + int err;
> +
> + mutex_lock(&i915->drm.struct_mutex);
> + err = i915_gem_evict_for_node(vma->vm, &evict, 0);
> + mutex_unlock(&i915->drm.struct_mutex);
> +
> + return err;
> +}
> +
> +static int __igt_reset_evict_vma(struct drm_i915_private *i915,
> + struct i915_address_space *vm)
> +{
> + struct drm_i915_gem_object *obj;
> + struct task_struct *tsk = NULL;
> + struct i915_request *rq;
> + struct i915_vma *vma;
> + struct hang h;
> + int err;
> +
> + if (!intel_engine_can_store_dword(i915->engine[RCS]))
> + return 0;
> +
> + /* Check that we can recover an unbind stuck on a hanging request */
> +
> + global_reset_lock(i915);
> +
> + mutex_lock(&i915->drm.struct_mutex);
> + err = hang_init(&h, i915);
> + if (err)
> + goto unlock;
> +
> + obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
> + if (IS_ERR(obj)) {
> + err = PTR_ERR(obj);
> + goto fini;
> + }
> +
> + vma = i915_vma_instance(obj, vm, NULL);
> + if (IS_ERR(vma)) {
> + err = PTR_ERR(vma);
> + goto out_obj;
> + }
> +
> + rq = hang_create_request(&h, i915->engine[RCS]);
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + goto out_obj;
> + }
> +
> + err = i915_vma_pin(vma, 0, 0,
> + i915_vma_is_ggtt(vma) ? PIN_GLOBAL : PIN_USER);
> + if (err)
> + goto out_obj;
> +
> + err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
> + i915_vma_unpin(vma);
> +
> + i915_request_get(rq);
> + i915_request_add(rq);
> + if (err)
> + goto out_rq;
> +
> + mutex_unlock(&i915->drm.struct_mutex);
> +
> + if (!wait_until_running(&h, rq)) {
> + struct drm_printer p = drm_info_printer(i915->drm.dev);
> +
> + pr_err("%s: Failed to start request %x, at %x\n",
> + __func__, rq->fence.seqno, hws_seqno(&h, rq));
> + intel_engine_dump(rq->engine, &p, "%s\n", rq->engine->name);
> +
> + i915_gem_set_wedged(i915);
> + goto out_reset;
> + }
> +
> + tsk = kthread_run(evict_vma, vma, "igt/evict_vma");
> +
> + if (wait_for(waitqueue_active(&rq->execute), 10)) {
So this is 10ms for the thread to start and start waiting. I am trying
to judge if it will be stable enough.. And since it cannot distinguish
between the failure of thread to start and get to the bit where it is
supposed to wait, versus actually not waiting.
Maybe set a flag just before i915_gem_evict_for_node and wait on that first?
And there is a comment against waitqueue_active that warns about races
but I think it is fine in this case.
> + struct drm_printer p = drm_info_printer(i915->drm.dev);
> +
> + pr_err("igt/evict_vma kthread did not wait\n");
> + intel_engine_dump(rq->engine, &p, "%s\n", rq->engine->name);
> +
> + i915_gem_set_wedged(i915);
> + goto out_reset;
> + }
> +
> +out_reset:
> + fake_hangcheck(rq, intel_engine_flag(rq->engine));
> +
> + if (tsk) {
> + struct igt_wedge_me w;
> +
> + igt_wedge_on_timeout(&w, i915, HZ / 5)
> + err = kthread_stop(tsk);
This means 200ms grace for reset to terminate the spinner and unblock
the eviction thread? Is that enough on all platforms?
> + }
> +
> + mutex_lock(&i915->drm.struct_mutex);
> +out_rq:
> + i915_request_put(rq);
> +out_obj:
> + i915_gem_object_put(obj);
> +fini:
> + hang_fini(&h);
> +unlock:
> + mutex_unlock(&i915->drm.struct_mutex);
> + global_reset_unlock(i915);
> +
> + if (i915_terminally_wedged(&i915->gpu_error))
> + return -EIO;
> +
> + return err;
> +}
> +
> +static int igt_reset_evict_ggtt(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> +
> + return __igt_reset_evict_vma(i915, &i915->ggtt.vm);
> +}
> +
> +static int igt_reset_evict_ppgtt(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct i915_gem_context *ctx;
> + int err;
> +
> + mutex_lock(&i915->drm.struct_mutex);
> + ctx = kernel_context(i915);
> + mutex_unlock(&i915->drm.struct_mutex);
> + if (IS_ERR(ctx))
> + return PTR_ERR(ctx);
> +
> + err = 0;
> + if (ctx->ppgtt)
Just skip the test when !USES_FULL_PPGTT?
> + err = __igt_reset_evict_vma(i915, &ctx->ppgtt->vm);
> +
> + kernel_context_close(ctx);
> + return err;
> +}
> +
> static int wait_for_others(struct drm_i915_private *i915,
> struct intel_engine_cs *exclude)
> {
> @@ -1240,8 +1387,10 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
> SUBTEST(igt_reset_idle_engine),
> SUBTEST(igt_reset_active_engine),
> SUBTEST(igt_reset_engines),
> - SUBTEST(igt_wait_reset),
> SUBTEST(igt_reset_queue),
> + SUBTEST(igt_reset_wait),
> + SUBTEST(igt_reset_evict_ggtt),
> + SUBTEST(igt_reset_evict_ppgtt),
> SUBTEST(igt_handle_error),
> };
> bool saved_hangcheck;
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RESEND 3/6] drm/i915/execlists: Always clear preempt status on cancelling all
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
2018-07-16 8:03 ` [RESEND 2/6] drm/i915/selftests: Exercise reset to break stuck GTT eviction Chris Wilson
@ 2018-07-16 8:03 ` Chris Wilson
2018-07-16 9:45 ` Tvrtko Ursulin
2018-07-16 8:03 ` [RESEND 4/6] drm/i915/execlists: Disable submission tasklet upon wedging Chris Wilson
` (6 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2018-07-16 8:03 UTC (permalink / raw)
To: intel-gfx
On reset/wedging, we cancel all pending replies from the HW and we also
want to cancel an outstanding preemption event. Since we use the same
function to cancel the pending replies for reset and for a preemption
event, we can simply clear the active tracking for all.
v2: Keep execlists_user_end() markup for wedging
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_guc_submission.c | 2 --
drivers/gpu/drm/i915/intel_lrc.c | 5 ++---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index de57cf6085d1..3290d77b0194 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -636,8 +636,6 @@ static void complete_preempt_context(struct intel_engine_cs *engine)
wait_for_guc_preempt_report(engine);
intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0);
-
- execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 703b76dcfcd2..4ef4439ff438 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -566,8 +566,6 @@ static void complete_preempt_context(struct intel_engine_execlists *execlists)
__unwind_incomplete_requests(container_of(execlists,
struct intel_engine_cs,
execlists));
-
- execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT);
}
static void execlists_dequeue(struct intel_engine_cs *engine)
@@ -795,7 +793,7 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
port++;
}
- execlists_user_end(execlists);
+ execlists->active = 0;
}
static void reset_csb_pointers(struct intel_engine_execlists *execlists)
@@ -841,6 +839,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
/* Cancel the requests on the HW and clear the ELSP tracker. */
execlists_cancel_port_requests(execlists);
+ execlists_user_end(execlists);
/* Mark all executing requests as skipped. */
list_for_each_entry(rq, &engine->timeline.requests, link) {
--
2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RESEND 3/6] drm/i915/execlists: Always clear preempt status on cancelling all
2018-07-16 8:03 ` [RESEND 3/6] drm/i915/execlists: Always clear preempt status on cancelling all Chris Wilson
@ 2018-07-16 9:45 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-07-16 9:45 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 16/07/2018 09:03, Chris Wilson wrote:
> On reset/wedging, we cancel all pending replies from the HW and we also
> want to cancel an outstanding preemption event. Since we use the same
> function to cancel the pending replies for reset and for a preemption
> event, we can simply clear the active tracking for all.
>
> v2: Keep execlists_user_end() markup for wedging
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Presumably there is some commit to cite as being fixed with this?
> ---
> drivers/gpu/drm/i915/intel_guc_submission.c | 2 --
> drivers/gpu/drm/i915/intel_lrc.c | 5 ++---
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index de57cf6085d1..3290d77b0194 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -636,8 +636,6 @@ static void complete_preempt_context(struct intel_engine_cs *engine)
>
> wait_for_guc_preempt_report(engine);
> intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0);
> -
> - execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 703b76dcfcd2..4ef4439ff438 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -566,8 +566,6 @@ static void complete_preempt_context(struct intel_engine_execlists *execlists)
> __unwind_incomplete_requests(container_of(execlists,
> struct intel_engine_cs,
> execlists));
> -
> - execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT);
> }
>
> static void execlists_dequeue(struct intel_engine_cs *engine)
> @@ -795,7 +793,7 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
> port++;
> }
>
> - execlists_user_end(execlists);
> + execlists->active = 0;
We end up with a mish-mash of direct access and helpers to
execlists->active. :(
Do two helpers in a row result in optimized code?
> }
>
> static void reset_csb_pointers(struct intel_engine_execlists *execlists)
> @@ -841,6 +839,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
>
> /* Cancel the requests on the HW and clear the ELSP tracker. */
> execlists_cancel_port_requests(execlists);
> + execlists_user_end(execlists);
And here execlists_cancel_port_requests already zeroed the whole field.
>
> /* Mark all executing requests as skipped. */
> list_for_each_entry(rq, &engine->timeline.requests, link) {
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RESEND 4/6] drm/i915/execlists: Disable submission tasklet upon wedging
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
2018-07-16 8:03 ` [RESEND 2/6] drm/i915/selftests: Exercise reset to break stuck GTT eviction Chris Wilson
2018-07-16 8:03 ` [RESEND 3/6] drm/i915/execlists: Always clear preempt status on cancelling all Chris Wilson
@ 2018-07-16 8:03 ` Chris Wilson
2018-07-16 9:59 ` Tvrtko Ursulin
2018-07-16 8:03 ` [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private Chris Wilson
` (5 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2018-07-16 8:03 UTC (permalink / raw)
To: intel-gfx
If we declare the driver wedged before the GPU truly is, then we may see
the GPU complete some CS events following our cancellation. This leaves
us quite confused as we deleted all the bookkeeping and thus complain
about the inconsistent state.
We can just ignore the remaining events and let the GPU idle by not
feeding it, and so avoid trying to racily overwrite shared state. We
rely on there being a full GPU reset before unwedging, giving us the
opportunity to reset the shared state.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107188
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_lrc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4ef4439ff438..2c0050bab71e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -811,6 +811,11 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
WRITE_ONCE(*execlists->csb_write, execlists->csb_write_reset);
}
+static void nop_submission_tasklet(unsigned long data)
+{
+ /* The driver is wedged; don't process any more events. */
+}
+
static void execlists_cancel_requests(struct intel_engine_cs *engine)
{
struct intel_engine_execlists * const execlists = &engine->execlists;
@@ -871,6 +876,9 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
execlists->queue = RB_ROOT_CACHED;
GEM_BUG_ON(port_isset(execlists->port));
+ GEM_BUG_ON(__tasklet_is_enabled(&execlists->tasklet));
+ execlists->tasklet.func = nop_submission_tasklet;
+
spin_unlock_irqrestore(&engine->timeline.lock, flags);
}
--
2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RESEND 4/6] drm/i915/execlists: Disable submission tasklet upon wedging
2018-07-16 8:03 ` [RESEND 4/6] drm/i915/execlists: Disable submission tasklet upon wedging Chris Wilson
@ 2018-07-16 9:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-07-16 9:59 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 16/07/2018 09:03, Chris Wilson wrote:
> If we declare the driver wedged before the GPU truly is, then we may see
> the GPU complete some CS events following our cancellation. This leaves
> us quite confused as we deleted all the bookkeeping and thus complain
> about the inconsistent state.
>
> We can just ignore the remaining events and let the GPU idle by not
> feeding it, and so avoid trying to racily overwrite shared state. We
> rely on there being a full GPU reset before unwedging, giving us the
> opportunity to reset the shared state.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107188
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 4ef4439ff438..2c0050bab71e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -811,6 +811,11 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
> WRITE_ONCE(*execlists->csb_write, execlists->csb_write_reset);
> }
>
> +static void nop_submission_tasklet(unsigned long data)
> +{
> + /* The driver is wedged; don't process any more events. */
> +}
> +
> static void execlists_cancel_requests(struct intel_engine_cs *engine)
> {
> struct intel_engine_execlists * const execlists = &engine->execlists;
> @@ -871,6 +876,9 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
> execlists->queue = RB_ROOT_CACHED;
> GEM_BUG_ON(port_isset(execlists->port));
>
> + GEM_BUG_ON(__tasklet_is_enabled(&execlists->tasklet));
> + execlists->tasklet.func = nop_submission_tasklet;
> +
> spin_unlock_irqrestore(&engine->timeline.lock, flags);
> }
>
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
I am a bit uncertain whether we are theoretically safe without any
memory barriers in or after intel_engines_reset_default_submission.
Concern being that we reset and re-enable everything and someone still
see the no_submission_tasklet as the set one. There's a lot of code in
between so hopefully it is not possible.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
` (2 preceding siblings ...)
2018-07-16 8:03 ` [RESEND 4/6] drm/i915/execlists: Disable submission tasklet upon wedging Chris Wilson
@ 2018-07-16 8:03 ` Chris Wilson
2018-07-16 10:04 ` Tvrtko Ursulin
2018-07-16 10:32 ` Michal Wajdeczko
2018-07-16 8:03 ` [RESEND 6/6] drm/i915/selftests: Downgrade igt_timeout message Chris Wilson
` (4 subsequent siblings)
8 siblings, 2 replies; 18+ messages in thread
From: Chris Wilson @ 2018-07-16 8:03 UTC (permalink / raw)
To: intel-gfx
On an aborted module load, we unwind and free our device private - but
we left a dangling pointer to our privates inside the pci_device. After
the attempted aborted unload, we may still get a call to i915_pci_remove()
when the module is removed, potentially chasing stale data.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/i915_pci.c | 13 ++++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 700ffb611187..3834bd758a2e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1424,6 +1424,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
drm_dev_fini(&dev_priv->drm);
out_free:
kfree(dev_priv);
+ pci_set_drvdata(pdev, NULL);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 55543f1b0236..6a4d1388ad2d 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -674,10 +674,16 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
static void i915_pci_remove(struct pci_dev *pdev)
{
- struct drm_device *dev = pci_get_drvdata(pdev);
+ struct drm_device *dev;
+
+ dev = pci_get_drvdata(pdev);
+ if (!dev) /* driver load aborted, nothing to cleanup */
+ return;
i915_driver_unload(dev);
drm_dev_put(dev);
+
+ pci_set_drvdata(pdev, NULL);
}
static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -712,6 +718,11 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
return err;
+ if (i915_inject_load_failure()) {
+ i915_pci_remove(pdev);
+ return -ENODEV;
+ }
+
err = i915_live_selftests(pdev);
if (err) {
i915_pci_remove(pdev);
--
2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private
2018-07-16 8:03 ` [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private Chris Wilson
@ 2018-07-16 10:04 ` Tvrtko Ursulin
2018-07-16 10:32 ` Michal Wajdeczko
1 sibling, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-07-16 10:04 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 16/07/2018 09:03, Chris Wilson wrote:
> On an aborted module load, we unwind and free our device private - but
> we left a dangling pointer to our privates inside the pci_device. After
> the attempted aborted unload, we may still get a call to i915_pci_remove()
> when the module is removed, potentially chasing stale data.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_pci.c | 13 ++++++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 700ffb611187..3834bd758a2e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1424,6 +1424,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
> drm_dev_fini(&dev_priv->drm);
> out_free:
> kfree(dev_priv);
> + pci_set_drvdata(pdev, NULL);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 55543f1b0236..6a4d1388ad2d 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -674,10 +674,16 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>
> static void i915_pci_remove(struct pci_dev *pdev)
> {
> - struct drm_device *dev = pci_get_drvdata(pdev);
> + struct drm_device *dev;
> +
> + dev = pci_get_drvdata(pdev);
> + if (!dev) /* driver load aborted, nothing to cleanup */
> + return;
>
> i915_driver_unload(dev);
> drm_dev_put(dev);
> +
> + pci_set_drvdata(pdev, NULL);
> }
>
> static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> @@ -712,6 +718,11 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (err)
> return err;
>
> + if (i915_inject_load_failure()) {
> + i915_pci_remove(pdev);
> + return -ENODEV;
> + }
> +
> err = i915_live_selftests(pdev);
> if (err) {
> i915_pci_remove(pdev);
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private
2018-07-16 8:03 ` [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private Chris Wilson
2018-07-16 10:04 ` Tvrtko Ursulin
@ 2018-07-16 10:32 ` Michal Wajdeczko
2018-07-16 10:37 ` Chris Wilson
1 sibling, 1 reply; 18+ messages in thread
From: Michal Wajdeczko @ 2018-07-16 10:32 UTC (permalink / raw)
To: intel-gfx, Chris Wilson
On Mon, 16 Jul 2018 10:03:31 +0200, Chris Wilson
<chris@chris-wilson.co.uk> wrote:
> On an aborted module load, we unwind and free our device private - but
> we left a dangling pointer to our privates inside the pci_device. After
> the attempted aborted unload, we may still get a call to
> i915_pci_remove()
> when the module is removed, potentially chasing stale data.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_pci.c | 13 ++++++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 700ffb611187..3834bd758a2e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1424,6 +1424,7 @@ int i915_driver_load(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> drm_dev_fini(&dev_priv->drm);
> out_free:
> kfree(dev_priv);
> + pci_set_drvdata(pdev, NULL);
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/i915_pci.c
> b/drivers/gpu/drm/i915/i915_pci.c
> index 55543f1b0236..6a4d1388ad2d 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -674,10 +674,16 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
> static void i915_pci_remove(struct pci_dev *pdev)
> {
> - struct drm_device *dev = pci_get_drvdata(pdev);
> + struct drm_device *dev;
> +
> + dev = pci_get_drvdata(pdev);
> + if (!dev) /* driver load aborted, nothing to cleanup */
> + return;
> i915_driver_unload(dev);
> drm_dev_put(dev);
> +
> + pci_set_drvdata(pdev, NULL);
Why we can't put this inside i915_driver_release() ?
Then it will be in sync with kfree()
Michal
> }
> static int i915_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> @@ -712,6 +718,11 @@ static int i915_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
> if (err)
> return err;
> + if (i915_inject_load_failure()) {
> + i915_pci_remove(pdev);
> + return -ENODEV;
> + }
> +
> err = i915_live_selftests(pdev);
> if (err) {
> i915_pci_remove(pdev);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private
2018-07-16 10:32 ` Michal Wajdeczko
@ 2018-07-16 10:37 ` Chris Wilson
0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2018-07-16 10:37 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
Quoting Michal Wajdeczko (2018-07-16 11:32:22)
> On Mon, 16 Jul 2018 10:03:31 +0200, Chris Wilson
> <chris@chris-wilson.co.uk> wrote:
>
> > On an aborted module load, we unwind and free our device private - but
> > we left a dangling pointer to our privates inside the pci_device. After
> > the attempted aborted unload, we may still get a call to
> > i915_pci_remove()
> > when the module is removed, potentially chasing stale data.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 1 +
> > drivers/gpu/drm/i915/i915_pci.c | 13 ++++++++++++-
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 700ffb611187..3834bd758a2e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1424,6 +1424,7 @@ int i915_driver_load(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > drm_dev_fini(&dev_priv->drm);
> > out_free:
> > kfree(dev_priv);
> > + pci_set_drvdata(pdev, NULL);
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index 55543f1b0236..6a4d1388ad2d 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -674,10 +674,16 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
> > static void i915_pci_remove(struct pci_dev *pdev)
> > {
> > - struct drm_device *dev = pci_get_drvdata(pdev);
> > + struct drm_device *dev;
> > +
> > + dev = pci_get_drvdata(pdev);
> > + if (!dev) /* driver load aborted, nothing to cleanup */
> > + return;
> > i915_driver_unload(dev);
> > drm_dev_put(dev);
> > +
> > + pci_set_drvdata(pdev, NULL);
>
> Why we can't put this inside i915_driver_release() ?
> Then it will be in sync with kfree()
It pairs here with the pci remove. The driver release callback (being
the kref_put release) can happen later. That was my other choice, but I
went with doing it here because this we have the pci device here and the
synergy with the get+test followed set. My thinking was that if we tried
to dig out the pci device in a later release function (or pci callback
after remove), that should be clearly erroneous.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RESEND 6/6] drm/i915/selftests: Downgrade igt_timeout message
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
` (3 preceding siblings ...)
2018-07-16 8:03 ` [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private Chris Wilson
@ 2018-07-16 8:03 ` Chris Wilson
2018-07-16 10:06 ` Tvrtko Ursulin
2018-07-16 8:32 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang Patchwork
` (3 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2018-07-16 8:03 UTC (permalink / raw)
To: intel-gfx
Give in, since CI continues to incorrectly insist that KERN_NOTICE is a
warning and flags the timeout message as unwanted spam. At first, the
intention was to use the message to indicate which tests might warrant
an extended run, but virtually all tests require a timeout so it is
simply not as interesting as first thought.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103667
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_selftest.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
index 9766e806dce6..a73472dd12fd 100644
--- a/drivers/gpu/drm/i915/i915_selftest.h
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -99,6 +99,6 @@ __printf(2, 3)
bool __igt_timeout(unsigned long timeout, const char *fmt, ...);
#define igt_timeout(t, fmt, ...) \
- __igt_timeout((t), KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+ __igt_timeout((t), KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#endif /* !__I915_SELFTEST_H__ */
--
2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RESEND 6/6] drm/i915/selftests: Downgrade igt_timeout message
2018-07-16 8:03 ` [RESEND 6/6] drm/i915/selftests: Downgrade igt_timeout message Chris Wilson
@ 2018-07-16 10:06 ` Tvrtko Ursulin
0 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-07-16 10:06 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 16/07/2018 09:03, Chris Wilson wrote:
> Give in, since CI continues to incorrectly insist that KERN_NOTICE is a
> warning and flags the timeout message as unwanted spam. At first, the
> intention was to use the message to indicate which tests might warrant
> an extended run, but virtually all tests require a timeout so it is
> simply not as interesting as first thought.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103667
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_selftest.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index 9766e806dce6..a73472dd12fd 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -99,6 +99,6 @@ __printf(2, 3)
> bool __igt_timeout(unsigned long timeout, const char *fmt, ...);
>
> #define igt_timeout(t, fmt, ...) \
> - __igt_timeout((t), KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> + __igt_timeout((t), KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>
> #endif /* !__I915_SELFTEST_H__ */
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Do we need two timeout flavours in selftests - expected (limiting
runtime) and unexpected (timed out waiting for some event)?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
` (4 preceding siblings ...)
2018-07-16 8:03 ` [RESEND 6/6] drm/i915/selftests: Downgrade igt_timeout message Chris Wilson
@ 2018-07-16 8:32 ` Patchwork
2018-07-16 8:53 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-07-16 8:32 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang
URL : https://patchwork.freedesktop.org/series/46571/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
26d8219c9def drm/i915/selftests: Force a preemption hang
570a0ebc0c98 drm/i915/selftests: Exercise reset to break stuck GTT eviction
-:121: WARNING:WAITQUEUE_ACTIVE: waitqueue_active without comment
#121: FILE: drivers/gpu/drm/i915/selftests/intel_hangcheck.c:1081:
+ if (wait_for(waitqueue_active(&rq->execute), 10)) {
total: 0 errors, 1 warnings, 0 checks, 178 lines checked
75e6dcb97596 drm/i915/execlists: Always clear preempt status on cancelling all
f642984f5e54 drm/i915/execlists: Disable submission tasklet upon wedging
23666fefa850 drm/i915: Remove pci private pointer after destroying the device private
ca124485d43f drm/i915/selftests: Downgrade igt_timeout message
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* ✓ Fi.CI.BAT: success for series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
` (5 preceding siblings ...)
2018-07-16 8:32 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang Patchwork
@ 2018-07-16 8:53 ` Patchwork
2018-07-16 9:08 ` [RESEND 1/6] " Tvrtko Ursulin
2018-07-16 15:04 ` ✓ Fi.CI.IGT: success for series starting with [RESEND,1/6] " Patchwork
8 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-07-16 8:53 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang
URL : https://patchwork.freedesktop.org/series/46571/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4492 -> Patchwork_9666 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/46571/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9666 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_coherency:
fi-gdg-551: PASS -> DMESG-FAIL (fdo#107164)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: NOTRUN -> INCOMPLETE (fdo#103713)
==== Possible fixes ====
igt@debugfs_test@read_all_entries:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
igt@drv_selftest@live_workarounds:
fi-kbl-x1275: INCOMPLETE (fdo#107220) -> PASS
igt@drv_selftest@mock_scatterlist:
fi-bdw-5557u: DMESG-WARN (fdo#103667) -> PASS
fi-bwr-2160: DMESG-WARN (fdo#103667) -> PASS
fi-snb-2600: DMESG-WARN (fdo#103667) -> PASS
fi-skl-6770hq: DMESG-WARN (fdo#103667) -> PASS
fi-whl-u: DMESG-WARN (fdo#103667) -> PASS
fi-elk-e7500: DMESG-WARN (fdo#103667) -> PASS
fi-ivb-3520m: DMESG-WARN (fdo#103667) -> PASS
fi-skl-6260u: DMESG-WARN (fdo#103667) -> PASS
{fi-skl-iommu}: DMESG-WARN (fdo#103667) -> PASS
fi-glk-j4005: DMESG-WARN (fdo#103667) -> PASS
fi-ilk-650: DMESG-WARN (fdo#103667) -> PASS
fi-bdw-gvtdvm: DMESG-WARN (fdo#103667) -> PASS
fi-kbl-x1275: DMESG-WARN (fdo#103667) -> PASS
fi-cfl-guc: DMESG-WARN (fdo#103667) -> PASS
fi-kbl-7567u: DMESG-WARN (fdo#103667) -> PASS
fi-skl-guc: DMESG-WARN (fdo#103667) -> PASS
fi-glk-dsi: DMESG-WARN (fdo#103667) -> PASS
fi-cfl-8700k: DMESG-WARN (fdo#103667) -> PASS
fi-gdg-551: DMESG-WARN (fdo#103667) -> PASS
fi-kbl-7500u: DMESG-WARN (fdo#103667) -> PASS
fi-kbl-guc: DMESG-WARN (fdo#103667) -> PASS
fi-hsw-4770: DMESG-WARN (fdo#103667) -> PASS
fi-kbl-7560u: DMESG-WARN (fdo#103667) -> PASS
fi-bxt-dsi: DMESG-WARN (fdo#103667) -> PASS
fi-bxt-j4205: DMESG-WARN (fdo#103667) -> PASS
fi-ivb-3770: DMESG-WARN (fdo#103667) -> PASS
fi-bsw-n3050: DMESG-WARN (fdo#103667) -> PASS
fi-blb-e6850: DMESG-WARN (fdo#103667) -> PASS
fi-skl-6600u: DMESG-WARN (fdo#103667) -> PASS
fi-kbl-r: DMESG-WARN (fdo#103667) -> PASS
fi-hsw-4770r: DMESG-WARN (fdo#103667) -> PASS
fi-byt-n2820: DMESG-WARN (fdo#103667) -> PASS
{fi-cfl-8109u}: DMESG-WARN (fdo#103667) -> PASS
fi-cfl-s3: DMESG-WARN (fdo#103667) -> PASS
==== Warnings ====
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: INCOMPLETE (fdo#107139) -> DMESG-WARN (fdo#107222, fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103667 https://bugs.freedesktop.org/show_bug.cgi?id=103667
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107220 https://bugs.freedesktop.org/show_bug.cgi?id=107220
fdo#107222 https://bugs.freedesktop.org/show_bug.cgi?id=107222
== Participating hosts (38 -> 40) ==
Additional (4): fi-hsw-peppy fi-skl-gvtdvm fi-skl-6700k2 fi-pnv-d510
Missing (2): fi-ilk-m540 fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4492 -> Patchwork_9666
CI_DRM_4492: 1a9b9e1bf9510b583f8c78a42e65ea7fa323040a @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4558: d8e97e1710b27a3931a1c53d1dd88c0e709c085b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9666: ca124485d43fc19eccca5667cb39fad5e7111985 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
ca124485d43f drm/i915/selftests: Downgrade igt_timeout message
23666fefa850 drm/i915: Remove pci private pointer after destroying the device private
f642984f5e54 drm/i915/execlists: Disable submission tasklet upon wedging
75e6dcb97596 drm/i915/execlists: Always clear preempt status on cancelling all
570a0ebc0c98 drm/i915/selftests: Exercise reset to break stuck GTT eviction
26d8219c9def drm/i915/selftests: Force a preemption hang
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9666/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RESEND 1/6] drm/i915/selftests: Force a preemption hang
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
` (6 preceding siblings ...)
2018-07-16 8:53 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-07-16 9:08 ` Tvrtko Ursulin
2018-07-16 15:04 ` ✓ Fi.CI.IGT: success for series starting with [RESEND,1/6] " Patchwork
8 siblings, 0 replies; 18+ messages in thread
From: Tvrtko Ursulin @ 2018-07-16 9:08 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 16/07/2018 09:03, Chris Wilson wrote:
> Inject a failure into preemption completion to pretend as if the HW
> didn't successfully handle preemption and we are forced to do a reset in
> the middle.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_guc_submission.c | 3 +
> drivers/gpu/drm/i915/intel_lrc.c | 3 +
> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +
> drivers/gpu/drm/i915/selftests/intel_lrc.c | 101 ++++++++++++++++++++
> 4 files changed, 109 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index cc444dc5f3ad..de57cf6085d1 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -628,6 +628,9 @@ static void complete_preempt_context(struct intel_engine_cs *engine)
>
> GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT));
>
> + if (I915_SELFTEST_ONLY(execlists->preempt_hang))
> + return;
> +
> execlists_cancel_port_requests(execlists);
> execlists_unwind_incomplete_requests(execlists);
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 8fd8de71c2b5..703b76dcfcd2 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -559,6 +559,9 @@ static void complete_preempt_context(struct intel_engine_execlists *execlists)
> {
> GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT));
>
> + if (I915_SELFTEST_ONLY(execlists->preempt_hang))
> + return;
> +
> execlists_cancel_port_requests(execlists);
> __unwind_incomplete_requests(container_of(execlists,
> struct intel_engine_cs,
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index d1eee08e5f6b..37a7c819435a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -333,6 +333,8 @@ struct intel_engine_execlists {
> * @csb_head: context status buffer head
> */
> u8 csb_head;
> +
> + I915_SELFTEST_DECLARE(bool preempt_hang;)
> };
>
> #define INTEL_ENGINE_CS_MAX_NAME 8
> diff --git a/drivers/gpu/drm/i915/selftests/intel_lrc.c b/drivers/gpu/drm/i915/selftests/intel_lrc.c
> index 636cb68191e3..d642e78ef145 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c
> @@ -451,12 +451,113 @@ static int live_late_preempt(void *arg)
> goto err_ctx_lo;
> }
>
> +static int live_preempt_hang(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct i915_gem_context *ctx_hi, *ctx_lo;
> + struct spinner spin_hi, spin_lo;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + int err = -ENOMEM;
> +
> + if (!HAS_LOGICAL_RING_PREEMPTION(i915))
> + return 0;
> +
> + if (!intel_has_reset_engine(i915))
> + return 0;
> +
> + mutex_lock(&i915->drm.struct_mutex);
> +
> + if (spinner_init(&spin_hi, i915))
> + goto err_unlock;
> +
> + if (spinner_init(&spin_lo, i915))
> + goto err_spin_hi;
> +
> + ctx_hi = kernel_context(i915);
> + if (!ctx_hi)
> + goto err_spin_lo;
> + ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
Use more than max user to be more robust against possible scheduling
policy changes in the future?
> +
> + ctx_lo = kernel_context(i915);
> + if (!ctx_lo)
> + goto err_ctx_hi;
> + ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
> +
> + for_each_engine(engine, i915, id) {
> + struct i915_request *rq;
> +
> + rq = spinner_create_request(&spin_lo, ctx_lo, engine,
> + MI_ARB_CHECK);
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + goto err_ctx_lo;
> + }
> +
> + i915_request_add(rq);
> + if (!wait_for_spinner(&spin_lo, rq)) {
> + GEM_TRACE("lo spinner failed to start\n");
> + GEM_TRACE_DUMP();
> + i915_gem_set_wedged(i915);
> + err = -EIO;
> + goto err_ctx_lo;
> + }
> +
> + rq = spinner_create_request(&spin_hi, ctx_hi, engine,
> + MI_ARB_CHECK);
This one doesn't need to be preemptable but doesn't matter.
> + if (IS_ERR(rq)) {
> + spinner_end(&spin_lo);
> + err = PTR_ERR(rq);
> + goto err_ctx_lo;
> + }
> +
> + engine->execlists.preempt_hang = true;
> + i915_request_add(rq);
Wait for timeout waiting for preempting spinner to run? Otherwise we are
less sure that preempt_hang injection worked.
> +
> + set_bit(I915_RESET_ENGINE + id, &i915->gpu_error.flags);
> + i915_reset_engine(engine, NULL);
> + clear_bit(I915_RESET_ENGINE + id, &i915->gpu_error.flags);
> +
> + engine->execlists.preempt_hang = false;
> +
> + if (!wait_for_spinner(&spin_hi, rq)) {
> + GEM_TRACE("hi spinner failed to start\n");
> + GEM_TRACE_DUMP();
> + i915_gem_set_wedged(i915);
> + err = -EIO;
> + goto err_ctx_lo;
> + }
> +
> + spinner_end(&spin_hi);
> + spinner_end(&spin_lo);
> + if (igt_flush_test(i915, I915_WAIT_LOCKED)) {
> + err = -EIO;
> + goto err_ctx_lo;
> + }
> + }
> +
> + err = 0;
> +err_ctx_lo:
> + kernel_context_close(ctx_lo);
> +err_ctx_hi:
> + kernel_context_close(ctx_hi);
> +err_spin_lo:
> + spinner_fini(&spin_lo);
> +err_spin_hi:
> + spinner_fini(&spin_hi);
> +err_unlock:
> + igt_flush_test(i915, I915_WAIT_LOCKED);
> + mutex_unlock(&i915->drm.struct_mutex);
> + return err;
> +}
> +
> int intel_execlists_live_selftests(struct drm_i915_private *i915)
> {
> static const struct i915_subtest tests[] = {
> SUBTEST(live_sanitycheck),
> SUBTEST(live_preempt),
> SUBTEST(live_late_preempt),
> + SUBTEST(live_preempt_hang),
> };
>
> if (!HAS_EXECLISTS(i915))
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread* ✓ Fi.CI.IGT: success for series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang
2018-07-16 8:03 [RESEND 1/6] drm/i915/selftests: Force a preemption hang Chris Wilson
` (7 preceding siblings ...)
2018-07-16 9:08 ` [RESEND 1/6] " Tvrtko Ursulin
@ 2018-07-16 15:04 ` Patchwork
8 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-07-16 15:04 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RESEND,1/6] drm/i915/selftests: Force a preemption hang
URL : https://patchwork.freedesktop.org/series/46571/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4492_full -> Patchwork_9666_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9666_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9666_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9666_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-bsd1:
shard-kbl: PASS -> SKIP +3
igt@gem_exec_schedule@reorder-wide-render:
shard-glk: SKIP -> PASS +37
igt@gem_userptr_blits@map-fixed-invalidate:
shard-apl: SKIP -> PASS +37
igt@perf_pmu@frequency:
shard-kbl: SKIP -> PASS +35
== Known issues ==
Here are the changes found in Patchwork_9666_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_hangcheck:
shard-kbl: PASS -> DMESG-FAIL (fdo#106947)
igt@kms_flip@2x-dpms-vs-vblank-race-interruptible:
shard-hsw: PASS -> FAIL (fdo#103060) +2
igt@kms_flip@plain-flip-ts-check-interruptible:
shard-glk: PASS -> FAIL (fdo#100368) +1
igt@kms_rotation_crc@sprite-rotation-180:
shard-hsw: PASS -> FAIL (fdo#103925)
igt@kms_universal_plane@cursor-fb-leak-pipe-a:
shard-apl: PASS -> FAIL (fdo#107241)
==== Possible fixes ====
igt@drv_missed_irq:
shard-apl: INCOMPLETE (fdo#103927) -> PASS +1
shard-glk: INCOMPLETE (k.org#198133, fdo#103359) -> PASS +1
igt@drv_selftest@mock_scatterlist:
shard-glk: DMESG-WARN (fdo#103667) -> PASS
shard-hsw: DMESG-WARN (fdo#103667) -> PASS
shard-kbl: DMESG-WARN (fdo#103667) -> PASS
shard-snb: DMESG-WARN (fdo#103667) -> PASS
shard-apl: DMESG-WARN (fdo#103667) -> PASS
igt@drv_suspend@shrink:
shard-glk: FAIL (fdo#106886) -> PASS
igt@gem_busy@extended-parallel-blt:
shard-snb: INCOMPLETE (fdo#105411) -> PASS
igt@gem_exec_schedule@preempt-hang-bsd:
shard-glk: DMESG-FAIL -> PASS +3
igt@gem_exec_schedule@preempt-hang-bsd1:
shard-kbl: DMESG-FAIL -> PASS +4
igt@gem_exec_schedule@preempt-hang-render:
shard-apl: DMESG-FAIL -> PASS +3
igt@kms_cursor_crc@cursor-256x256-suspend:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS +3
igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
shard-hsw: FAIL (fdo#103355) -> PASS
igt@kms_draw_crc@draw-method-xrgb2101010-blt-xtiled:
shard-kbl: FAIL -> PASS +1
igt@kms_flip@2x-flip-vs-wf_vblank-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS +1
igt@kms_frontbuffer_tracking@fbc-1p-shrfb-fliptrack:
shard-glk: FAIL -> PASS +8
shard-kbl: FAIL (fdo#106067) -> PASS +1
igt@kms_plane_lowres@pipe-c-tiling-y:
shard-kbl: FAIL (fdo#106066) -> PASS +3
igt@kms_rotation_crc@bad-pixel-format:
shard-apl: FAIL -> PASS +6
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103667 https://bugs.freedesktop.org/show_bug.cgi?id=103667
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#106066 https://bugs.freedesktop.org/show_bug.cgi?id=106066
fdo#106067 https://bugs.freedesktop.org/show_bug.cgi?id=106067
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107241 https://bugs.freedesktop.org/show_bug.cgi?id=107241
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4492 -> Patchwork_9666
CI_DRM_4492: 1a9b9e1bf9510b583f8c78a42e65ea7fa323040a @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4558: d8e97e1710b27a3931a1c53d1dd88c0e709c085b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9666: ca124485d43fc19eccca5667cb39fad5e7111985 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9666/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread