Re: [PATCH 3/3] drm/amdkfd: Check cpu page faulted vmf->page got migrated to system RAM

["Chen, Xiaogang" <xiaogang.chen@amd.com>]

On 8/18/2026 4:36 PM, Felix Kuehling wrote:
> On 2026-08-17 09:59, Xiaogang.Chen wrote:
>> From: Xiaogang Chen <xiaogang.chen@amd.com>
>>
>> Cpu page fault hander __handle_mm_fault calls 
>> dev_pagemap_ops->migrate_to_ram
>> when faulted page is device private memory. This callback needs 
>> return either
>> success or vm_fault_t to let handler know what happened when handling 
>> the fault.
>>
>> Current driver returns success if it did not hit a hard error. That 
>> does not
>> means the device page is now in RAM. Some cases like fault page pinned,
>> lock fail or other mapping prevent faulted device page got migrated.
>>
>> Then the CPU instruction will fault again(retry loop), unless 
>> something else
>> changed the PTE.
>>
>> The patch explicitly checks if the faulted page vmf->page got 
>> migrated to
>> system RAM. If not, returns error to let cpu page fault core handler 
>> handle
>> the page fault in error path.
>
> Are you sure this is the right thing to do? If the errors are 
> transient in nature, then maybe retrying is the better option. What 
> happens if you return an error? Does the application die with a SIGBUS?
>
> Should we consult with an HMM maintainer here? Did you check how other 
> drivers with HMM support deal with this situation?

I notice this issue by reading code, not from real case. pinned 
/not-migratable vmf->page -> no copy from this page to sys-ram. It will 
trigger infinite retry,

I checked drm_pagemap_migrate_to_ram()(xe driver) and 
nouveau_dmem_migrate_to_ram(nouveau driver) that handle cpu page fault 
from device private memory access. They do same as amdgpu driver: skip 
pinned page or populate skips the slot that is (!(src[i] & 
MIGRATE_PFN_MIGRATE)), so those bytes are not copied, and the function 
returns 0.

For NVIDIA Linux open GPU kernel modules at 
https://github.com/NVIDIA/open-gpu-kernel-modules: It is more 
complicated. I asked Cursor analysis: NVIDIA UVM has the same 
CPU-visible outcome for a pinned vmf->page, but it does not return 
VM_FAULT_SIGBUS, return NV_WARN_MORE_PROCESSING_REQUIRED instead, then 
retry inside kernel.

This patch returns VM_FAULT_SIGBUS for that case, then fault handler 
sends SIGBUS signal, the default handler of this signal is killing the 
process. This may not be people want.

It is a blur area. kernel-doc for migrate_vma_setup():

  * If the caller cannot migrate a device page
  * back to system memory, then it must return VM_FAULT_SIGBUS, which 
has severe
  * consequences for the userspace process, so it must be avoided if at all
  * possible.

Summary:

Current xe/nouveau/amdgpu driver do not return VM_FAULT_SIGBUS, just 
think it is success. That may cause #PF handler retry loop between user 
and kernel space.

Nivida driver does retry inside kernel.

This patch detects the case then return  VM_FAULT_SIGBUS.


Maybe you said retry fault handler inside kernel is a better way: we let 
driver return VM_FAULT_RETRY for pinned vmf->page, let kernel retry 
handle_mm_fault, then hope the pin will be drop soon?


Regards

Xiaogang


