Skip to content

Commit

Permalink
drm/i915: Use a dummy timeline name for a signaled fence
Browse files Browse the repository at this point in the history
Michał Winiarski pointed out that the debugging infrastructure (such as
trace_dma_fence_release) likes to pretty print the timeline name, long
after we have freed the timeline. Our timelines currently live as part of
the GTT (due to the strict ordering we currently use through each) which
belong to the context. We aim to free the context and release its
hardware resources as soon as we able to (i.e. when the last
fence/request using it has been signaled and retired). As the
.get_timeline_name is purely a debug feature, rather than extending the
lifetime of the context, or splitting it into many different release
phases just to keep the name around, replace the timeline name with a
constant after the fence has been signaled. This avoids the potential
use-after-free.

Reported-by: Krzysztof Olinski <krzysztof.e.olinski@intel.com>
Fixes: 80b204b ("drm/i915: Enable multiple timelines")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.10+
Link: http://patchwork.freedesktop.org/patch/msgid/20170330111614.29757-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
  • Loading branch information
Chris Wilson committed Mar 30, 2017
1 parent a69035e commit 05506b5
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions drivers/gpu/drm/i915/i915_gem_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ static const char *i915_fence_get_driver_name(struct dma_fence *fence)

static const char *i915_fence_get_timeline_name(struct dma_fence *fence)
{
/* The timeline struct (as part of the ppgtt underneath a context)
* may be freed when the request is no longer in use by the GPU.
* We could extend the life of a context to beyond that of all
* fences, possibly keeping the hw resource around indefinitely,
* or we just give them a false name. Since
* dma_fence_ops.get_timeline_name is a debug feature, the occasional
* lie seems justifiable.
*/
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return "signaled";

return to_request(fence)->timeline->common->name;
}

Expand Down

0 comments on commit 05506b5

Please sign in to comment.