On 2026-10-30 10:05, David Francis wrote:
There was an amdgpu_vm_ready check in svm_range_unmap_from_gpu.
This is improper, because
- amdgpu_vm_ready checks for the vm to be locked, and unmap is
expected to occur without that lock held
- amdgpu_vm_ready fails if the vm is in the process of eviction,
but there's no need for unmap during eviction to be an error
Remove it (and swallow the -EBUSY error from eviction in progress)
This change is necessary to avoid dmesg spam from the vm lock
check; handling eviction may require additional checks
Signed-off-by: David Francis <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index ed5e3318877be..4c0ccd585ff8e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1425,6 +1425,7 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
{
uint64_t init_pte_value = adev->gmc.init_pte_flags;
uint64_t gpu_start, gpu_end;
+ int ret;
/* Convert CPU page range to GPU page range */
gpu_start = start * AMDGPU_GPU_PAGES_IN_CPU_PAGE;
@@ -1433,14 +1434,13 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
pr_debug("CPU[0x%llx 0x%llx] -> GPU[0x%llx 0x%llx]\n", start, last,
gpu_start, gpu_end);
- if (!amdgpu_vm_ready(vm)) {
- pr_debug("VM not ready, canceling unmap\n");
- return -EINVAL;
- }
-
- return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL,
gpu_start,
+ ret = amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL,
gpu_start,
gpu_end, init_pte_value, 0, 0, NULL, NULL,
fence);
+ /* EBUSY here means "vm not ready for updates because it in the process of
evicting."
+ * If that is true, it's safe to not update the range now since the vm
is not in use.
I guess this is true. But as we discovered, we'll need to do the unmap
or re-validation of the range eventually, before the VM is restored (in
restore_process_worker) and user mode queues restarted. It's not a new
bug, so we can do this in a later patch.
+ */
+ return ret == EBUSY ? 0 : ret;
This should be -EBUSY (negative sign). With that fixed, the patch is
Reviewed-by: Felix Kuehling <[email protected]>
}
static int
