[PATCH 04/12] drm/ttm: Switch LRU cursor to track dma_resv instead of buffer objects

[Christian König]

Refactor the LRU cursor to hold references to dma_resv directly while
locking it rather than the buffer object.

This avoid the need to grab a reference to the BO and so allows handling
of BOs with zero reference count.

Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c  | 124 +++++++++++++++--------------
 drivers/gpu/drm/ttm/ttm_resource.c |  18 +++++
 include/drm/ttm/ttm_bo.h           |  10 +--
 include/drm/ttm/ttm_resource.h     |   2 +
 4 files changed, 87 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 029c218f9fb47..1f7361604b552 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -819,19 +819,17 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
 }
 
 static bool ttm_lru_walk_trylock(struct ttm_bo_lru_cursor *curs,
-				 struct ttm_buffer_object *bo)
+				 struct dma_resv *resv)
 {
 	struct ttm_operation_ctx *ctx = curs->arg->ctx;
 
-	curs->needs_unlock = false;
-
-	if (dma_resv_trylock(bo->base.resv)) {
-		curs->needs_unlock = true;
+	if (dma_resv_trylock(resv)) {
+		curs->resv = dma_resv_get(resv);
 		return true;
 	}
 
-	if (bo->base.resv == ctx->resv && ctx->allow_res_evict) {
-		dma_resv_assert_held(bo->base.resv);
+	if (resv == ctx->resv && ctx->allow_res_evict) {
+		dma_resv_assert_held(resv);
 		return true;
 	}
 
@@ -839,18 +837,18 @@ static bool ttm_lru_walk_trylock(struct ttm_bo_lru_cursor *curs,
 }
 
 static int ttm_lru_walk_ticketlock(struct ttm_bo_lru_cursor *curs,
-				   struct ttm_buffer_object *bo)
+				   struct dma_resv *resv)
 {
 	struct ttm_lru_walk_arg *arg = curs->arg;
 	int ret;
 
 	if (arg->ctx->interruptible)
-		ret = dma_resv_lock_interruptible(bo->base.resv, arg->ticket);
+		ret = dma_resv_lock_interruptible(resv, arg->ticket);
 	else
-		ret = dma_resv_lock(bo->base.resv, arg->ticket);
+		ret = dma_resv_lock(resv, arg->ticket);
 
 	if (!ret) {
-		curs->needs_unlock = true;
+		curs->resv = dma_resv_get(resv);
 		/*
 		 * Only a single ticketlock per loop. Ticketlocks are prone
 		 * to return -EDEADLK causing the eviction to fail, so
@@ -920,14 +918,16 @@ s64 ttm_lru_walk_for_evict(struct ttm_lru_walk *walk, struct ttm_device *bdev,
 }
 EXPORT_SYMBOL(ttm_lru_walk_for_evict);
 
-static void ttm_bo_lru_cursor_cleanup_bo(struct ttm_bo_lru_cursor *curs)
+static void ttm_bo_lru_cursor_cleanup(struct ttm_bo_lru_cursor *curs)
 {
-	struct ttm_buffer_object *bo = curs->bo;
+	if (curs->resv) {
+		dma_resv_unlock(curs->resv);
+		dma_resv_put(curs->resv);
+		curs->resv = NULL;
+	}
 
-	if (bo) {
-		if (curs->needs_unlock)
-			dma_resv_unlock(bo->base.resv);
-		ttm_bo_put(bo);
+	if (curs->bo) {
+		drm_gem_object_put(&curs->bo->base);
 		curs->bo = NULL;
 	}
 }
@@ -941,7 +941,7 @@ void ttm_bo_lru_cursor_fini(struct ttm_bo_lru_cursor *curs)
 {
 	spinlock_t *lru_lock = &curs->res_curs.man->bdev->lru_lock;
 
-	ttm_bo_lru_cursor_cleanup_bo(curs);
+	ttm_bo_lru_cursor_cleanup(curs);
 	spin_lock(lru_lock);
 	ttm_resource_cursor_fini(&curs->res_curs);
 	spin_unlock(lru_lock);
@@ -972,21 +972,18 @@ ttm_bo_lru_cursor_init(struct ttm_bo_lru_cursor *curs,
 EXPORT_SYMBOL(ttm_bo_lru_cursor_init);
 
 static struct ttm_buffer_object *
-__ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
+__ttm_bo_lru_cursor_iter(struct ttm_bo_lru_cursor *curs, bool first)
 {
 	spinlock_t *lru_lock = &curs->res_curs.man->bdev->lru_lock;
-	struct ttm_resource *res = NULL;
-	struct ttm_buffer_object *bo;
 	struct ttm_lru_walk_arg *arg = curs->arg;
-	bool first = !curs->bo;
-
-	ttm_bo_lru_cursor_cleanup_bo(curs);
+	int ret;
 
-	spin_lock(lru_lock);
 	for (;;) {
-		int mem_type, ret = 0;
-		bool bo_locked = false;
+		struct ttm_resource *res;
+
+		ttm_bo_lru_cursor_cleanup(curs);
 
+		spin_lock(lru_lock);
 		if (first) {
 			res = ttm_resource_manager_first(&curs->res_curs);
 			first = false;
@@ -996,43 +993,48 @@ __ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
 		if (!res)
 			break;
 
-		bo = res->bo;
-		if (ttm_lru_walk_trylock(curs, bo))
-			bo_locked = true;
-		else if (!arg->ticket || arg->ctx->no_wait_gpu || arg->trylock_only)
-			continue;
-
-		if (!ttm_bo_get_unless_zero(bo)) {
-			if (curs->needs_unlock)
-				dma_resv_unlock(bo->base.resv);
-			continue;
+		if (!ttm_lru_walk_trylock(curs, res->bo->base.resv)) {
+			struct dma_resv *resv;
+
+			if (!arg->ticket || arg->ctx->no_wait_gpu ||
+			    arg->trylock_only) {
+				spin_unlock(lru_lock);
+				continue;
+			}
+
+			resv = dma_resv_get(res->bo->base.resv);
+			spin_unlock(lru_lock);
+
+			ret = ttm_lru_walk_ticketlock(curs, resv);
+			if (ret && ret != -EALREADY)
+				return ERR_PTR(ret);
+
+			/*
+			 * We need to double check that we still have the same
+			 * dma_resv object.
+			 */
+			spin_lock(lru_lock);
+			res = ttm_resource_manager_current(&curs->res_curs);
+			if (ret || !res || res->bo->base.resv != resv) {
+				spin_unlock(lru_lock);
+				dma_resv_put(resv);
+				continue;
+			}
+			dma_resv_put(resv);
 		}
-
-		mem_type = res->mem_type;
 		spin_unlock(lru_lock);
-		if (!bo_locked)
-			ret = ttm_lru_walk_ticketlock(curs, bo);
+
+		/* Grab a GEM reference to the BO if it isn't already deleted */
+		if (kref_get_unless_zero(&res->bo->base.refcount))
+			curs->bo = res->bo;
 
 		/*
-		 * Note that in between the release of the lru lock and the
-		 * ticketlock, the bo may have switched resource,
-		 * and also memory type, since the resource may have been
-		 * freed and allocated again with a different memory type.
-		 * In that case, just skip it.
+		 * The BO is now locked so it can't be released any more until
+		 * we drop both the lock and the eventual GEM reference.
 		 */
-		curs->bo = bo;
-		if (!ret && bo->resource && bo->resource->mem_type == mem_type)
-			return bo;
-
-		ttm_bo_lru_cursor_cleanup_bo(curs);
-		if (ret && ret != -EALREADY)
-			return ERR_PTR(ret);
-
-		spin_lock(lru_lock);
+		return res->bo;
 	}
-
-	spin_unlock(lru_lock);
-	return res ? bo : NULL;
+	return NULL;
 }
 
 /**
@@ -1046,7 +1048,7 @@ __ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
  */
 struct ttm_buffer_object *ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
 {
-	return __ttm_bo_lru_cursor_next(curs);
+	return __ttm_bo_lru_cursor_iter(curs, false);
 }
 EXPORT_SYMBOL(ttm_bo_lru_cursor_next);
 
@@ -1060,8 +1062,8 @@ EXPORT_SYMBOL(ttm_bo_lru_cursor_next);
  */
 struct ttm_buffer_object *ttm_bo_lru_cursor_first(struct ttm_bo_lru_cursor *curs)
 {
-	ttm_bo_lru_cursor_cleanup_bo(curs);
-	return __ttm_bo_lru_cursor_next(curs);
+	ttm_bo_lru_cursor_cleanup(curs);
+	return __ttm_bo_lru_cursor_iter(curs, true);
 }
 EXPORT_SYMBOL(ttm_bo_lru_cursor_first);
 
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 154d6739256f8..4a765b25472c3 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -714,6 +714,24 @@ ttm_resource_manager_next(struct ttm_resource_cursor *cursor)
 	return NULL;
 }
 
