* [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet
@ 2017-12-19 22:09 Chris Wilson
2017-12-19 22:30 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Chris Wilson @ 2017-12-19 22:09 UTC (permalink / raw)
To: intel-gfx
A lesson that has to be relearnt over and over again is that the request
does not keep a reference to the context and so we cannot freely
dereference the context from inside the execlists_submission_tasklet. In
particular, we try to do so in the new GEM_TRACE() so convert those over
to the port->context_id we keep for GEM debugging. This means the
tracing now depends on DRM_I915_GEM_DEBUG.
Fixes: bccd3b831185 ("drm/i915: Use trace_printk to provide a death rattle for GEM")
References: https://bugs.freedesktop.org/show_bug.cgi?id=104066
References: https://bugs.freedesktop.org/show_bug.cgi?id=104162
References: https://bugs.freedesktop.org/show_bug.cgi?id=104242
References: https://bugs.freedesktop.org/show_bug.cgi?id=104310
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/Kconfig.debug | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index fa36491495b1..c846b250b9c4 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -29,7 +29,6 @@ config DRM_I915_DEBUG
select SW_SYNC # signaling validation framework (igt/syncobj*)
select DRM_I915_SW_FENCE_DEBUG_OBJECTS
select DRM_I915_SELFTEST
- select DRM_I915_TRACE_GEM
default n
help
Choose this option to turn on extra driver debugging that may affect
@@ -55,6 +54,7 @@ config DRM_I915_TRACE_GEM
bool "Insert extra ftrace output from the GEM internals"
select TRACING
default n
+ depends on DRM_I915_DEBUG_GEM
help
Enable additional and verbose debugging output that will spam
ordinary tests, but may be vital for post-mortem debugging when
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index eee718e3f371..64d49d5054b9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -449,7 +449,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
GEM_TRACE("%s in[%d]: ctx=%d.%d, seqno=%x\n",
engine->name, n,
- rq->ctx->hw_id, count,
+ port[n].context_id, count,
rq->global_seqno);
} else {
GEM_BUG_ON(!n);
@@ -861,7 +861,7 @@ static void execlists_submission_tasklet(unsigned long data)
*/
status = READ_ONCE(buf[2 * head]); /* maybe mmio! */
- GEM_TRACE("%s csb[%dd]: status=0x%08x:0x%08x\n",
+ GEM_TRACE("%s csb[%d]: status=0x%08x:0x%08x\n",
engine->name, head,
status, buf[2*head + 1]);
@@ -905,7 +905,7 @@ static void execlists_submission_tasklet(unsigned long data)
rq = port_unpack(port, &count);
GEM_TRACE("%s out[0]: ctx=%d.%d, seqno=%x\n",
engine->name,
- rq->ctx->hw_id, count,
+ port->context_id, count,
rq->global_seqno);
GEM_BUG_ON(count == 0);
if (--count == 0) {
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Avoid context dereference inside execlists_submission_tasklet
2017-12-19 22:09 [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet Chris Wilson
@ 2017-12-19 22:30 ` Patchwork
2017-12-19 22:43 ` [PATCH] " Michel Thierry
2017-12-20 0:12 ` ✗ Fi.CI.IGT: failure for " Patchwork
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-12-19 22:30 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Avoid context dereference inside execlists_submission_tasklet
URL : https://patchwork.freedesktop.org/series/35604/
State : success
== Summary ==
Series 35604v1 drm/i915: Avoid context dereference inside execlists_submission_tasklet
https://patchwork.freedesktop.org/api/1.0/series/35604/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
incomplete -> PASS (fi-snb-2520m) fdo#103713
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:434s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:441s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:384s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:499s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:276s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:496s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:496s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:475s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:470s
fi-elk-e7500 total:224 pass:163 dwarn:15 dfail:0 fail:0 skip:45
fi-gdg-551 total:288 pass:178 dwarn:1 dfail:0 fail:1 skip:108 time:265s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:531s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:405s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:411s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:389s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:484s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:424s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:480s
fi-kbl-7560u total:288 pass:268 dwarn:1 dfail:0 fail:0 skip:19 time:519s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:460s
fi-kbl-r total:288 pass:260 dwarn:1 dfail:0 fail:0 skip:27 time:525s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:588s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:447s
fi-skl-6600u total:288 pass:260 dwarn:1 dfail:0 fail:0 skip:27 time:530s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:557s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:503s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:503s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:448s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:538s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:410s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:590s
fi-cnl-y total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:625s
2c8d1d02646400b21aa2d68b9dc61b6cc00f661f drm-tip: 2017y-12m-19d-21h-47m-30s UTC integration manifest
6afc86a3f13b drm/i915: Avoid context dereference inside execlists_submission_tasklet
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7542/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet
2017-12-19 22:09 [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet Chris Wilson
2017-12-19 22:30 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-12-19 22:43 ` Michel Thierry
2017-12-19 23:06 ` Chris Wilson
2017-12-20 0:12 ` ✗ Fi.CI.IGT: failure for " Patchwork
2 siblings, 1 reply; 5+ messages in thread
From: Michel Thierry @ 2017-12-19 22:43 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 12/19/2017 2:09 PM, Chris Wilson wrote:
> A lesson that has to be relearnt over and over again is that the request
> does not keep a reference to the context and so we cannot freely
> dereference the context from inside the execlists_submission_tasklet. In
> particular, we try to do so in the new GEM_TRACE() so convert those over
> to the port->context_id we keep for GEM debugging. This means the
> tracing now depends on DRM_I915_GEM_DEBUG.
>
Even before the port->context_id dependency, I don't think many people
would enable DRM_I915_TRACE_GEM without DRM_I915_DEBUG_GEM.
> Fixes: bccd3b831185 ("drm/i915: Use trace_printk to provide a death rattle for GEM")
> References: https://bugs.freedesktop.org/show_bug.cgi?id=104066
> References: https://bugs.freedesktop.org/show_bug.cgi?id=104162
> References: https://bugs.freedesktop.org/show_bug.cgi?id=104242
> References: https://bugs.freedesktop.org/show_bug.cgi?id=104310
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/Kconfig.debug | 2 +-
> drivers/gpu/drm/i915/intel_lrc.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index fa36491495b1..c846b250b9c4 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -29,7 +29,6 @@ config DRM_I915_DEBUG
> select SW_SYNC # signaling validation framework (igt/syncobj*)
> select DRM_I915_SW_FENCE_DEBUG_OBJECTS
> select DRM_I915_SELFTEST
> - select DRM_I915_TRACE_GEM
> default n
> help
> Choose this option to turn on extra driver debugging that may affect
> @@ -55,6 +54,7 @@ config DRM_I915_TRACE_GEM
> bool "Insert extra ftrace output from the GEM internals"
> select TRACING
> default n
> + depends on DRM_I915_DEBUG_GEM
> help
> Enable additional and verbose debugging output that will spam
> ordinary tests, but may be vital for post-mortem debugging when
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index eee718e3f371..64d49d5054b9 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -449,7 +449,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
>
> GEM_TRACE("%s in[%d]: ctx=%d.%d, seqno=%x\n",
> engine->name, n,
> - rq->ctx->hw_id, count,
> + port[n].context_id, count,
> rq->global_seqno);
> } else {
> GEM_BUG_ON(!n);
> @@ -861,7 +861,7 @@ static void execlists_submission_tasklet(unsigned long data)
> */
>
> status = READ_ONCE(buf[2 * head]); /* maybe mmio! */
> - GEM_TRACE("%s csb[%dd]: status=0x%08x:0x%08x\n",
> + GEM_TRACE("%s csb[%d]: status=0x%08x:0x%08x\n",
> engine->name, head,
> status, buf[2*head + 1]);
>
> @@ -905,7 +905,7 @@ static void execlists_submission_tasklet(unsigned long data)
> rq = port_unpack(port, &count);
> GEM_TRACE("%s out[0]: ctx=%d.%d, seqno=%x\n",
> engine->name,
> - rq->ctx->hw_id, count,
> + port->context_id, count,
> rq->global_seqno);
> GEM_BUG_ON(count == 0);
> if (--count == 0) {
> --
> 2.15.1
>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet
2017-12-19 22:43 ` [PATCH] " Michel Thierry
@ 2017-12-19 23:06 ` Chris Wilson
0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-12-19 23:06 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
Quoting Michel Thierry (2017-12-19 22:43:57)
> On 12/19/2017 2:09 PM, Chris Wilson wrote:
> > A lesson that has to be relearnt over and over again is that the request
> > does not keep a reference to the context and so we cannot freely
> > dereference the context from inside the execlists_submission_tasklet. In
> > particular, we try to do so in the new GEM_TRACE() so convert those over
> > to the port->context_id we keep for GEM debugging. This means the
> > tracing now depends on DRM_I915_GEM_DEBUG.
> >
>
> Even before the port->context_id dependency, I don't think many people
> would enable DRM_I915_TRACE_GEM without DRM_I915_DEBUG_GEM.
Indeed :)
> > Fixes: bccd3b831185 ("drm/i915: Use trace_printk to provide a death rattle for GEM")
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=104066
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=104162
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=104242
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=104310
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> > drivers/gpu/drm/i915/Kconfig.debug | 2 +-
> > drivers/gpu/drm/i915/intel_lrc.c | 6 +++---
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> > index fa36491495b1..c846b250b9c4 100644
> > --- a/drivers/gpu/drm/i915/Kconfig.debug
> > +++ b/drivers/gpu/drm/i915/Kconfig.debug
> > @@ -29,7 +29,6 @@ config DRM_I915_DEBUG
> > select SW_SYNC # signaling validation framework (igt/syncobj*)
> > select DRM_I915_SW_FENCE_DEBUG_OBJECTS
> > select DRM_I915_SELFTEST
> > - select DRM_I915_TRACE_GEM
> > default n
> > help
> > Choose this option to turn on extra driver debugging that may affect
> > @@ -55,6 +54,7 @@ config DRM_I915_TRACE_GEM
> > bool "Insert extra ftrace output from the GEM internals"
> > select TRACING
> > default n
> > + depends on DRM_I915_DEBUG_GEM
> > help
> > Enable additional and verbose debugging output that will spam
> > ordinary tests, but may be vital for post-mortem debugging when
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index eee718e3f371..64d49d5054b9 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -449,7 +449,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
> >
> > GEM_TRACE("%s in[%d]: ctx=%d.%d, seqno=%x\n",
> > engine->name, n,
> > - rq->ctx->hw_id, count,
> > + port[n].context_id, count,
> > rq->global_seqno);
> > } else {
> > GEM_BUG_ON(!n);
> > @@ -861,7 +861,7 @@ static void execlists_submission_tasklet(unsigned long data)
> > */
> >
> > status = READ_ONCE(buf[2 * head]); /* maybe mmio! */
> > - GEM_TRACE("%s csb[%dd]: status=0x%08x:0x%08x\n",
> > + GEM_TRACE("%s csb[%d]: status=0x%08x:0x%08x\n",
> > engine->name, head,
> > status, buf[2*head + 1]);
> >
> > @@ -905,7 +905,7 @@ static void execlists_submission_tasklet(unsigned long data)
> > rq = port_unpack(port, &count);
> > GEM_TRACE("%s out[0]: ctx=%d.%d, seqno=%x\n",
> > engine->name,
> > - rq->ctx->hw_id, count,
> > + port->context_id, count,
> > rq->global_seqno);
> > GEM_BUG_ON(count == 0);
> > if (--count == 0) {
> > --
> > 2.15.1
> >
>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Thanks, I'm looking forward to seeing how many incompletes now
evaporate.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915: Avoid context dereference inside execlists_submission_tasklet
2017-12-19 22:09 [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet Chris Wilson
2017-12-19 22:30 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-12-19 22:43 ` [PATCH] " Michel Thierry
@ 2017-12-20 0:12 ` Patchwork
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-12-20 0:12 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Avoid context dereference inside execlists_submission_tasklet
URL : https://patchwork.freedesktop.org/series/35604/
State : failure
== Summary ==
Test kms_cursor_crc:
Subgroup cursor-64x64-offscreen:
pass -> SKIP (shard-snb)
Test pm_rpm:
Subgroup basic-rte:
pass -> SKIP (shard-hsw)
Test kms_flip:
Subgroup modeset-vs-vblank-race-interruptible:
pass -> FAIL (shard-hsw) fdo#103060 +1
Subgroup flip-vs-fences:
pass -> INCOMPLETE (shard-hsw)
Subgroup vblank-vs-modeset-suspend:
skip -> PASS (shard-snb) fdo#102365
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
fail -> PASS (shard-snb) fdo#101623
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
shard-hsw total:2701 pass:1527 dwarn:1 dfail:0 fail:12 skip:1160 time:9244s
shard-snb total:2712 pass:1309 dwarn:1 dfail:0 fail:10 skip:1392 time:8088s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7542/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-20 0:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-19 22:09 [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet Chris Wilson
2017-12-19 22:30 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-12-19 22:43 ` [PATCH] " Michel Thierry
2017-12-19 23:06 ` Chris Wilson
2017-12-20 0:12 ` ✗ Fi.CI.IGT: failure for " Patchwork
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.