[PATCH v2 1/3] Revert "drm/amdkfd: return migration pages from copy function"

[PATCH v2 1/3] Revert "drm/amdkfd: return migration pages from copy function"

[James Zhu]

This reverts commit cab1cec78c8fd52e014546739875a81150f11080.

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.
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 72 ++++++++++++------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 5d7eb0234002..f0b690d4bb46 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -260,7 +260,20 @@ static void svm_migrate_put_sys_page(unsigned long addr)
 	put_page(page);
 }
 
-static long
+static unsigned long svm_migrate_unsuccessful_pages(struct migrate_vma *migrate)
+{
+	unsigned long upages = 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++;
+	}
+	return upages;
+}
+
+static int
 svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 			 struct migrate_vma *migrate, struct dma_fence **mfence,
 			 dma_addr_t *scratch, uint64_t ttm_res_offset)
@@ -269,7 +282,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 	struct amdgpu_device *adev = node->adev;
 	struct device *dev = adev->dev;
 	struct amdgpu_res_cursor cursor;
-	long mpages;
+	uint64_t mpages = 0;
 	dma_addr_t *src;
 	uint64_t *dst;
 	uint64_t i, j;
@@ -283,7 +296,6 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 
 	amdgpu_res_first(prange->ttm_res, ttm_res_offset,
 			 npages << PAGE_SHIFT, &cursor);
-	mpages = 0;
 	for (i = j = 0; (i < npages) && (mpages < migrate->cpages); i++) {
 		struct page *spage;
 
@@ -344,14 +356,13 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 out_free_vram_pages:
 	if (r) {
 		pr_debug("failed %d to copy memory to vram\n", r);
-		while (i-- && mpages) {
+		for (i = 0; i < npages && mpages; i++) {
 			if (!dst[i])
 				continue;
 			svm_migrate_put_vram_page(adev, dst[i]);
 			migrate->dst[i] = 0;
 			mpages--;
 		}
-		mpages = r;
 	}
 
 #ifdef DEBUG_FORCE_MIXED_DOMAINS
@@ -369,7 +380,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 	}
 #endif
 
-	return mpages;
+	return r;
 }
 
 static long
@@ -384,7 +395,7 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct svm_range *prange,
 	struct dma_fence *mfence = NULL;
 	struct migrate_vma migrate = { 0 };
 	unsigned long cpages = 0;
-	long mpages = 0;
+	unsigned long mpages = 0;
 	dma_addr_t *scratch;
 	void *buf;
 	int r = -ENOMEM;
@@ -430,17 +441,15 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct svm_range *prange,
 	else
 		pr_debug("0x%lx pages collected\n", cpages);
 
-	mpages = svm_migrate_copy_to_vram(node, prange, &migrate, &mfence, scratch, ttm_res_offset);
+	r = svm_migrate_copy_to_vram(node, prange, &migrate, &mfence, scratch, ttm_res_offset);
 	migrate_vma_pages(&migrate);
 
 	svm_migrate_copy_done(adev, mfence);
 	migrate_vma_finalize(&migrate);
 
-	if (mpages >= 0)
-		pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
+	mpages = cpages - svm_migrate_unsuccessful_pages(&migrate);
+	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
 			 mpages, cpages, migrate.npages);
-	else
-		r = mpages;
 
 	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
 
@@ -450,13 +459,14 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct svm_range *prange,
 				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
 				    0, node->id, trigger, r);
 out:
