When svm_range_needs_unmap() unmaps a range, retry faults queued in the soft IH ring before set_attr no-access was called must be dropped, since those faults arrived before the app chose to revoke GPU access and should not re-map the range.
Extract and add helper svm_range_update_checkpoint_timestamp() from svm_range_unmap_from_cpu() so both callers share the same logic. Change checkpoint_ts in svm_range_list from uint64_t to atomic64_t so svm_range_restore_pages() can read it safely from the page fault handler without holding the svms lock. Signed-off-by: Philip Yang <[email protected]> Acked-by: Kent Russell <[email protected]> --- drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 85 +++++++++++++++------------ 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index f00c522fba74..d3dcc3b8d546 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -893,7 +893,7 @@ struct svm_range_list { DECLARE_BITMAP(bitmap_supported, MAX_GPU_INSTANCE); struct task_struct *faulting_task; /* check point ts decides if page fault recovery need be dropped */ - uint64_t checkpoint_ts[MAX_GPU_INSTANCE]; + atomic64_t checkpoint_ts[MAX_GPU_INSTANCE]; /* Default granularity to use in buffer migration * and restoration of backing memory while handling diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index e039b6f2942f..acaa364244d0 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c @@ -759,6 +759,48 @@ svm_range_check_attr(struct kfd_process *p, return 0; } +static void svm_range_update_checkpoint_timestamp(struct kfd_process *p) +{ + struct svm_range_list *svms; + int i; + + svms = &p->svms; + + /* calculate time stamps that are used to decide which page faults need be + * dropped or handled before unmap pages from gpu vm + */ + for_each_set_bit(i, svms->bitmap_supported, p->n_pdds) { + struct kfd_process_device *pdd; + struct amdgpu_device *adev; + struct amdgpu_ih_ring *ih; + uint32_t checkpoint_wptr; + + pdd = p->pdds[i]; + if (!pdd) + continue; + + adev = pdd->dev->adev; + + /* Check and drain ih1 ring if cam not available */ + if (!adev->irq.retry_cam_enabled && adev->irq.ih1.ring_size) { + ih = &adev->irq.ih1; + checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); + if (ih->rptr != checkpoint_wptr) { + atomic64_set(&svms->checkpoint_ts[i], + amdgpu_ih_decode_iv_ts(adev, ih, checkpoint_wptr, -1)); + continue; + } + } + + /* check if dev->irq.ih_soft is not empty */ + ih = &adev->irq.ih_soft; + checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); + if (ih->rptr != checkpoint_wptr) + atomic64_set(&svms->checkpoint_ts[i], + amdgpu_ih_decode_iv_ts(adev, ih, checkpoint_wptr, -1)); + } +} + static void svm_range_apply_attrs(struct kfd_process *p, struct svm_range *prange, uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs, @@ -784,6 +826,8 @@ svm_range_apply_attrs(struct kfd_process *p, struct svm_range *prange, gpuidx = kfd_process_gpuidx_from_gpuid(p, attrs[i].value); if (attrs[i].type == KFD_IOCTL_SVM_ATTR_NO_ACCESS) { + svm_range_update_checkpoint_timestamp(p); + bitmap_clear(prange->bitmap_access, gpuidx, 1); bitmap_clear(prange->bitmap_aip, gpuidx, 1); bitmap_set(prange->bitmap_needs_unmap, gpuidx, 1); @@ -2560,7 +2604,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange, struct kfd_process *p; unsigned long s, l; bool unmap_parent; - uint32_t i; if (atomic_read(&prange->queue_refcount)) { int r; @@ -2580,38 +2623,7 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange, pr_debug("svms 0x%p prange 0x%p [0x%lx 0x%lx] [0x%lx 0x%lx]\n", svms, prange, prange->start, prange->last, start, last); - /* calculate time stamps that are used to decide which page faults need be - * dropped or handled before unmap pages from gpu vm - */ - for_each_set_bit(i, svms->bitmap_supported, p->n_pdds) { - struct kfd_process_device *pdd; - struct amdgpu_device *adev; - struct amdgpu_ih_ring *ih; - uint32_t checkpoint_wptr; - - pdd = p->pdds[i]; - if (!pdd) - continue; - - adev = pdd->dev->adev; - - /* Check and drain ih1 ring if cam not available */ - if (!adev->irq.retry_cam_enabled && adev->irq.ih1.ring_size) { - ih = &adev->irq.ih1; - checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); - if (ih->rptr != checkpoint_wptr) { - svms->checkpoint_ts[i] = - amdgpu_ih_decode_iv_ts(adev, ih, checkpoint_wptr, -1); - continue; - } - } - - /* check if dev->irq.ih_soft is not empty */ - ih = &adev->irq.ih_soft; - checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); - if (ih->rptr != checkpoint_wptr) - svms->checkpoint_ts[i] = amdgpu_ih_decode_iv_ts(adev, ih, checkpoint_wptr, -1); - } + svm_range_update_checkpoint_timestamp(p); unmap_parent = start <= prange->start && last >= prange->last; @@ -3121,8 +3133,9 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid, mutex_lock(&svms->lock); /* check if this page fault time stamp is before svms->checkpoint_ts */ - if (svms->checkpoint_ts[gpuidx] != 0) { - if (amdgpu_ih_ts_after_or_equal(ts, svms->checkpoint_ts[gpuidx])) { + if (atomic64_read(&svms->checkpoint_ts[gpuidx]) != 0) { + if (amdgpu_ih_ts_after_or_equal(ts, + atomic64_read(&svms->checkpoint_ts[gpuidx]))) { pr_debug("draining retry fault, drop fault 0x%llx\n", addr); if (write_locked) mmap_write_downgrade(mm); @@ -3132,7 +3145,7 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid, /* ts is after svms->checkpoint_ts now, reset svms->checkpoint_ts * to zero to avoid following ts wrap around give wrong comparing */ - svms->checkpoint_ts[gpuidx] = 0; + atomic64_set(&svms->checkpoint_ts[gpuidx], 0); } } -- 2.50.1
