On Thu, Mar 30, 2017 at 12:16:14PM +0100, Chris Wilson wrote: > 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 current 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 along, replace the timeline name with a > constant after the fence has been signaled. This avoids the potential > use-after-free. > > Reported-by: Michał Winiarski <[email protected]>
Actually, I'm just a messenger ;) Reported-by: Krzysztof Olinski <[email protected]> Reviewed-by: Michał Winiarski <[email protected]> -Michał > Fixes: 80b204bce8f2 ("drm/i915: Enable multiple timelines") > Signed-off-by: Chris Wilson <[email protected]> > Cc: Michał Winiarski <[email protected]> > Cc: Joonas Lahtinen <[email protected]> > Cc: <[email protected]> # v4.10+ > --- > drivers/gpu/drm/i915/i915_gem_request.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c > b/drivers/gpu/drm/i915/i915_gem_request.c > index cb9209589d2e..2cb2206b2f6b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_request.c > +++ b/drivers/gpu/drm/i915/i915_gem_request.c > @@ -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; > } > > -- > 2.11.0 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
