* [Intel-gfx] [PATCH] drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
@ 2020-02-10 16:42 Chris Wilson
2020-02-10 17:25 ` Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Chris Wilson @ 2020-02-10 16:42 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
We manipulate ring->head while active in i915_request_retire underneath
the timeline manipulation. We cannot rely on a stable ring->head outside
of the timeline->mutex, in particular while setting up the context for
resume and reset.
Closes: https://gitlab.freedesktop.org/drm/intel/issues/1126
References: f3c0efc9fe7a ("drm/i915/execlists: Leave resetting ring to intel_ring")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 36 ++++++++++++--------------
drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
2 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 230470c58ec9..3f75861a53c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -235,7 +235,8 @@ static void execlists_init_reg_state(u32 *reg_state,
bool close);
static void
__execlists_update_reg_state(const struct intel_context *ce,
- const struct intel_engine_cs *engine);
+ const struct intel_engine_cs *engine,
+ u32 head);
static void mark_eio(struct i915_request *rq)
{
@@ -1184,12 +1185,11 @@ static void reset_active(struct i915_request *rq,
head = rq->tail;
else
head = active_request(ce->timeline, rq)->head;
- ce->ring->head = intel_ring_wrap(ce->ring, head);
- intel_ring_update_space(ce->ring);
+ head = intel_ring_wrap(ce->ring, head);
/* Scrub the context image to prevent replaying the previous batch */
restore_default_state(ce, engine);
- __execlists_update_reg_state(ce, engine);
+ __execlists_update_reg_state(ce, engine, head);
/* We've switched away, so this should be a no-op, but intent matters */
ce->lrc_desc |= CTX_DESC_FORCE_RESTORE;
@@ -2878,16 +2878,17 @@ static void execlists_context_unpin(struct intel_context *ce)
static void
__execlists_update_reg_state(const struct intel_context *ce,
- const struct intel_engine_cs *engine)
+ const struct intel_engine_cs *engine,
+ u32 head)
{
struct intel_ring *ring = ce->ring;
u32 *regs = ce->lrc_reg_state;
- GEM_BUG_ON(!intel_ring_offset_valid(ring, ring->head));
+ GEM_BUG_ON(!intel_ring_offset_valid(ring, head));
GEM_BUG_ON(!intel_ring_offset_valid(ring, ring->tail));
regs[CTX_RING_START] = i915_ggtt_offset(ring->vma);
- regs[CTX_RING_HEAD] = ring->head;
+ regs[CTX_RING_HEAD] = head;
regs[CTX_RING_TAIL] = ring->tail;
/* RPCS */
@@ -2916,7 +2917,7 @@ __execlists_context_pin(struct intel_context *ce,
ce->lrc_desc = lrc_descriptor(ce, engine) | CTX_DESC_FORCE_RESTORE;
ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
- __execlists_update_reg_state(ce, engine);
+ __execlists_update_reg_state(ce, engine, ce->ring->tail);
return 0;
}
@@ -2941,7 +2942,7 @@ static void execlists_context_reset(struct intel_context *ce)
/* Scrub away the garbage */
execlists_init_reg_state(ce->lrc_reg_state,
ce, ce->engine, ce->ring, true);
- __execlists_update_reg_state(ce, ce->engine);
+ __execlists_update_reg_state(ce, ce->engine, ce->ring->tail);
ce->lrc_desc |= CTX_DESC_FORCE_RESTORE;
}
@@ -3538,6 +3539,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
struct intel_engine_execlists * const execlists = &engine->execlists;
struct intel_context *ce;
struct i915_request *rq;
+ u32 head;
mb(); /* paranoia: read the CSB pointers from after the reset */
clflush(execlists->csb_write);
@@ -3565,15 +3567,15 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
if (i915_request_completed(rq)) {
/* Idle context; tidy up the ring so we can restart afresh */
- ce->ring->head = intel_ring_wrap(ce->ring, rq->tail);
+ head = intel_ring_wrap(ce->ring, rq->tail);
goto out_replay;
}
/* Context has requests still in-flight; it should not be idle! */
GEM_BUG_ON(i915_active_is_idle(&ce->active));
rq = active_request(ce->timeline, rq);
- ce->ring->head = intel_ring_wrap(ce->ring, rq->head);
- GEM_BUG_ON(ce->ring->head == ce->ring->tail);
+ head = intel_ring_wrap(ce->ring, rq->head);
+ GEM_BUG_ON(head == ce->ring->tail);
/*
* If this request hasn't started yet, e.g. it is waiting on a
@@ -3618,10 +3620,9 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
out_replay:
ENGINE_TRACE(engine, "replay {head:%04x, tail:%04x}\n",
- ce->ring->head, ce->ring->tail);
- intel_ring_update_space(ce->ring);
+ head, ce->ring->tail);
__execlists_reset_reg_state(ce, engine);
- __execlists_update_reg_state(ce, engine);
+ __execlists_update_reg_state(ce, engine, head);
ce->lrc_desc |= CTX_DESC_FORCE_RESTORE; /* paranoid: GPU was reset! */
unwind:
@@ -5265,10 +5266,7 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
restore_default_state(ce, engine);
/* Rerun the request; its payload has been neutered (if guilty). */
- ce->ring->head = head;
- intel_ring_update_space(ce->ring);
-
- __execlists_update_reg_state(ce, engine);
+ __execlists_update_reg_state(ce, engine, head);
}
bool
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 79b9f7d092e4..6f0295f18b38 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -201,7 +201,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio)
}
GEM_BUG_ON(!ce[1]->ring->size);
intel_ring_reset(ce[1]->ring, ce[1]->ring->size / 2);
- __execlists_update_reg_state(ce[1], engine);
+ __execlists_update_reg_state(ce[1], engine, ce[1]->ring->head);
rq[0] = igt_spinner_create_request(&spin, ce[0], MI_ARB_CHECK);
if (IS_ERR(rq[0])) {
--
2.25.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
2020-02-10 16:42 [Intel-gfx] [PATCH] drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex Chris Wilson
@ 2020-02-10 17:25 ` Chris Wilson
2020-02-10 18:41 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-02-10 19:06 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2020-02-10 17:25 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Quoting Chris Wilson (2020-02-10 16:42:15)
> We manipulate ring->head while active in i915_request_retire underneath
> the timeline manipulation. We cannot rely on a stable ring->head outside
> of the timeline->mutex, in particular while setting up the context for
> resume and reset.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1126
Fixes: 0881954965e3 ("drm/i915: Introduce intel_context.pin_mutex for pin management")
Fixes: e5dadff4b093 ("drm/i915: Protect request retirement with timeline->mutex")
> References: f3c0efc9fe7a ("drm/i915/execlists: Leave resetting ring to intel_ring")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
2020-02-10 16:42 [Intel-gfx] [PATCH] drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex Chris Wilson
2020-02-10 17:25 ` Chris Wilson
@ 2020-02-10 18:41 ` Patchwork
2020-02-10 19:06 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Patchwork @ 2020-02-10 18:41 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
URL : https://patchwork.freedesktop.org/series/73248/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
3838891e9c61 drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
-:13: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#13:
References: f3c0efc9fe7a ("drm/i915/execlists: Leave resetting ring to intel_ring")
-:13: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit f3c0efc9fe7a ("drm/i915/execlists: Leave resetting ring to intel_ring")'
#13:
References: f3c0efc9fe7a ("drm/i915/execlists: Leave resetting ring to intel_ring")
total: 1 errors, 1 warnings, 0 checks, 115 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
2020-02-10 16:42 [Intel-gfx] [PATCH] drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex Chris Wilson
2020-02-10 17:25 ` Chris Wilson
2020-02-10 18:41 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2020-02-10 19:06 ` Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Patchwork @ 2020-02-10 19:06 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
URL : https://patchwork.freedesktop.org/series/73248/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_7903 -> Patchwork_16504
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/index.html
Known issues
------------
Here are the changes found in Patchwork_16504 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_blt:
- fi-hsw-4770r: [PASS][1] -> [DMESG-FAIL][2] ([i915#725])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-hsw-4770r/igt@i915_selftest@live_blt.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-hsw-4770r/igt@i915_selftest@live_blt.html
#### Possible fixes ####
* igt@gem_exec_parallel@fds:
- fi-byt-n2820: [FAIL][3] ([i915#694]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-byt-n2820/igt@gem_exec_parallel@fds.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-byt-n2820/igt@gem_exec_parallel@fds.html
* igt@i915_selftest@live_blt:
- fi-ivb-3770: [DMESG-FAIL][5] ([i915#725]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-ivb-3770/igt@i915_selftest@live_blt.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-ivb-3770/igt@i915_selftest@live_blt.html
- fi-hsw-4770: [DMESG-FAIL][7] ([i915#553] / [i915#725]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-hsw-4770/igt@i915_selftest@live_blt.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-hsw-4770/igt@i915_selftest@live_blt.html
* igt@i915_selftest@live_gem_contexts:
- fi-cfl-8700k: [DMESG-FAIL][9] ([i915#623]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
- fi-byt-n2820: [DMESG-FAIL][11] ([i915#1052]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
- fi-cfl-guc: [INCOMPLETE][13] ([CI#80] / [fdo#106070] / [i915#424]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-cfl-guc/igt@i915_selftest@live_gem_contexts.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-cfl-guc/igt@i915_selftest@live_gem_contexts.html
- fi-cml-s: [DMESG-FAIL][15] ([i915#877]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-cml-s/igt@i915_selftest@live_gem_contexts.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-cml-s/igt@i915_selftest@live_gem_contexts.html
* igt@i915_selftest@live_gtt:
- fi-bdw-5557u: [TIMEOUT][17] ([fdo#112271]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7903/fi-bdw-5557u/igt@i915_selftest@live_gtt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/fi-bdw-5557u/igt@i915_selftest@live_gtt.html
[CI#80]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/80
[fdo#106070]: https://bugs.freedesktop.org/show_bug.cgi?id=106070
[fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
[i915#1052]: https://gitlab.freedesktop.org/drm/intel/issues/1052
[i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424
[i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
[i915#623]: https://gitlab.freedesktop.org/drm/intel/issues/623
[i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
[i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
[i915#877]: https://gitlab.freedesktop.org/drm/intel/issues/877
Participating hosts (47 -> 42)
------------------------------
Additional (4): fi-hsw-peppy fi-skl-lmem fi-gdg-551 fi-snb-2600
Missing (9): fi-ilk-m540 fi-bdw-samus fi-bsw-n3050 fi-hsw-4200u fi-byt-squawks fi-ilk-650 fi-blb-e6850 fi-byt-clapper fi-skl-6600u
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7903 -> Patchwork_16504
CI-20190529: 20190529
CI_DRM_7903: 47b768c475f4a11a48bc43e6228660f8b26a542b @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5433: 6a96c17f3a1b4e1f90b1a0b0ce42a7219875d1a4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_16504: 3838891e9c6169eed76e9b91ad655d9f39e5595f @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
3838891e9c61 drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16504/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-10 19:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 16:42 [Intel-gfx] [PATCH] drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex Chris Wilson
2020-02-10 17:25 ` Chris Wilson
2020-02-10 18:41 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-02-10 19:06 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).