From: Philip Yang <yangp@amd.com>
To: James Zhu <James.Zhu@amd.com>, amd-gfx@lists.freedesktop.org
Cc: Felix.kuehling@amd.com, philip.yang@amd.com,
chengjun.yao@amd.com, jamesz@amd.com
Subject: Re: [PATCH v2 1/3] Revert "drm/amdkfd: return migration pages from copy function"
Date: Tue, 9 Sep 2025 10:34:41 -0400 [thread overview]
Message-ID: <e8239092-65aa-7add-e9ff-1426b7dcc18e@amd.com> (raw)
In-Reply-To: <20250908161526.99413-1-James.Zhu@amd.com>
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
prev parent reply other threads:[~2025-09-09 14:34 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-08 16:15 [PATCH v2 1/3] Revert "drm/amdkfd: return migration pages from copy function" James Zhu
2025-09-08 16:15 ` [PATCH v2 2/3] drm/amdkfd: add function svm_migrate_successful_pages James Zhu
2025-09-09 14:46 ` Philip Yang
2025-09-09 20:42 ` [PATCH v3 " James Zhu
2025-09-10 19:56 ` Philip Yang
2025-09-08 16:15 ` [PATCH v2 3/3] drm/amdkfd: free system struct pages when migration bit is cleared James Zhu
2025-09-09 14:50 ` Philip Yang
2025-09-09 20:43 ` [PATCH v3 " James Zhu
2025-09-10 20:13 ` Philip Yang
2025-09-09 14:34 ` Philip Yang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e8239092-65aa-7add-e9ff-1426b7dcc18e@amd.com \
--to=yangp@amd.com \
--cc=Felix.kuehling@amd.com \
--cc=James.Zhu@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=chengjun.yao@amd.com \
--cc=jamesz@amd.com \
--cc=philip.yang@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox