<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 2023-10-19 12:20, Chen, Xiaogang
wrote:<br>
</div>
<blockquote type="cite" cite="mid:56bbfe3b-38ec-22c3-4f57-61c409bc7c6c@amd.com">
<br>
On 10/19/2023 11:08 AM, Philip Yang wrote:
<br>
<blockquote type="cite">
<br>
<br>
On 2023-10-19 10:24, Xiaogang.Chen wrote:
<br>
<blockquote type="cite">From: Xiaogang
Chen<a class="moz-txt-link-rfc2396E" href="mailto:xiaogang.chen@amd.com"><xiaogang.chen@amd.com></a>
<br>
<br>
After partial migration to recover GPU page fault this patch
does GPU vm
<br>
space mapping for same page range that got migrated instead of
mapping all
<br>
pages of svm range in which the page fault happened.
<br>
<br>
Signed-off-by: Xiaogang Chen<a class="moz-txt-link-rfc2396E" href="mailto:Xiaogang.Chen@amd.com"><Xiaogang.Chen@amd.com></a>
<br>
---
<br>
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 33
+++++++++++++++++++++-------
<br>
1 file changed, 25 insertions(+), 8 deletions(-)
<br>
<br>
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
<br>
index 54af7a2b29f8..81dbcc8a4ccc 100644
<br>
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
<br>
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
<br>
@@ -1619,6 +1619,7 @@ static void *kfd_svm_page_owner(struct
kfd_process *p, int32_t gpuidx)
<br>
* 5. Release page table (and SVM BO) reservation
<br>
*/
<br>
static int svm_range_validate_and_map(struct mm_struct *mm,
<br>
+ unsigned long map_start, unsigned long
map_last,
<br>
struct svm_range *prange, int32_t
gpuidx,
<br>
bool intr, bool wait, bool flush_tlb)
<br>
{
<br>
@@ -1630,6 +1631,12 @@ static int
svm_range_validate_and_map(struct mm_struct *mm,
<br>
int32_t idx;
<br>
int r = 0;
<br>
+ if (map_start < prange->start || map_last >
prange->last) {
<br>
</blockquote>
This is not needed, as this case should never happen, and you
also use max/min to limit map_start, map_last below.
<br>
</blockquote>
This was just a sanity check, I can remove it.
<br>
<blockquote type="cite">
<blockquote type="cite">+ pr_debug("range [0x%lx 0x%lx]
out prange [0x%lx 0x%lx]\n",
<br>
+ map_start, map_last, prange->start,
prange->last);
<br>
+ return -EFAULT;
<br>
+ }
<br>
+
<br>
ctx = kzalloc(sizeof(struct svm_validate_context),
GFP_KERNEL);
<br>
if (!ctx)
<br>
return -ENOMEM;
<br>
@@ -1747,9 +1754,16 @@ static int
svm_range_validate_and_map(struct mm_struct *mm,
<br>
r = -EAGAIN;
<br>
}
<br>
- if (!r)
<br>
- r = svm_range_map_to_gpus(prange, offset, npages,
readonly,
<br>
- ctx->bitmap, wait, flush_tlb);
<br>
+ if (!r) {
<br>
+ map_start = max(map_start, prange->start +
offset);
<br>
+ map_last = min(map_last, prange->start +
offset + npages);
<br>
</blockquote>
<br>
This should move forward to outside the for loop, otherwise
amdgpu_hmm_range_get_pages and svm_range_dma_map still work on
the entire prange, and then prange->vram_pages update logic
should be changed accordingly.
<br>
<br>
</blockquote>
We need use hmm function to get all vram page number on whole
range as we did not know how many vram pages after partial
migration, then we know if can release vram bo.
<br>
</blockquote>
<p>ok,migrate to vram and migrate to ram have the vram pages updated
already, the is needed for the splite prange. We could update
prange->vram_pages when splitting prange, this can be done in
another patch.</p>
<p>map_last is inclusive,<br>
</p>
<p>+ map_last = min(map_last, prange->start + offset +
npages - 1); <br>
</p>
<p>Regards,</p>
<p>Philip<br>
</p>
<blockquote type="cite" cite="mid:56bbfe3b-38ec-22c3-4f57-61c409bc7c6c@amd.com">
<br>
Regards
<br>
<br>
Xiaogang
<br>
<br>
<blockquote type="cite">Regards,
<br>
<br>
Philip
<br>
<br>
<blockquote type="cite">+ if (map_start <=
map_last) {
<br>
+ offset = map_start - prange->start;
<br>
+ npages = map_last - map_start + 1;
<br>
+ r = svm_range_map_to_gpus(prange, offset,
npages, readonly,
<br>
+ ctx->bitmap, wait,
flush_tlb);
<br>
+ }
<br>
+ }
<br>
if (!r && next == end)
<br>
prange->mapped_to_gpu = true;
<br>
@@ -1855,8 +1869,8 @@ static void
svm_range_restore_work(struct work_struct *work)
<br>
*/
<br>
mutex_lock(&prange->migrate_mutex);
<br>
- r = svm_range_validate_and_map(mm, prange,
MAX_GPU_INSTANCE,
<br>
- false, true, false);
<br>
+ r = svm_range_validate_and_map(mm, prange->start,
prange->last, prange,
<br>
+ MAX_GPU_INSTANCE, false, true,
false);
<br>
if (r)
<br>
pr_debug("failed %d to map 0x%lx to gpus\n", r,
<br>
prange->start);
<br>
@@ -3069,6 +3083,8 @@ svm_range_restore_pages(struct
amdgpu_device *adev, unsigned int pasid,
<br>
kfd_smi_event_page_fault_start(node,
p->lead_thread->pid, addr,
<br>
write_fault, timestamp);
<br>
+ start = prange->start;
<br>
+ last = prange->last;
<br>
if (prange->actual_loc != 0 || best_loc != 0) {
<br>
migration = true;
<br>
/* Align migration range start and size to
granularity size */
<br>
@@ -3102,10 +3118,11 @@ svm_range_restore_pages(struct
amdgpu_device *adev, unsigned int pasid,
<br>
}
<br>
}
<br>
- r = svm_range_validate_and_map(mm, prange, gpuidx,
false, false, false);
<br>
+ r = svm_range_validate_and_map(mm, start, last, prange,
gpuidx, false,
<br>
+ false, false);
<br>
if (r)
<br>
pr_debug("failed %d to map svms 0x%p [0x%lx 0x%lx]
to gpus\n",
<br>
- r, svms, prange->start, prange->last);
<br>
+ r, svms, start, last);
<br>
kfd_smi_event_page_fault_end(node,
p->lead_thread->pid, addr,
<br>
migration);
<br>
@@ -3650,7 +3667,7 @@ svm_range_set_attr(struct kfd_process
*p, struct mm_struct *mm,
<br>
flush_tlb = !migrated && update_mapping
&& prange->mapped_to_gpu;
<br>
- r = svm_range_validate_and_map(mm, prange,
MAX_GPU_INSTANCE,
<br>
+ r = svm_range_validate_and_map(mm, prange->start,
prange->last, prange, MAX_GPU_INSTANCE,
<br>
true, true, flush_tlb);
<br>
if (r)
<br>
pr_debug("failed %d to map svm range\n", r);
<br>
</blockquote>
</blockquote>
</blockquote>
</body>
</html>