>
> Regards,
>   Felix
>
>
>>
>> Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 28 +++++++++++++++++++-----
>>   1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> index 263dae49bb3e..656197dee9b1 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> @@ -582,8 +582,9 @@ static void svm_migrate_folio_free(struct folio 
>> *folio)
>>   static int
>>   svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct 
>> svm_range *prange,
>>               struct migrate_vma *migrate, struct dma_fence **mfence,
>> -            dma_addr_t *scratch, u64 npages)
>> +            dma_addr_t *scratch, u64 npages, bool *fault_handled)
>>   {
>> +    struct page *fault_page = migrate->fault_page;
>>       struct device *dev = adev->dev;
>>       struct page *dpage = NULL;
>>       dma_addr_t *dst;
>> @@ -646,6 +647,13 @@ svm_migrate_copy_to_ram(struct amdgpu_device 
>> *adev, struct svm_range *prange,
>>               goto out_oom;
>>           }
>>   +        /* if this spage is not migrated the function returns error
>> +         * final decide whether the fault got handled is decided by
>> +         * fault_handled and this function returned value
>> +         */
>> +        if (fault_handled && fault_page && fault_page == spage)
>> +            *fault_handled = true;
>> +
>>           pr_debug_ratelimited("dma mapping dst to 0x%llx, pfn 0x%lx\n",
>>                        dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
>>   @@ -701,6 +709,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device 
>> *adev, struct svm_range *prange,
>>    * @node: kfd node device to migrate from
>>    * @trigger: reason of migration
>>    * @fault_page: is from vmf->page, svm_migrate_to_ram(), this is 
>> CPU page fault callback
>> + * @fault_handled: whether CPU page fault got handled
>>    *
>>    * Context: Process context, caller hold mmap read lock, 
>> prange->migrate_mutex
>>    *
>> @@ -711,7 +720,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device 
>> *adev, struct svm_range *prange,
>>   static long
>>   svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range 
>> *prange,
>>                  struct vm_area_struct *vma, u64 start, u64 end,
>> -               uint32_t trigger, struct page *fault_page)
>> +               uint32_t trigger, struct page *fault_page, bool 
>> *fault_handled)
>>   {
>>       struct kfd_process *p = container_of(prange->svms, struct 
>> kfd_process, svms);
>>       u64 npages = (end - start) >> PAGE_SHIFT;
>> @@ -771,7 +780,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, 
>> struct svm_range *prange,
>>           pr_debug("0x%lx pages collected\n", cpages);
>>         r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
>> -                    scratch, npages);
>> +                    scratch, npages, fault_handled);
>>       migrate_vma_pages(&migrate);
>>         mpages = svm_migrate_successful_pages(&migrate);
>> @@ -816,8 +825,9 @@ int svm_migrate_vram_to_ram(struct svm_range 
>> *prange, struct mm_struct *mm,
>>                   unsigned long start_mgr, unsigned long last_mgr,
>>                   uint32_t trigger, struct page *fault_page)
>>   {
>> -    struct kfd_node *node;
>> +    bool fault_handled = false;
>>       struct vm_area_struct *vma;
>> +    struct kfd_node *node;
>>       unsigned long addr;
>>       unsigned long start;
>>       unsigned long end;
>> @@ -861,7 +871,7 @@ int svm_migrate_vram_to_ram(struct svm_range 
>> *prange, struct mm_struct *mm,
>>             next = min(vma->vm_end, end);
>>           r = svm_migrate_vma_to_ram(node, prange, vma, addr, next, 
>> trigger,
>> -            fault_page);
>> +            fault_page, &fault_handled);
>>           if (r < 0) {
>>               pr_debug("failed %ld to migrate prange %p\n", r, prange);
>>               break;
>> @@ -886,6 +896,10 @@ int svm_migrate_vram_to_ram(struct svm_range 
>> *prange, struct mm_struct *mm,
>>           }
>>       }
>>   +    /* need handle cpu page fault, but not handled */
>> +    if (fault_page && !fault_handled)
>> +        return -1;
>> +
>>       return r < 0 ? r : 0;
>>   }
>>   @@ -1027,7 +1041,9 @@ static vm_fault_t svm_migrate_to_ram(struct 
>> vm_fault *vmf)
>>   out_unlock_svms:
>>       mutex_unlock(&p->svms.lock);
>>   out_unref_process:
>> -    pr_debug("CPU fault svms 0x%p address 0x%lx done\n", &p->svms, 
>> addr);
>> +    pr_debug("CPU fault svms 0x%p address 0x%lx done with erro=%d\n",
>> +         &p->svms, addr, r);
>> +
>>       kfd_unref_process(p);
>>   out_mmput:
>>       mmput(mm);