[PATCH 1/2] drm/amdkfd: Evict SVM BOs synchronously from TTM eviction

[Philip Yang <Philip.Yang@amd.com>]

svm_range_evict_svm_bo_worker() migrated an SVM BO's pages back to system
memory from a work item that took mmap_read_lock. When an mmap writer was
pending, that read lock blocked behind the writer while the thread
allocating a new migration VRAM BO waited on this BO's eviction fence - a
circular wait that hung the SVM workers.

Evict the SVM BO synchronously from the TTM eviction path
(amdgpu_ttm_bo_eviction_valuable) instead of deferring to a work item.
The BO is already reserved and the lock order is mmap_lock -> BO
reservation, so only trylock the owning process's mmap lock; on
contention return -EBUSY so TTM skips this BO. This removes the eviction
work item and the enable_signaling path, so no worker can block on
mmap_read_lock.

The SVM BO uses AMDGPU_GEM_CREATE_DISCARDABLE, so ttm_bo_evict takes the
pipeline_gutting path and skips allocating a system memory placement.
That would be wasted work, since svm_migrate_vram_to_ram allocates the
system pages and copies the data back itself.

Eviction now migrates ranges directly, so it must serialize with the
owning process: it trylocks migrate_mutex under svm_bo->list_lock before
unlinking the range, and svm_range_free() unlinks the range then waits on
migrate_mutex, so a concurrent eviction cannot free a range under it.

Drop the mm reference with mmput_async so exit_mmap() does not run under
the BO reservation.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    | 22 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c  |  3 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       | 16 ++++
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c          | 88 ++++++++++++-------
 drivers/gpu/drm/amd/amdkfd/kfd_svm.h          | 10 +--
 6 files changed, 100 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 121282dd30c1..be764b6802b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -39,6 +39,7 @@
 #if IS_ENABLED(CONFIG_HSA_AMD)
 #include "kfd_priv.h"
 #endif
+#include "kfd_svm.h"
 
 /* Total memory size in system memory and all GPU VRAM. Used to
  * estimate worst case amount of memory to reserve for page tables
@@ -972,3 +973,24 @@ int amdgpu_amdkfd_reset_mes_queue(struct amdgpu_device *adev,
 	return kgd2kfd_reset_mes_queue(adev->kfd.dev, node_id, queue_type,
 				       pipe, queue, db);
 }
+
+int amdgpu_amdkfd_evict_svm_bo(struct amdgpu_bo *bo)
+{
+	struct dma_resv_iter cursor;
+	struct dma_fence *fence;
+	int r = 0;
+
+	dma_resv_iter_begin(&cursor, bo->tbo.base.resv, DMA_RESV_USAGE_BOOKKEEP);
+	dma_resv_for_each_fence_unlocked(&cursor, fence) {
+		struct amdgpu_amdkfd_fence *f = to_amdgpu_amdkfd_fence(fence);
+
+		if (f && f->svm_bo) {
+			r = svm_range_evict_svm_bo(f->svm_bo);
+			if (r)
+				break;
+		}
+	}
+	dma_resv_iter_end(&cursor);
+
+	return r;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 338412a750ed..b4840ee36f2b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -286,6 +286,7 @@ int amdgpu_amdkfd_reset_mes_queue(struct amdgpu_device *adev,
 				  int queue_type,
 				  int pipe, int queue,
 				  unsigned int db);
+int amdgpu_amdkfd_evict_svm_bo(struct amdgpu_bo *bo);
 
 /* Read user wptr from a specified user address space with page fault
  * disabled. The memory must be pinned and mapped to the hardware when
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
index b0299d861903..553d26c2744e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
@@ -135,9 +135,6 @@ static bool amdkfd_fence_enable_signaling(struct dma_fence *f)
 	if (!fence->svm_bo) {
 		if (!kgd2kfd_schedule_evict_and_restore_process(fence->mm, fence->context_id, f))
 			return true;
-	} else {
-		if (!svm_range_schedule_evict_svm_bo(fence))
-			return true;
 	}
 	return false;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 03c1e5e3580c..746290216aae 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1488,6 +1488,7 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 					    const struct ttm_place *place)
 {
 	struct dma_resv_iter resv_cursor;
+	struct amdgpu_bo *abo;
 	struct dma_fence *f;
 
 	if (!amdgpu_bo_is_amdgpu_bo(bo))
@@ -1497,6 +1498,21 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 	if (bo->resource->mem_type == TTM_PL_SYSTEM)
 		return true;
 
+	abo = ttm_to_amdgpu_bo(bo);
+	if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
+		/*
+		 * SVM BOs are migrated to system memory synchronously in this
+		 * TTM eviction context. The migration needs the owning
+		 * process's mmap lock, but the normal lock order is
+		 * mmap_lock -> BO reservation and the BO is already reserved
+		 * here. svm_range_evict_svm_bo() only trylocks the mmap lock;
+		 * if the eviction fails for any reason, we return false so TTM
+		 * skips this BO instead of risking a deadlock.
+		 */
+		if (amdgpu_amdkfd_evict_svm_bo(abo) < 0)
+			return false;
+	}
+
 	if (bo->type == ttm_bo_type_kernel &&
 	    !amdgpu_vm_evictable(ttm_to_amdgpu_bo(bo)))
 		return false;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 30ad10bbd47e..4c6700c6e88d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -69,7 +69,6 @@ struct criu_svm_metadata {
 	struct kfd_criu_svm_range_priv_data data;
 };
 
