Re: [PATCH 2/2] drm/amdkfd: add svm_migrate_successful_pages

[Philip Yang]

On 2025-05-28 13:19, James Zhu wrote:
to get migration pages. When migrating pages from system to vram,
needn't check bit MIGRATE_PFN_VALID, since the system page could
be allocated, but not be accessed.

I think the corner case is vram_pages becomes negative value when 
migrating prange from vram to system memory, if the src page is on 
system, src MIGRATE_PFN_MIGRATE set, MIGRATE_PFN_VALID not set, this 
count as successful migrate page by svm_migrate_unsuccessful_page.

As discussed offline, we could check dst MIGRATE_PFN_MIGRATE flag for 
both system to vram or vram to system cases.

Regards,

Philip


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

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index f0b690d4bb46..131838bf8751 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -260,17 +260,20 @@ static void svm_migrate_put_sys_page(unsigned long addr)
        put_page(page);
  }
  
-static unsigned long svm_migrate_unsuccessful_pages(struct migrate_vma *migrate)
+static unsigned long svm_migrate_successful_pages(struct migrate_vma *migrate)
  {
-       unsigned long upages = 0;
+       unsigned long mpages = 0;
        unsigned long i;
  
  	for (i = 0; i < migrate->npages; i++) {
-               if (migrate->src[i] & MIGRATE_PFN_VALID &&
-                   !(migrate->src[i] & MIGRATE_PFN_MIGRATE))
-                       upages++;
+                if (migrate->flags == MIGRATE_VMA_SELECT_SYSTEM &&
+                       migrate->src[i] & MIGRATE_PFN_MIGRATE)
+                        mpages++;
+                 else if (migrate->src[i] & MIGRATE_PFN_VALID &&
+                               migrate->src[i] & MIGRATE_PFN_MIGRATE)
+                        mpages++;
        }
-       return upages;
+       return mpages;
  }
  
  static int
@@ -447,7 +450,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 = cpages - svm_migrate_unsuccessful_pages(&migrate);
+       mpages = svm_migrate_successful_pages(&migrate);
        pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
                         mpages, cpages, migrate.npages);
  
@@ -688,7 +691,6 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
  {
        struct kfd_process *p = container_of(prange->svms, struct kfd_process, 
svms);
        uint64_t npages = (end - start) >> PAGE_SHIFT;
-       unsigned long upages = npages;
        unsigned long cpages = 0;
        unsigned long mpages = 0;
        struct amdgpu_device *adev = node->adev;
@@ -748,9 +750,9 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct 
svm_range *prange,
                                    scratch, npages);
        migrate_vma_pages(&migrate);
  
-	upages = svm_migrate_unsuccessful_pages(&migrate);
-       pr_debug("unsuccessful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
-                upages, cpages, migrate.npages);
+       mpages = svm_migrate_successful_pages(&migrate);
+       pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
+                mpages, cpages, migrate.npages);
  
  	svm_migrate_copy_done(adev, mfence);
        migrate_vma_finalize(&migrate);
@@ -764,7 +766,6 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct 
svm_range *prange,
                                    node->id, 0, trigger, r);
  out:
        if (!r && cpages) {
-               mpages = cpages - upages;
                pdd = svm_range_get_pdd_by_node(prange, node);
                if (pdd)
                        WRITE_ONCE(pdd->page_out, pdd->page_out + mpages);