[PATCH 1/1] drm/amdkfd: Migrate in CPU page fault use current mm

[Philip Yang]

On 2022-09-09 11:13, Felix Kuehling wrote:
> Am 2022-09-09 um 09:12 schrieb Philip Yang:
>> migrate_vma_setup shows below warning because we don't hold another
>> process mm mmap_lock. We should use current vmf->vma->vm_mm instead, the
>> caller already hold current mmap lock inside CPU page fault handler.
>>
>>   WARNING: CPU: 10 PID: 3054 at include/linux/mmap_lock.h:155 find_vma
>>   Call Trace:
>>    walk_page_range+0x76/0x150
>>    migrate_vma_setup+0x18a/0x640
>>    svm_migrate_vram_to_ram+0x245/0xa10 [amdgpu]
>>    svm_migrate_to_ram+0x36f/0x470 [amdgpu]
>>    do_swap_page+0xcfe/0xec0
>>    __handle_mm_fault+0x96b/0x15e0
>>    handle_mm_fault+0x13f/0x3e0
>>    do_user_addr_fault+0x1e7/0x690
>>
>> Fixes: 5e5bbf36a2c0 ("drm/amdkfd: handle CPU fault on COW mapping")
>> Signed-off-by: Philip Yang <Philip.Yang at amd.com>
>
> For a quick fix, this looks OK.
>
> Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
>
> For a better fix, I notice that svm_migrate_vram_to_ram only uses the 
> mm to look up the vma. But you already know the vma here, so that look 
> up is completely unnecessary. So could you just call 
> svm_migrate_vma_to_ram directly? Then you don't need the mm at all and 
> you save yourself an unnecessary vma lookup from the virtual address.

Thanks, call svm_migrate_vma_to_ram directly works, that was my first 
approach, but the prange we want to migrate may include multiple vma, it 
is safer to call svm_migrate_vram_to_ram to handle this case.

Regards,

Philip

>
> Regards,
>   Felix
>
>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> index f62c4561f0f4..1cfa4fcd28b3 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> @@ -947,7 +947,8 @@ static vm_fault_t svm_migrate_to_ram(struct 
>> vm_fault *vmf)
>>           goto out_unlock_prange;
>>       }
>>   -    r = svm_migrate_vram_to_ram(prange, mm, 
>> KFD_MIGRATE_TRIGGER_PAGEFAULT_CPU);
>> +    r = svm_migrate_vram_to_ram(prange, vmf->vma->vm_mm,
>> +                    KFD_MIGRATE_TRIGGER_PAGEFAULT_CPU);
>>       if (r)
>>           pr_debug("failed %d migrate svms 0x%p range 0x%p [0x%lx 
>> 0x%lx]\n",
>>                r, prange->svms, prange, prange->start, prange->last);