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 <[email protected]> Reviewed-by: Felix Kuehling <[email protected]> --- 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
