Re: [PATCH v2 3/3] drm/amdkfd: free system struct pages when migration bit is cleared

[Philip Yang]

On 2025-09-08 12:15, James Zhu wrote:
if destination is on system ram. migrate_vma_pages can fail if a CPU
thread faults on the same page. However, the page table is locked and
only one of the new pages will be inserted. The device driver will see
that the MIGRATE_PFN_MIGRATE bit is cleared if it loses the race.

Signed-off-by: James Zhu <james....@amd.com>
---
  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 15 ++++++++++-----
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 83b9d019c885..eb43542896e0 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -260,15 +260,20 @@ static void svm_migrate_put_sys_page(unsigned long addr)
        put_page(page);
  }
  
-static unsigned long svm_migrate_successful_pages(struct migrate_vma *migrate)
+static unsigned long svm_migrate_successful_pages(struct migrate_vma *migrate,
+                                               bool dst_on_ram)

Use if (!(migrate.flags & MIGRATE_VMA_SELECT_SYSTEM)), don't add extra 
parameter.

Thanks for catching this system memory page leaking corner case.

Regards,

Philip

  {
        unsigned long mpages = 0;
        unsigned long i;
  
  	for (i = 0; i < migrate->npages; i++) {
-               if (migrate->dst[i] & MIGRATE_PFN_VALID &&
-                       migrate->src[i] & MIGRATE_PFN_MIGRATE)
+               if (migrate->dst[i] & MIGRATE_PFN_VALID) {
+                       if (migrate->src[i] & MIGRATE_PFN_MIGRATE) {
                                mpages++;
+                       } else if (dst_on_ram) {
+                               svm_migrate_put_sys_page(migrate->dst[i]);
+                               migrate->dst[i] = 0;
+                       }
                }
        }
        return mpages;
@@ -448,7 +453,7 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct 
svm_range *prange,
        svm_migrate_copy_done(adev, mfence);
        migrate_vma_finalize(&migrate);
  
-	mpages = svm_migrate_successful_pages(&migrate);
+       mpages = svm_migrate_successful_pages(&migrate, false);
        pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
                         mpages, cpages, migrate.npages);
  
@@ -748,7 +753,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
                                    scratch, npages);
        migrate_vma_pages(&migrate);
  
-	mpages = svm_migrate_successful_pages(&migrate);
+       mpages = svm_migrate_successful_pages(&migrate, true);
        pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
                mpages, cpages, migrate.npages);