-	if (!r && mpages > 0) {
+	if (!r && mpages) {
 		pdd = svm_range_get_pdd_by_node(prange, node);
 		if (pdd)
 			WRITE_ONCE(pdd->page_in, pdd->page_in + mpages);
-	}
 
-	return r ? r : mpages;
+		return mpages;
+	}
+	return r;
 }
 
 /**
@@ -567,7 +577,7 @@ static void svm_migrate_page_free(struct page *page)
 	}
 }
 
-static long
+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, uint64_t npages)
@@ -576,7 +586,6 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 	uint64_t *src;
 	dma_addr_t *dst;
 	struct page *dpage;
-	long mpages;
 	uint64_t i = 0, j;
 	uint64_t addr;
 	int r = 0;
@@ -589,7 +598,6 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 	src = (uint64_t *)(scratch + npages);
 	dst = scratch;
 
-	mpages = 0;
 	for (i = 0, j = 0; i < npages; i++, addr += PAGE_SIZE) {
 		struct page *spage;
 
@@ -638,7 +646,6 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 				     dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
 
 		migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
-		mpages++;
 		j++;
 	}
 
@@ -648,17 +655,13 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 out_oom:
 	if (r) {
 		pr_debug("failed %d copy to ram\n", r);
-		while (i-- && mpages) {
-			if (!migrate->dst[i])
-				continue;
+		while (i--) {
 			svm_migrate_put_sys_page(dst[i]);
 			migrate->dst[i] = 0;
-			mpages--;
 		}
-		mpages = r;
 	}
 
-	return mpages;
+	return r;
 }
 
 /**
@@ -685,8 +688,9 @@ 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;
-	long mpages = 0;
+	unsigned long mpages = 0;
 	struct amdgpu_device *adev = node->adev;
 	struct kfd_process_device *pdd;
 	struct dma_fence *mfence = NULL;
@@ -740,15 +744,13 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
 	else
 		pr_debug("0x%lx pages collected\n", cpages);
 
-	mpages = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
+	r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
 				    scratch, npages);
 	migrate_vma_pages(&migrate);
 
-	if (mpages >= 0)
-		pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
-		 mpages, cpages, migrate.npages);
-	else
-		r = mpages;
+	upages = svm_migrate_unsuccessful_pages(&migrate);
+	pr_debug("unsuccessful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
+		 upages, cpages, migrate.npages);
 
 	svm_migrate_copy_done(adev, mfence);
 	migrate_vma_finalize(&migrate);
@@ -761,7 +763,8 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
 				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
 				    node->id, 0, trigger, r);
 out:
-	if (!r && mpages > 0) {
+	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);
@@ -844,9 +847,6 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
 	}
 
 	if (r >= 0) {
-		WARN_ONCE(prange->vram_pages < mpages,
-			"Recorded vram pages(0x%llx) should not be less than migration pages(0x%lx).",
-			prange->vram_pages, mpages);
 		prange->vram_pages -= mpages;
 
 		/* prange does not have vram page set its actual_loc to system
-- 
2.34.1

[PATCH v2 2/3] drm/amdkfd: add function svm_migrate_successful_pages

[James Zhu]

to get migration pages. dst MIGRATE_PFN_VALID bit and src
MIGRATE_PFN_MIGRATE bit should always be set when migration success.

cpage includes src MIGRATE_PFN_MIGRATE bit set and MIGRATE_PFN_VALID
bit unset pages for both ram and vram when memory is only allocated
without being populated before migration, those ram pages should be
counted as migrate pages and those vram pages should not be counted
as migrate pages. Here migration pages refer to how many vram pages
invloved.

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

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index f0b690d4bb46..83b9d019c885 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -260,17 +260,18 @@ 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->dst[i] & MIGRATE_PFN_VALID &&
+			migrate->src[i] & MIGRATE_PFN_MIGRATE)
+				mpages++;
+		}
 	}
-	return upages;
+	return mpages;
 }
 
 static int
@@ -447,8 +448,8 @@ 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);
-	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
+	mpages = svm_migrate_successful_pages(&migrate);
+	pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
 			 mpages, cpages, migrate.npages);
 
 	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
@@ -688,7 +689,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 +748,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("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
+		mpages, cpages, migrate.npages);
 
 	svm_migrate_copy_done(adev, mfence);
 	migrate_vma_finalize(&migrate);
@@ -763,8 +763,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
 				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
 				    node->id, 0, trigger, r);
 out:
-	if (!r && cpages) {
-		mpages = cpages - upages;
+	if (!r && mpages) {
 		pdd = svm_range_get_pdd_by_node(prange, node);
 		if (pdd)
 			WRITE_ONCE(pdd->page_out, pdd->page_out + mpages);
@@ -847,6 +846,9 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
 	}
 
 	if (r >= 0) {
+		WARN_ONCE(prange->vram_pages < mpages,
+			"Recorded vram pages(0x%llx) should not be less than migration pages(0x%lx).",
+			prange->vram_pages, mpages);
 		prange->vram_pages -= mpages;
 
 		/* prange does not have vram page set its actual_loc to system
-- 
2.34.1

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

[James Zhu]

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.Zhu@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)
 {
 	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);
 
-- 
2.34.1

Re: [PATCH v2 1/3] Revert "drm/amdkfd: return migration pages from copy function"

[Philip Yang]

On 2025-09-08 12:15, James Zhu wrote:
> This reverts commit cab1cec78c8fd52e014546739875a81150f11080.
>
> 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.

Missing Signed-off-by tag, with tag added, this patch is

Reviewed-by: Philip Yang <Philip.Yang@amd.com>

> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 72 ++++++++++++------------
>   1 file changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 5d7eb0234002..f0b690d4bb46 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -260,7 +260,20 @@ static void svm_migrate_put_sys_page(unsigned long addr)
>   	put_page(page);
>   }
>   
> -static long
> +static unsigned long svm_migrate_unsuccessful_pages(struct migrate_vma *migrate)
> +{
> +	unsigned long upages = 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++;
> +	}
> +	return upages;
> +}
> +
> +static int
>   svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   			 struct migrate_vma *migrate, struct dma_fence **mfence,
>   			 dma_addr_t *scratch, uint64_t ttm_res_offset)
> @@ -269,7 +282,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   	struct amdgpu_device *adev = node->adev;
>   	struct device *dev = adev->dev;
>   	struct amdgpu_res_cursor cursor;
> -	long mpages;
> +	uint64_t mpages = 0;
>   	dma_addr_t *src;
>   	uint64_t *dst;
>   	uint64_t i, j;
> @@ -283,7 +296,6 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   
>   	amdgpu_res_first(prange->ttm_res, ttm_res_offset,
>   			 npages << PAGE_SHIFT, &cursor);
> -	mpages = 0;
>   	for (i = j = 0; (i < npages) && (mpages < migrate->cpages); i++) {
>   		struct page *spage;
>   
> @@ -344,14 +356,13 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   out_free_vram_pages:
>   	if (r) {
>   		pr_debug("failed %d to copy memory to vram\n", r);
> -		while (i-- && mpages) {
> +		for (i = 0; i < npages && mpages; i++) {
>   			if (!dst[i])
>   				continue;
>   			svm_migrate_put_vram_page(adev, dst[i]);
>   			migrate->dst[i] = 0;
>   			mpages--;
>   		}
> -		mpages = r;
>   	}
>   
>   #ifdef DEBUG_FORCE_MIXED_DOMAINS
> @@ -369,7 +380,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   	}
>   #endif
>   
> -	return mpages;
> +	return r;
>   }
>   
>   static long
> @@ -384,7 +395,7 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct svm_range *prange,
>   	struct dma_fence *mfence = NULL;
>   	struct migrate_vma migrate = { 0 };
>   	unsigned long cpages = 0;
> -	long mpages = 0;
> +	unsigned long mpages = 0;
>   	dma_addr_t *scratch;
>   	void *buf;
>   	int r = -ENOMEM;
> @@ -430,17 +441,15 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct svm_range *prange,
>   	else
>   		pr_debug("0x%lx pages collected\n", cpages);
>   
> -	mpages = svm_migrate_copy_to_vram(node, prange, &migrate, &mfence, scratch, ttm_res_offset);
> +	r = svm_migrate_copy_to_vram(node, prange, &migrate, &mfence, scratch, ttm_res_offset);
>   	migrate_vma_pages(&migrate);
>   
>   	svm_migrate_copy_done(adev, mfence);
>   	migrate_vma_finalize(&migrate);
>   
> -	if (mpages >= 0)
> -		pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
> +	mpages = cpages - svm_migrate_unsuccessful_pages(&migrate);
> +	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
>   			 mpages, cpages, migrate.npages);
> -	else
> -		r = mpages;
>   
>   	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
>   
> @@ -450,13 +459,14 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct svm_range *prange,
>   				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
>   				    0, node->id, trigger, r);
>   out:
> -	if (!r && mpages > 0) {
> +	if (!r && mpages) {
>   		pdd = svm_range_get_pdd_by_node(prange, node);
>   		if (pdd)
>   			WRITE_ONCE(pdd->page_in, pdd->page_in + mpages);
> -	}
>   
> -	return r ? r : mpages;
> +		return mpages;
> +	}
> +	return r;
>   }
>   
>   /**
> @@ -567,7 +577,7 @@ static void svm_migrate_page_free(struct page *page)
>   	}
>   }
>   
> -static long
> +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, uint64_t npages)
> @@ -576,7 +586,6 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   	uint64_t *src;
>   	dma_addr_t *dst;
>   	struct page *dpage;
> -	long mpages;
>   	uint64_t i = 0, j;
>   	uint64_t addr;
>   	int r = 0;
> @@ -589,7 +598,6 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   	src = (uint64_t *)(scratch + npages);
>   	dst = scratch;
>   
> -	mpages = 0;
>   	for (i = 0, j = 0; i < npages; i++, addr += PAGE_SIZE) {
>   		struct page *spage;
>   
> @@ -638,7 +646,6 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   				     dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
>   
>   		migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
> -		mpages++;
>   		j++;
>   	}
>   
> @@ -648,17 +655,13 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   out_oom:
>   	if (r) {
>   		pr_debug("failed %d copy to ram\n", r);
> -		while (i-- && mpages) {
> -			if (!migrate->dst[i])
> -				continue;
> +		while (i--) {
>   			svm_migrate_put_sys_page(dst[i]);
>   			migrate->dst[i] = 0;
> -			mpages--;
>   		}
> -		mpages = r;
>   	}
>   
> -	return mpages;
> +	return r;
>   }
>   
>   /**
> @@ -685,8 +688,9 @@ 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;
> -	long mpages = 0;
> +	unsigned long mpages = 0;
>   	struct amdgpu_device *adev = node->adev;
>   	struct kfd_process_device *pdd;
>   	struct dma_fence *mfence = NULL;
> @@ -740,15 +744,13 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
>   	else
>   		pr_debug("0x%lx pages collected\n", cpages);
>   
> -	mpages = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
> +	r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
>   				    scratch, npages);
>   	migrate_vma_pages(&migrate);
>   
> -	if (mpages >= 0)
> -		pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
> -		 mpages, cpages, migrate.npages);
> -	else
> -		r = mpages;
> +	upages = svm_migrate_unsuccessful_pages(&migrate);
> +	pr_debug("unsuccessful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
> +		 upages, cpages, migrate.npages);
>   
>   	svm_migrate_copy_done(adev, mfence);
>   	migrate_vma_finalize(&migrate);
> @@ -761,7 +763,8 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
>   				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
>   				    node->id, 0, trigger, r);
>   out:
> -	if (!r && mpages > 0) {
> +	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);
> @@ -844,9 +847,6 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
>   	}
>   
>   	if (r >= 0) {
> -		WARN_ONCE(prange->vram_pages < mpages,
> -			"Recorded vram pages(0x%llx) should not be less than migration pages(0x%lx).",
> -			prange->vram_pages, mpages);
>   		prange->vram_pages -= mpages;
>   
>   		/* prange does not have vram page set its actual_loc to system

Re: [PATCH v2 2/3] drm/amdkfd: add function svm_migrate_successful_pages

[Philip Yang]

On 2025-09-08 12:15, James Zhu wrote:
> to get migration pages. dst MIGRATE_PFN_VALID bit and src
> MIGRATE_PFN_MIGRATE bit should always be set when migration success.
>
> cpage includes src MIGRATE_PFN_MIGRATE bit set and MIGRATE_PFN_VALID
> bit unset pages for both ram and vram when memory is only allocated
> without being populated before migration, those ram pages should be
> counted as migrate pages and those vram pages should not be counted
> as migrate pages. Here migration pages refer to how many vram pages
> invloved.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 30 +++++++++++++-----------
>   1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index f0b690d4bb46..83b9d019c885 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -260,17 +260,18 @@ 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->dst[i] & MIGRATE_PFN_VALID &&
> +			migrate->src[i] & MIGRATE_PFN_MIGRATE)
> +				mpages++;
> +		}
>   	}
> -	return upages;
> +	return mpages;
>   }
>   

To fix the incorrect page counting, maybe add this check in 
svm_migrate_unsuccessful_pages, I fell this is easier to understand than 
the condition added in svm_migrate_successful_pages

@@ -278,6 +278,15 @@ static unsigned long 
svm_migrate_unsuccessful_pages(struct migrate_vma *migrate)
                 if (migrate->src[i] & MIGRATE_PFN_VALID &&
                     !(migrate->src[i] & MIGRATE_PFN_MIGRATE))
                         upages++;
+               /*
+                * if migrating from vram to ram, don't count device pages
+                * which are not populated, this could happen if svm_bo is
+                * evicted.
+                */
+               if (!(migrate.flags & MIGRATE_VMA_SELECT_SYSTEM) &&
+                   !(migrate->src[i] & MIGRATE_PFN_VALID) &&
+                   migrate->src[i] & MIGRATE_PFN_MIGRATE)
+                       upages++;
         }
         return upages;

