On 10/8/26 19:33, 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
> protection, 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.
> 
> Guard the hardware-accessing migration sections with
> drm_dev_enter()/drm_dev_exit() so unplug cannot proceed while a
> migration submits jobs and waits on them, and bound the job fence
> wait in svm_migrate_copy_done() with AMDGPU_FENCE_JIFFIES_TIMEOUT so
> a fence that can never signal does not stall the critical section
> (or the faulting task) indefinitely.
> 
> Verified on MI50 behind Thunderbolt: the workload takes SIGBUS,
> coredump completes, kfd_processes_count drains in ~7s, 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]>
> ---
> Changes in v5:
> - dropped drm_dev_is_unplugged()/pci_dev_is_disconnected() checks;
>   drm_dev_enter()/exit() alone is sufficient (Felix Kuehling)
> 
> Changes in v4:
> - use AMDGPU_FENCE_JIFFIES_TIMEOUT instead of an open-coded 10s
>   (Felix Kuehling)
> - guard migration sections with drm_dev_enter()/exit() instead of
>   racing flag checks (Felix Kuehling)
> - keep pci_dev_is_disconnected() early-out for the dead-link window
> - moved svm_range_restore_work() change to patch 2
> 
> Changes in v3:
> - new
> ---
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 253365a..14f2273 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -202,7 +202,17 @@ svm_migrate_copy_done(struct amdgpu_device *adev, struct 
> dma_fence *mfence)
>       int r = 0;
>  
>       if (mfence) {
> -             r = dma_fence_wait(mfence, false);
> +             /* Bound the wait: on a dying device the fence may never
> +              * signal.
> +              */

Fences *must* always signal especially when the device is hot removed.

> +             long ret;
> +
> +             ret = dma_fence_wait_timeout(mfence, false,
> +                             AMDGPU_FENCE_JIFFIES_TIMEOUT);
> +             if (!ret)
> +                     r = -ETIMEDOUT;
> +             else if (ret < 0)
> +                     r = ret;

That change makes no sense at all, a non interruptible wait with 
AMDGPU_FENCE_JIFFIES_TIMEOUT can never fail.

Regards,
Christian.

>               dma_fence_put(mfence);
>               pr_debug("sdma copy memory fence done\n");
>       }
> @@ -494,6 +504,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, 
> uint32_t best_loc,
>       u64 ttm_res_offset;
>       struct kfd_node *node;
>       unsigned long mpages = 0;
> +     int idx;
>       long r = 0;
>  
>       if (start_mgr < prange->start || last_mgr > prange->last) {
> @@ -507,6 +518,9 @@ 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;
>       }
> +     /* Block unplug while the migration submits jobs and waits on them */
> +     if (!drm_dev_enter(adev_to_drm(node->adev), &idx))
> +             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,
> @@ -521,6 +535,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, 
> uint32_t best_loc,
>                                       node->xcp ? node->xcp->id : 0);
>       if (r) {
>               dev_dbg(node->adev->dev, "failed to reserve VRAM, r: %ld\n", r);
> +             drm_dev_exit(idx);
>               return -ENOSPC;
>       }
>  
> @@ -565,6 +580,7 @@ out:
>                                       prange->npages * PAGE_SIZE,
>                                       KFD_IOC_ALLOC_MEM_FLAGS_VRAM,
>                                       node->xcp ? node->xcp->id : 0);
> +     drm_dev_exit(idx);
>       return r < 0 ? r : 0;
>  }
>  
> @@ -821,6 +837,7 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, 
> struct mm_struct *mm,
>       unsigned long start;
>       unsigned long end;
>       unsigned long mpages = 0;
> +     int idx;
>       long r = 0;
>  
>       /* this pragne has no any vram page to migrate to sys ram */
> @@ -841,6 +858,9 @@ 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;
>       }
> +     /* Block unplug while the migration submits jobs and waits on them */
> +     if (!drm_dev_enter(adev_to_drm(node->adev), &idx))
> +             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);
> @@ -885,6 +905,7 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, 
> struct mm_struct *mm,
>               }
>       }
>  
> +     drm_dev_exit(idx);
>       return r < 0 ? r : 0;
>  }
>  

Reply via email to