On August 6, 2021 15:18:59 Daniel Vetter daniel.vetter@ffwll.ch wrote:
gem context refcounting is another exercise in least locking design it seems, where most things get destroyed upon context closure (which can race with anything really). Only the actual memory allocation and the locks survive while holding a reference.
This tripped up Jason when reimplementing the single timeline feature in
commit 00dae4d3d35d4f526929633b76e00b0ab4d3970d Author: Jason Ekstrand jason@jlekstrand.net Date: Thu Jul 8 10:48:12 2021 -0500
drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)
We could fix the bug by holding ctx->mutex, but it's cleaner to just
What bug is this fixing, exactly?
--Jason
make the context object actually invariant over its _entire_ lifetime.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Fixes: 00dae4d3d35d ("drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)") Cc: Jason Ekstrand jason@jlekstrand.net Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Matthew Brost matthew.brost@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: "Thomas Hellström" thomas.hellstrom@intel.com Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com Cc: Dave Airlie airlied@redhat.com
drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 754b9b8d4981..93ba0197d70a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -940,6 +940,9 @@ void i915_gem_context_release(struct kref *ref) trace_i915_context_free(ctx); GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
- if (ctx->syncobj)
- drm_syncobj_put(ctx->syncobj);
mutex_destroy(&ctx->engines_mutex); mutex_destroy(&ctx->lut_mutex);
@@ -1159,9 +1162,6 @@ static void context_close(struct i915_gem_context *ctx) if (vm) i915_vm_close(vm);
- if (ctx->syncobj)
- drm_syncobj_put(ctx->syncobj);
ctx->file_priv = ERR_PTR(-EBADF);
/*
2.32.0