From: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Use a separate acquire context list and a separate locking function for objects that are locked for eviction. These objects are then properly referenced while on the list and can be unlocked early in the ww transaction.
Co-developed-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Matthew Auld matthew.auld@intel.com --- drivers/gpu/drm/i915/gem/i915_gem_object.h | 67 +++++++++++++++++-- .../gpu/drm/i915/gem/i915_gem_object_types.h | 5 ++ drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 14 +++- drivers/gpu/drm/i915/i915_gem_ww.c | 51 ++++++++++---- drivers/gpu/drm/i915/i915_gem_ww.h | 3 + 5 files changed, 122 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 52a36b4052f0..e237b0fb0e79 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -158,6 +158,32 @@ static inline void assert_object_held_shared(struct drm_i915_gem_object *obj) assert_object_held(obj); }
+static inline int +i915_gem_object_lock_to_evict(struct drm_i915_gem_object *obj, + struct i915_gem_ww_ctx *ww) +{ + int ret; + + if (ww->intr) + ret = dma_resv_lock_interruptible(obj->base.resv, &ww->ctx); + else + ret = dma_resv_lock(obj->base.resv, &ww->ctx); + + if (!ret) { + list_add_tail(&obj->obj_link, &ww->eviction_list); + i915_gem_object_get(obj); + obj->evict_locked = true; + } + + GEM_WARN_ON(ret == -EALREADY); + if (ret == -EDEADLK) { + ww->contended_evict = true; + ww->contended = i915_gem_object_get(obj); + } + + return ret; +} + static inline int __i915_gem_object_lock(struct drm_i915_gem_object *obj, struct i915_gem_ww_ctx *ww, bool intr) @@ -169,13 +195,25 @@ 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) { list_add_tail(&obj->obj_link, &ww->obj_list); - if (ret == -EALREADY) - ret = 0; + obj->evict_locked = false; + }
- if (ret == -EDEADLK) + if (ret == -EALREADY) { + ret = 0; + /* We've already evicted an object needed for this batch. */ + if (obj->evict_locked) { + list_move_tail(&obj->obj_link, &ww->obj_list); + i915_gem_object_put(obj); + obj->evict_locked = false; + } + } + + if (ret == -EDEADLK) { + ww->contended_evict = false; ww->contended = i915_gem_object_get(obj); + }
return ret; } @@ -580,6 +618,27 @@ i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, __i915_gem_object_invalidate_frontbuffer(obj, origin); }
+/** + * i915_gem_get_locking_ctx - Get the locking context of a locked object + * if any. + * + * @obj: The object to get the locking ctx from + * + * RETURN: The locking context if the object was locked using a context. + * NULL otherwise. + */ +static inline struct i915_gem_ww_ctx * +i915_gem_get_locking_ctx(const struct drm_i915_gem_object *obj) +{ + struct ww_acquire_ctx *ctx; + + ctx = obj->base.resv->lock.ctx; + if (!ctx) + return NULL; + + return container_of(ctx, struct i915_gem_ww_ctx, ctx); +} + #ifdef CONFIG_MMU_NOTIFIER static inline bool i915_gem_object_is_userptr(struct drm_i915_gem_object *obj) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 331d113f7d5b..c42c0d3d5d67 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -142,6 +142,11 @@ struct drm_i915_gem_object { */ struct list_head obj_link;
+ /** + * @evict_locked: Whether @obj_link sits on the eviction_list + */ + bool evict_locked; + /** Stolen memory for this object, instead of being backed by shmem. */ struct drm_mm_node *stolen; union { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c index 27674048f17d..59d0f14b90ea 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c @@ -100,6 +100,7 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, unsigned long *nr_scanned, unsigned int shrink) { + struct drm_i915_gem_object *obj; const struct { struct list_head *list; unsigned int bit; @@ -164,7 +165,6 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, */ for (phase = phases; phase->list; phase++) { struct list_head still_in_list; - struct drm_i915_gem_object *obj; unsigned long flags;
if ((shrink & phase->bit) == 0) @@ -197,6 +197,10 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, if (!can_release_pages(obj)) continue;
+ /* Already locked this object? */ + if (ww && ww == i915_gem_get_locking_ctx(obj)) + continue; + if (!kref_get_unless_zero(&obj->base.refcount)) continue;
@@ -209,7 +213,11 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, if (!i915_gem_object_trylock(obj)) goto skip; } else { - err = i915_gem_object_lock(obj, ww); + err = i915_gem_object_lock_to_evict(obj, ww); + if (err == -EALREADY) { + err = 0; + goto skip; + } if (err) goto skip; } @@ -235,6 +243,8 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, if (err) return err; } + if (ww) + i915_gem_ww_ctx_unlock_evictions(ww);
if (shrink & I915_SHRINK_BOUND) intel_runtime_pm_put(&i915->runtime_pm, wakeref); diff --git a/drivers/gpu/drm/i915/i915_gem_ww.c b/drivers/gpu/drm/i915/i915_gem_ww.c index 43960d8595eb..811bf7677d78 100644 --- a/drivers/gpu/drm/i915/i915_gem_ww.c +++ b/drivers/gpu/drm/i915/i915_gem_ww.c @@ -10,24 +10,45 @@ void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ww, bool intr) { ww_acquire_init(&ww->ctx, &reservation_ww_class); INIT_LIST_HEAD(&ww->obj_list); + INIT_LIST_HEAD(&ww->eviction_list); ww->intr = intr; ww->contended = NULL; + ww->contended_evict = false; +} + +void i915_gem_ww_ctx_unlock_evictions(struct i915_gem_ww_ctx *ww) +{ + struct drm_i915_gem_object *obj, *next; + + list_for_each_entry_safe(obj, next, &ww->eviction_list, obj_link) { + list_del(&obj->obj_link); + GEM_WARN_ON(!obj->evict_locked); + i915_gem_object_unlock(obj); + i915_gem_object_put(obj); + } }
static void i915_gem_ww_ctx_unlock_all(struct i915_gem_ww_ctx *ww) { - struct drm_i915_gem_object *obj; + struct drm_i915_gem_object *obj, *next;
- while ((obj = list_first_entry_or_null(&ww->obj_list, struct drm_i915_gem_object, obj_link))) { + list_for_each_entry_safe(obj, next, &ww->obj_list, obj_link) { list_del(&obj->obj_link); + GEM_WARN_ON(obj->evict_locked); i915_gem_object_unlock(obj); } + + i915_gem_ww_ctx_unlock_evictions(ww); }
void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj) { + bool evict_locked = obj->evict_locked; + list_del(&obj->obj_link); i915_gem_object_unlock(obj); + if (evict_locked) + i915_gem_object_put(obj); }
void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ww) @@ -39,27 +60,33 @@ void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ww)
int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ww) { + struct drm_i915_gem_object *obj = ww->contended; int ret = 0;
- if (WARN_ON(!ww->contended)) + if (WARN_ON(!obj)) return -EINVAL;
i915_gem_ww_ctx_unlock_all(ww); if (ww->intr) - ret = dma_resv_lock_slow_interruptible(ww->contended->base.resv, &ww->ctx); + ret = dma_resv_lock_slow_interruptible(obj->base.resv, &ww->ctx); else - dma_resv_lock_slow(ww->contended->base.resv, &ww->ctx); + dma_resv_lock_slow(obj->base.resv, &ww->ctx); + if (ret) + goto out;
/* - * Unlocking the contended lock again, as might not need it in - * the retried transaction. This does not increase starvation, - * but it's opening up for a wakeup flood if there are many - * transactions relaxing on this object. + * Unlocking the contended lock again, if it was locked for eviction. + * We will most likely not need it in the retried transaction. */ - if (!ret) - dma_resv_unlock(ww->contended->base.resv); + if (ww->contended_evict) { + dma_resv_unlock(obj->base.resv); + } else { + obj->evict_locked = false; + list_add_tail(&obj->obj_link, &ww->obj_list); + }
- i915_gem_object_put(ww->contended); +out: + i915_gem_object_put(obj); ww->contended = NULL;
return ret; diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h b/drivers/gpu/drm/i915/i915_gem_ww.h index f6b1a796667b..11793b170cc2 100644 --- a/drivers/gpu/drm/i915/i915_gem_ww.h +++ b/drivers/gpu/drm/i915/i915_gem_ww.h @@ -10,15 +10,18 @@ struct i915_gem_ww_ctx { struct ww_acquire_ctx ctx; struct list_head obj_list; + struct list_head eviction_list; struct drm_i915_gem_object *contended; unsigned short intr; unsigned short loop; + unsigned short contended_evict; };
void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr); void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ctx); int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ctx); void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj); +void i915_gem_ww_ctx_unlock_evictions(struct i915_gem_ww_ctx *ww);
/* Internal functions used by the inlines! Don't use. */ static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)