>   static int
> @@ -447,8 +448,8 @@ 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);
> -	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
> +	mpages = svm_migrate_successful_pages(&migrate);
> +	pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
>   			 mpages, cpages, migrate.npages);
>   
>   	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
> @@ -688,7 +689,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 +748,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("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
> +		mpages, cpages, migrate.npages);
>   
>   	svm_migrate_copy_done(adev, mfence);
>   	migrate_vma_finalize(&migrate);
> @@ -763,8 +763,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
>   				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
>   				    node->id, 0, trigger, r);
>   out:
> -	if (!r && cpages) {
> -		mpages = cpages - upages;
> +	if (!r && mpages) {
>   		pdd = svm_range_get_pdd_by_node(prange, node);
>   		if (pdd)
>   			WRITE_ONCE(pdd->page_out, pdd->page_out + mpages);
> @@ -847,6 +846,9 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
>   	}
>   
>   	if (r >= 0) {
> +		WARN_ONCE(prange->vram_pages < mpages,
> +			"Recorded vram pages(0x%llx) should not be less than migration pages(0x%lx).",
> +			prange->vram_pages, mpages);

It is good to add warning once here, should we also change u64 
prange->vram_pages to 0 for this case, otherwise this could leak svm_bo 
as prange->vram_pages never become 0?

Regards,

Philip

>   		prange->vram_pages -= mpages;
>   
>   		/* prange does not have vram page set its actual_loc to system

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.Zhu@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);
>   

[PATCH v3 2/3] drm/amdkfd: add function svm_migrate_successful_pages

[James Zhu]

to get migration pages. dst MIGRATE_PFN_VALID bit and src
MIGRATE_PFN_MIGRATE bit should always be set when migration success.

cpage includes src MIGRATE_PFN_MIGRATE bit set and MIGRATE_PFN_VALID
bit unset pages for both RAM and VRAM when memory is only allocated
without being populated before migration, those ram pages should be
counted as migrated pages and those vram pages should not be counted
as migrated pages. Here migration pages refer to how many vram pages
invloved. Current svm_migrate_unsuccessful_pages only covers the
unsuccessful case that source is on RAM.

So far, we only see two unsuccessful migration cases. Since we
can clearly identify successful migration cases through dst
MIGRATE_PFN_VALID bit and src MIGRATE_PFN_MIGRATE bit within this
prange, also eventually successful migration pages will be used,
so we can use function svm_migrate_successful_pages to replace
function svm_migrate_unsuccessful_pages.

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

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index f0b690d4bb46..10e787e47191 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -260,17 +260,18 @@ 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->dst[i] & MIGRATE_PFN_VALID &&
+			migrate->src[i] & MIGRATE_PFN_MIGRATE)
+			mpages++;
+		}
 	}
-	return upages;
+	return mpages;
 }
 
 static int
@@ -447,8 +448,8 @@ 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);
-	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
+	mpages = svm_migrate_successful_pages(&migrate);
+	pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
 			 mpages, cpages, migrate.npages);
 
 	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
@@ -688,7 +689,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 +748,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("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
+		mpages, cpages, migrate.npages);
 
 	svm_migrate_copy_done(adev, mfence);
 	migrate_vma_finalize(&migrate);
@@ -763,8 +763,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
 				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
 				    node->id, 0, trigger, r);
 out:
-	if (!r && cpages) {
-		mpages = cpages - upages;
+	if (!r && mpages) {
 		pdd = svm_range_get_pdd_by_node(prange, node);
 		if (pdd)
 			WRITE_ONCE(pdd->page_out, pdd->page_out + mpages);
@@ -847,6 +846,9 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
 	}
 
 	if (r >= 0) {
+		WARN_ONCE(prange->vram_pages < mpages,
+			"Recorded vram pages(0x%llx) should not be less than migration pages(0x%lx).",
+			prange->vram_pages, mpages);
 		prange->vram_pages -= mpages;
 
 		/* prange does not have vram page set its actual_loc to system
-- 
2.34.1

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

[James Zhu]

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.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 10e787e47191..1a30764aa91b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -266,9 +266,13 @@ static unsigned long svm_migrate_successful_pages(struct migrate_vma *migrate)
 	unsigned long i;
 
 	for (i = 0; i < migrate->npages; i++) {
-		if (migrate->dst[i] & MIGRATE_PFN_VALID &&
-			migrate->src[i] & MIGRATE_PFN_MIGRATE)
-			mpages++;
+		if (migrate->dst[i] & MIGRATE_PFN_VALID) {
+			if (migrate->src[i] & MIGRATE_PFN_MIGRATE) {
+				mpages++;
+			} else if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) {
+				svm_migrate_put_sys_page(migrate->dst[i]);
+				migrate->dst[i] = 0;
+			}
 		}
 	}
 	return mpages;
-- 
2.34.1

Re: [PATCH v3 2/3] drm/amdkfd: add function svm_migrate_successful_pages

[Philip Yang]

On 2025-09-09 16:42, James Zhu wrote:
> to get migration pages. dst MIGRATE_PFN_VALID bit and src
> MIGRATE_PFN_MIGRATE bit should always be set when migration success.
>
> cpage includes src MIGRATE_PFN_MIGRATE bit set and MIGRATE_PFN_VALID
> bit unset pages for both RAM and VRAM when memory is only allocated
> without being populated before migration, those ram pages should be
> counted as migrated pages and those vram pages should not be counted
> as migrated pages. Here migration pages refer to how many vram pages
> invloved. Current svm_migrate_unsuccessful_pages only covers the
> unsuccessful case that source is on RAM.
>
> So far, we only see two unsuccessful migration cases. Since we
> can clearly identify successful migration cases through dst
> MIGRATE_PFN_VALID bit and src MIGRATE_PFN_MIGRATE bit within this
> prange, also eventually successful migration pages will be used,
> so we can use function svm_migrate_successful_pages to replace
> function svm_migrate_unsuccessful_pages.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 30 +++++++++++++-----------
>   1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index f0b690d4bb46..10e787e47191 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -260,17 +260,18 @@ 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->dst[i] & MIGRATE_PFN_VALID &&
> +			migrate->src[i] & MIGRATE_PFN_MIGRATE)
incorrect indent
> +			mpages++;
> +		}

this is extra braces, you should see the compile error w/o patch 3/3.

with those fixed, this patch is

Reviewed-by: Philip Yang<Philip.Yang@amd.com>

>   	}
> -	return upages;
> +	return mpages;
>   }
>   
>   static int
> @@ -447,8 +448,8 @@ 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);
> -	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
> +	mpages = svm_migrate_successful_pages(&migrate);
> +	pr_debug("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
>   			 mpages, cpages, migrate.npages);
>   
>   	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
> @@ -688,7 +689,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 +748,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("migrated/collected/requested 0x%lx/0x%lx/0x%lx\n",
> +		mpages, cpages, migrate.npages);
>   
>   	svm_migrate_copy_done(adev, mfence);
>   	migrate_vma_finalize(&migrate);
> @@ -763,8 +763,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
>   				    start >> PAGE_SHIFT, end >> PAGE_SHIFT,
>   				    node->id, 0, trigger, r);
>   out:
> -	if (!r && cpages) {
> -		mpages = cpages - upages;
> +	if (!r && mpages) {
>   		pdd = svm_range_get_pdd_by_node(prange, node);
>   		if (pdd)
>   			WRITE_ONCE(pdd->page_out, pdd->page_out + mpages);
> @@ -847,6 +846,9 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
>   	}
>   
>   	if (r >= 0) {
> +		WARN_ONCE(prange->vram_pages < mpages,
> +			"Recorded vram pages(0x%llx) should not be less than migration pages(0x%lx).",
> +			prange->vram_pages, mpages);
>   		prange->vram_pages -= mpages;
>   
>   		/* prange does not have vram page set its actual_loc to system

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

[Philip Yang]

On 2025-09-09 16:43, 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.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 10e787e47191..1a30764aa91b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -266,9 +266,13 @@ static unsigned long svm_migrate_successful_pages(struct migrate_vma *migrate)
>   	unsigned long i;
>   
>   	for (i = 0; i < migrate->npages; i++) {
> -		if (migrate->dst[i] & MIGRATE_PFN_VALID &&
> -			migrate->src[i] & MIGRATE_PFN_MIGRATE)
> -			mpages++;
> +		if (migrate->dst[i] & MIGRATE_PFN_VALID) {
> +			if (migrate->src[i] & MIGRATE_PFN_MIGRATE) {
> +				mpages++;
> +			} else if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) {

just notice migrate.flags is only available #ifdef 
HAVE_MIGRATE_VMA_PGMAP_OWNER, check if this is system page instead

        if (!is_zone_device_page(pfn_to_page(migrate->dst[i])))

Regards,

Philip

> +				svm_migrate_put_sys_page(migrate->dst[i]);
> +				migrate->dst[i] = 0;
> +			}
>   		}
>   	}
>   	return mpages;