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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.