-static void svm_range_evict_svm_bo_worker(struct work_struct *work);
 static bool
 svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
 				    const struct mmu_notifier_range *range,
@@ -97,7 +96,7 @@ static void svm_range_unlink(struct svm_range *prange)
 
 	if (prange->svm_bo) {
 		spin_lock(&prange->svm_bo->list_lock);
-		list_del(&prange->svm_bo_list);
+		list_del_init(&prange->svm_bo_list);
 		spin_unlock(&prange->svm_bo->list_lock);
 	}
 
@@ -286,6 +285,17 @@ static void svm_range_free(struct svm_range *prange, bool do_unmap)
 	pr_debug("svms 0x%p prange 0x%p [0x%lx 0x%lx]\n", prange->svms, prange,
 		 prange->start, prange->last);
 
+	/* Unlink from range_list; no-op if already unlinked. */
+	if (prange->svm_bo) {
+		spin_lock(&prange->svm_bo->list_lock);
+		list_del_init(&prange->svm_bo_list);
+		spin_unlock(&prange->svm_bo->list_lock);
+	}
+
+	/* Wait for any in-flight eviction of this range to finish. */
+	mutex_lock(&prange->migrate_mutex);
+	mutex_unlock(&prange->migrate_mutex);
+
 	svm_range_vram_node_free(prange);
 	if (do_unmap)
 		svm_range_dma_unmap(prange);
@@ -588,7 +598,6 @@ svm_range_vram_node_new(struct kfd_node *node, struct svm_range *prange,
 					   mm,
 					   svm_bo, p->context_id);
 	mmput(mm);
-	INIT_WORK(&svm_bo->eviction_work, svm_range_evict_svm_bo_worker);
 	svm_bo->evicting = 0;
 	memset(&bp, 0, sizeof(bp));
 	bp.size = prange->npages * PAGE_SIZE;
@@ -3631,39 +3640,36 @@ svm_range_trigger_migration(struct mm_struct *mm, struct svm_range *prange,
 	return 0;
 }
 
