[Intel-gfx] [PATCH] drm/i915: Avoid context dereference inside execlists_submission_tasklet
Chris Wilson
chris at chris-wilson.co.uk
Tue Dec 19 23:06:50 UTC 2017
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 at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at 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 at intel.com>
Thanks, I'm looking forward to seeing how many incompletes now
evaporate.
-Chris
More information about the Intel-gfx
mailing list