On 2026-10-06 14:02, Olivier Kaloudoff wrote:
On surprise removal the killed process's own exit path can fault on
its SVM managed ranges: SIGBUS -> coredump -> get_dump_page() ->
__get_user_pages() -> do_swap_page() -> svm_migrate_to_ram(). Without
an unplug check, svm_migrate_vram_to_ram() submits an SDMA copy job to
an already-killed drm_sched entity (observed as
"drm_sched_rq_add_entity: Trying to push to a killed entity" plus
refcount_t saturation warnings), then svm_migrate_copy_done() waits
uninterruptibly on a job fence that can never signal. The task dies in
D state holding svms->lock, which wedges svm_range_restore_work() on
the rwsem writer side, so kfd_processes_count never drains and the
pciehp removal thread wedges for good.
Fail fast instead of submitting work to a dead GPU. Check both
drm_dev_is_unplugged() and pci_dev_is_disconnected(): the PCI flag is
set as soon as the link drops, which can be well before
drm_dev_unplug() runs in the removal path.
- svm_migrate_vram_to_ram / svm_migrate_ram_to_vram: return -ENODEV
for a gone device, so the fault path turns it into VM_FAULT_SIGBUS
and the dying task can die
- svm_migrate_copy_done: skip the fence wait on a gone device, and
bound the wait to 10s otherwise to survive unplug racing in between
job submit and wait
- svm_range_restore_work: do not reschedule on -ENODEV, mirroring the
userptr restore worker fix
Verified on MI50 behind Thunderbolt: with these checks the workload
takes SIGBUS, coredump completes, kfd_processes_count drains in ~4s,
pciehp removal finishes, and the device re-enumerates cleanly on slot
re-power. Previously the same repro wedged the host.
Cc: Xiaogang Chen <[email protected]>
Signed-off-by: Olivier Kaloudoff <[email protected]>
---
New in v3.
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 30 +++++++++++++++++++++++-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +-
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 253365a..3568eb2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -202,7 +202,21 @@ svm_migrate_copy_done(struct amdgpu_device *adev, struct
dma_fence *mfence)
int r = 0;
if (mfence) {
- r = dma_fence_wait(mfence, false);
+ /* Job fences on a dead device never signal */
+ if (drm_dev_is_unplugged(adev_to_drm(adev)) ||
+ pci_dev_is_disconnected(adev->pdev)) {
+ r = -ENODEV;
+ } else {
+ long ret;
+
+ /* survive unplug between submit and wait */
+ ret = dma_fence_wait_timeout(mfence, false,
+ msecs_to_jiffies(10000));
I'd avoid arbitrary open-coded timeout values. Would
AMDGPU_FENCE_JIFFIES_TIMEOUT work? It's currently half a second.
+ if (!ret)
+ r = -ETIMEDOUT;
+ else if (ret < 0)
+ r = ret;
+ }
dma_fence_put(mfence);
pr_debug("sdma copy memory fence done\n");
}
@@ -507,6 +521,13 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t
best_loc,
pr_debug("failed to get kfd node by id 0x%x\n", best_loc);
return -ENODEV;
}
+ /* Fail the migration on a device that is gone for good */
+ if (drm_dev_is_unplugged(adev_to_drm(node->adev)) ||
+ pci_dev_is_disconnected(node->adev->pdev)) {
This seems racy. Should we use drm_dev_enter/exit instead?
+ pr_debug("gpu 0x%x unplugged, skip migration to vram\n",
+ best_loc);
+ return -ENODEV;
+ }
pr_debug("svms 0x%p [0x%lx 0x%lx] in [0x%lx 0x%lx] to gpu 0x%x\n",
prange->svms, start_mgr, last_mgr, prange->start, prange->last,
@@ -841,6 +862,13 @@ int svm_migrate_vram_to_ram(struct svm_range *prange,
struct mm_struct *mm,
pr_debug("failed to get kfd node by id 0x%x\n",
prange->actual_loc);
return -ENODEV;
}
+ /* Fail the migration on a device that is gone for good */
+ if (drm_dev_is_unplugged(adev_to_drm(node->adev)) ||
+ pci_dev_is_disconnected(node->adev->pdev)) {
Same as above.
+ pr_debug("gpu 0x%x unplugged, skip migration to ram\n",
+ prange->actual_loc);
+ return -ENODEV;
+ }
pr_debug("svms 0x%p prange 0x%p [0x%lx 0x%lx] from gpu 0x%x to ram\n",
prange->svms, prange, start_mgr, last_mgr,
prange->actual_loc);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index fa4054d..e209ed7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2014,7 +2014,7 @@ out_reschedule:
mutex_unlock(&process_info->lock);
/* If validation failed, reschedule another attempt */
- if (evicted_ranges) {
+ if (evicted_ranges && r != -ENODEV) {
I'm not convinced this is needed. cancel_delayed_work_sync in
svm_range_list_fini should take care of terminating the worker, even if
it rescheduled itself. It's more of an optimization to avoid
rescheduling the worker unnecessarily when it's known to be useless.
Either way, this would make more sense in patch 2, since it's similar to
the restore_userptr_worker change.
Regards,
Felix
pr_debug("reschedule to restore svm range\n");
queue_delayed_work(system_freezable_wq, &svms->restore_work,
msecs_to_jiffies(AMDGPU_SVM_RANGE_RESTORE_DELAY_MS));