-int svm_range_schedule_evict_svm_bo(struct amdgpu_amdkfd_fence *fence)
-{
-	/* Dereferencing fence->svm_bo is safe here because the fence hasn't
-	 * signaled yet and we're under the protection of the fence->lock.
-	 * After the fence is signaled in svm_range_bo_release, we cannot get
-	 * here any more.
-	 *
-	 * Reference is dropped in svm_range_evict_svm_bo_worker.
-	 */
-	if (svm_bo_ref_unless_zero(fence->svm_bo)) {
-		WRITE_ONCE(fence->svm_bo->evicting, 1);
-		schedule_work(&fence->svm_bo->eviction_work);
-	}
-
-	return 0;
-}
-
-static void svm_range_evict_svm_bo_worker(struct work_struct *work)
+int svm_range_evict_svm_bo(struct svm_range_bo *svm_bo)
 {
-	struct svm_range_bo *svm_bo;
 	struct mm_struct *mm;
 	int r = 0;
 
-	svm_bo = container_of(work, struct svm_range_bo, eviction_work);
+	if (!svm_bo_ref_unless_zero(svm_bo))
+		return 0;
 
-	if (mmget_not_zero(svm_bo->eviction_fence->mm)) {
-		mm = svm_bo->eviction_fence->mm;
-	} else {
+	if (!mmget_not_zero(svm_bo->eviction_fence->mm)) {
 		svm_range_bo_unref(svm_bo);
-		return;
+		return 0;
 	}
+	mm = svm_bo->eviction_fence->mm;
+
+	/*
+	 * Called with the BO reserved; lock order is mmap_lock -> BO
+	 * reservation. Only trylock mmap to invert that order safely: a
+	 * trylock never blocks, so it cannot deadlock against the reservation
+	 * and lockdep records no reverse dependency. On contention return
+	 * -EBUSY so TTM skips this BO.
+	 */
+	if (!mmap_read_trylock(mm)) {
+		pr_debug("skip eviction, contended to take mmap_read lock\n");
+		mmput_async(mm);
+		svm_range_bo_unref(svm_bo);
+		return -EBUSY;
+	}
+
+	WRITE_ONCE(svm_bo->evicting, 1);
 
-	mmap_read_lock(mm);
 	spin_lock(&svm_bo->list_lock);
 	while (!list_empty(&svm_bo->range_list) && !r) {
 		struct svm_range *prange =
@@ -3671,13 +3677,24 @@ static void svm_range_evict_svm_bo_worker(struct work_struct *work)
 						struct svm_range, svm_bo_list);
 		int retries = 3;
 
+		/*
+		 * Trylock migrate_mutex under list_lock, before unlinking the
+		 * range, so svm_range_free() cannot free it under us. On
+		 * contention the owner is migrating this range; skip the BO.
+		 */
+		if (!mutex_trylock(&prange->migrate_mutex)) {
+			pr_debug("skip eviction, contended migrate_mutex\n");
+			/* Clear evicting so the BO keeps being reused. */
+			WRITE_ONCE(svm_bo->evicting, 0);
+			r = -EBUSY;
+			break;
+		}
 		list_del_init(&prange->svm_bo_list);
 		spin_unlock(&svm_bo->list_lock);
 
 		pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms,
 			 prange->start, prange->last);
 
-		mutex_lock(&prange->migrate_mutex);
 		do {
 			/* migrate all vram pages in this prange to sys ram
 			 * after that prange->actual_loc should be zero
@@ -3701,15 +3718,24 @@ static void svm_range_evict_svm_bo_worker(struct work_struct *work)
 	}
 	spin_unlock(&svm_bo->list_lock);
 	mmap_read_unlock(mm);
-	mmput(mm);
+	/* Defer mmput: exit_mmap() must not run under the BO reservation. */
+	mmput_async(mm);
 
-	dma_fence_signal(&svm_bo->eviction_fence->base);
+	/*
+	 * Only signal the eviction fence once the ranges have been processed.
+	 * On -EBUSY we bailed out without migrating; leave the BO in VRAM and
+	 * let TTM retry later.
+	 */
+	if (r != -EBUSY)
+		dma_fence_signal(&svm_bo->eviction_fence->base);
 
 	/* This is the last reference to svm_bo, after svm_range_vram_node_free
 	 * has been called in svm_migrate_vram_to_ram
 	 */
 	WARN_ONCE(!r && kref_read(&svm_bo->kref) != 1, "This was not the last reference\n");
 	svm_range_bo_unref(svm_bo);
+
+	return r;
 }
 
 static int
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index a63dfc95b602..4232c47422e6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -44,7 +44,6 @@ struct svm_range_bo {
 	struct list_head		range_list; /* all svm ranges shared this bo */
 	spinlock_t			list_lock;
 	struct amdgpu_amdkfd_fence	*eviction_fence;
-	struct work_struct		eviction_work;
 	uint32_t			evicting;
 	struct work_struct		release_work;
 	struct kfd_node			*node;
@@ -175,7 +174,8 @@ void svm_range_vram_node_free(struct svm_range *prange);
 int svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
 			    uint32_t vmid, uint32_t node_id, uint64_t addr, uint64_t ts,
 			    bool write_fault);
-int svm_range_schedule_evict_svm_bo(struct amdgpu_amdkfd_fence *fence);
+int svm_range_evict_svm_bo(struct svm_range_bo *svm_bo);
+
 void svm_range_add_list_work(struct svm_range_list *svms,
 			     struct svm_range *prange, struct mm_struct *mm,
 			     enum svm_work_list_ops op);
@@ -229,11 +229,9 @@ static inline int svm_range_restore_pages(struct amdgpu_device *adev,
 	return -EFAULT;
 }
 
-static inline int svm_range_schedule_evict_svm_bo(
-		struct amdgpu_amdkfd_fence *fence)
+static inline int svm_range_evict_svm_bo(struct svm_range_bo *svm_bo)
 {
-	WARN_ONCE(1, "SVM eviction fence triggered, but SVM is disabled");
-	return -EINVAL;
+	return 0;
 }
 
 static inline void svm_range_get_info(struct kfd_process *p,
-- 
2.50.1