Skip to content

Commit

Permalink
drm/i915: Reference objects on the ww object list
Browse files Browse the repository at this point in the history
Since the ww transaction endpoint easily end up far out-of-scope of
the objects on the ww object list, particularly for contending lock
objects, make sure we reference objects on the list so they don't
disappear under us.

This comes with a performance penalty so it's been debated whether this
is really needed. But I think this is motivated by the fact that locking
is typically difficult to get right, and whatever we can do to make it
simpler for developers moving forward should be done, unless the
performance impact is far too high.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210617063018.92802-2-thomas.hellstrom@linux.intel.com
  • Loading branch information
Thomas Hellström authored and Matthew Auld committed Jun 17, 2021
1 parent a6c5b89 commit 1c4dbe0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
8 changes: 6 additions & 2 deletions drivers/gpu/drm/i915/gem/i915_gem_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,17 @@ static inline int __i915_gem_object_lock(struct drm_i915_gem_object *obj,
else
ret = dma_resv_lock(obj->base.resv, ww ? &ww->ctx : NULL);

if (!ret && ww)
if (!ret && ww) {
i915_gem_object_get(obj);
list_add_tail(&obj->obj_link, &ww->obj_list);
}
if (ret == -EALREADY)
ret = 0;

if (ret == -EDEADLK)
if (ret == -EDEADLK) {
i915_gem_object_get(obj);
ww->contended = obj;
}

return ret;
}
Expand Down
4 changes: 4 additions & 0 deletions drivers/gpu/drm/i915/i915_gem.c
Original file line number Diff line number Diff line change
Expand Up @@ -1216,13 +1216,15 @@ static void i915_gem_ww_ctx_unlock_all(struct i915_gem_ww_ctx *ww)
while ((obj = list_first_entry_or_null(&ww->obj_list, struct drm_i915_gem_object, obj_link))) {
list_del(&obj->obj_link);
i915_gem_object_unlock(obj);
i915_gem_object_put(obj);
}
}

void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj)
{
list_del(&obj->obj_link);
i915_gem_object_unlock(obj);
i915_gem_object_put(obj);
}

void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ww)
Expand All @@ -1247,6 +1249,8 @@ int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ww)

if (!ret)
list_add_tail(&ww->contended->obj_link, &ww->obj_list);
else
i915_gem_object_put(ww->contended);

ww->contended = NULL;

Expand Down

0 comments on commit 1c4dbe0

Please sign in to comment.