On 2026-10-05 03:44, Christian König wrote:
On 10/3/26 08:38, Olivier Kaloudoff wrote:
amdgpu_amdkfd_restore_userptr_worker() reschedules itself every
AMDGPU_USERPTR_RESTORE_DELAY_MS whenever validation of invalidated
userptr BOs fails. On PCIe surprise removal the VM update paths
already fail fast: amdgpu_vm_update_pdes() and amdgpu_vm_update_range()
return -ENODEV when drm_dev_enter() fails, which propagates up through
process_update_pds()/update_gpuvm_pte()/validate_invalid_user_pages().
The worker ignores the error code and reschedules unconditionally,
producing an endless retry loop on a device that is permanently gone.
The worker should get canceled in amdgpu_amdkfd_gpuvm_destroy_cb when
the VM is destroyed (when the render node device is closed as part of
process termination). So I'm not sure why we need a special case in the
restore worker for this.
Regards,
Felix
Observed on MI50 eGPU TB3 surprise removal: ~11k rescheduled restore
cycles, the worker holding reclaim-related locks until the system
soft-locked up.
Stop rescheduling when the validation failure is -ENODEV: the device
is gone for good and the affected processes are being torn down
anyway. Other errors keep the existing retry behavior, as they may be
transient.
Cc: Xiaogang Chen <[email protected]>
Signed-off-by: Olivier Kaloudoff <[email protected]>
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 22 ++++++++++++++-----
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index d668816..6c040ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -2827,6 +2827,7 @@ static void amdgpu_amdkfd_restore_userptr_worker(struct
work_struct *work)
struct task_struct *usertask;
struct mm_struct *mm;
uint32_t evicted_bos;
+ int ret = 0;
Please don't initialize return values when it isn't necessary.
Initialize it before the goto.
mutex_lock(&process_info->notifier_lock);
evicted_bos = process_info->evicted_bos;
@@ -2846,14 +2847,16 @@ static void amdgpu_amdkfd_restore_userptr_worker(struct
work_struct *work)
mutex_lock(&process_info->lock);
- if (update_invalid_user_pages(process_info, mm))
+ ret = update_invalid_user_pages(process_info, mm);
+ if (ret)
goto unlock_out;
/* userptr_inval_list can be empty if all evicted userptr BOs
* have been freed. In that case there is nothing to validate
* and we can just restart the queues.
*/
if (!list_empty(&process_info->userptr_inval_list)) {
- if (validate_invalid_user_pages(process_info))
+ ret = validate_invalid_user_pages(process_info);
+ if (ret)
goto unlock_out;
}
/* Final check for concurrent evicton and atomic update. If
@@ -2886,11 +2889,18 @@ unlock_out:
/* If validation failed, reschedule another attempt */
if (evicted_bos) {
- queue_delayed_work(system_freezable_wq,
- &process_info->restore_userptr_work,
- msecs_to_jiffies(AMDGPU_USERPTR_RESTORE_DELAY_MS));
+ if (ret == -ENODEV) {
+ /* Device is gone for good, stop retrying */
+ pr_warn_once("%s: device unplugged, aborting userptr restore
(pid %d, %s)\n",
+ __func__, pid_nr(process_info->pid),
+ usertask->comm);
That warning is most likely unnecessary and maybe even harmful.
That the device is hot removed should just be a perfectly normal abortion
condition for the worker.
Regards,
Christian.
+ } else {
+ queue_delayed_work(system_freezable_wq,
+ &process_info->restore_userptr_work,
+
msecs_to_jiffies(AMDGPU_USERPTR_RESTORE_DELAY_MS));
- kfd_smi_event_queue_restore_rescheduled(mm);
+ kfd_smi_event_queue_restore_rescheduled(mm);
+ }
}
mmput(mm);
put_task_struct(usertask);