+/* TODO */
+struct ttm_resource *
+ttm_resource_manager_current(struct ttm_resource_cursor *cursor)
+{
+	struct ttm_resource_manager *man = cursor->man;
+	struct ttm_lru_item *lru;
+
+	lockdep_assert_held(&man->bdev->lru_lock);
+
+	lru = &cursor->hitch;
+	list_for_each_entry_continue_reverse(lru, &man->lru[cursor->priority],
+					     link) {
+		if (ttm_lru_item_is_res(lru))
+			return ttm_lru_item_to_res(lru);
+	}
+	return NULL;
+}
+
 /**
  * ttm_lru_first_res_or_null() - Return the first resource on an lru list
  * @head: The list head of the lru list.
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 8310bc3d55f90..30e835414e721 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -488,15 +488,13 @@ struct ttm_bo_lru_cursor {
 	/** @res_curs: Embedded struct ttm_resource_cursor. */
 	struct ttm_resource_cursor res_curs;
 	/**
-	 * @bo: Buffer object pointer if a buffer object is refcounted,
-	 * NULL otherwise.
+	 * @resv: reference to the locked dma_resv
 	 */
-	struct ttm_buffer_object *bo;
+	struct dma_resv *resv;
 	/**
-	 * @needs_unlock: Valid iff @bo != NULL. The bo resv needs
-	 * unlock before the next iteration or after loop exit.
+	 * @bo: TTM BO with GEM reference, NULL for deleted BOs
 	 */
-	bool needs_unlock;
+	struct ttm_buffer_object *bo;
 	/** @arg: Pointer to common BO LRU walk arguments. */
 	struct ttm_lru_walk_arg *arg;
 };
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index a5d386583fb6e..e8e9c8b81ce4b 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -488,6 +488,8 @@ struct ttm_resource *
 ttm_resource_manager_first(struct ttm_resource_cursor *cursor);
 struct ttm_resource *
 ttm_resource_manager_next(struct ttm_resource_cursor *cursor);
+struct ttm_resource *
+ttm_resource_manager_current(struct ttm_resource_cursor *cursor);
 
 struct ttm_resource *
 ttm_lru_first_res_or_null(struct list_head *head);
-- 
2.43.0