<html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 2023-10-19 16:05, Felix Kuehling wrote: </div> <blockquote type="cite" cite="mid:0f8c20d2-65bb-4b03-901d-804c80f9300e@amd.com"> On 2023-10-18 18:26, Alex Sierra wrote: <blockquote type="cite">Split SVM ranges that have been mapped into 2MB page table entries, require to be remap in case the split has happened in a non-aligned VA. [WHY]: This condition causes the 2MB page table entries be split into 4KB PTEs. Signed-off-by: Alex Sierra <a class="moz-txt-link-rfc2396E" href="mailto:alex.sierra@amd.com"><alex.sierra@amd.com></a> --- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 45 +++++++++++++++++++++------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index 7b81233bc9ae..1dd9a1cf2358 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c @@ -1104,26 +1104,34 @@ svm_range_split(struct svm_range *prange, uint64_t start, uint64_t last, } static int -svm_range_split_tail(struct svm_range *prange, - uint64_t new_last, struct list_head *insert_list) +svm_range_split_tail(struct svm_range *prange, uint64_t new_last, + struct list_head *insert_list, struct list_head *remap_list) { struct svm_range *tail; int r = svm_range_split(prange, prange->start, new_last, &tail); - if (!r) + if (!r) { list_add(&tail->list, insert_list); + if (!IS_ALIGNED(tail->last + 1 - tail->start, + 1UL << tail->granularity)) </blockquote> I'm not sure about this condition. I thought this condition should be about the point where the range is split, not the size of it. So my understanding is that this should be if (!IS_ALIGNED(new_last+1, 1UL << prange->granularity)) </blockquote> I think prange->granularity is not always 9, 512 pages, we should check the original prange size is larger than 512. if (!IS_ALIGNED(new_last + 1, 512) && tail->last - prange->start >= 512) <blockquote type="cite" cite="mid:0f8c20d2-65bb-4b03-901d-804c80f9300e@amd.com"> <blockquote type="cite">+ list_add(&tail->update_list, remap_list); + } return r; } static int -svm_range_split_head(struct svm_range *prange, - uint64_t new_start, struct list_head *insert_list) +svm_range_split_head(struct svm_range *prange, uint64_t new_start, + struct list_head *insert_list, struct list_head *remap_list) { struct svm_range *head; int r = svm_range_split(prange, new_start, prange->last, &head); - if (!r) + if (!r) { list_add(&head->list, insert_list); + if (!IS_ALIGNED(head->last + 1 - head->start, + 1UL << head->granularity)) </blockquote> Similar as above. if (!IS_ALIGNED(new_start, 1UL << prange->granularity)) </blockquote> if (!IS_ALIGNED(new_start, 512) && prange->last - head->start >= 512) <blockquote type="cite" cite="mid:0f8c20d2-65bb-4b03-901d-804c80f9300e@amd.com"> Regards, Felix <blockquote type="cite">+ list_add(&head->update_list, remap_list); + } return r; } @@ -2113,7 +2121,7 @@ static int svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size, uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs, struct list_head *update_list, struct list_head *insert_list, - struct list_head *remove_list) + struct list_head *remove_list, struct list_head *remap_list) { unsigned long last = start + size - 1UL; struct svm_range_list *svms = &p->svms; @@ -2129,6 +2137,7 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size, INIT_LIST_HEAD(insert_list); INIT_LIST_HEAD(remove_list); INIT_LIST_HEAD(&new_list); + INIT_LIST_HEAD(remap_list); node = interval_tree_iter_first(&svms->objects, start, last); while (node) { @@ -2153,6 +2162,7 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size, struct svm_range *old = prange; prange = svm_range_clone(old); + </blockquote> </blockquote> unnecessary change. Regards, Philip <blockquote type="cite" cite="mid:0f8c20d2-65bb-4b03-901d-804c80f9300e@amd.com"> <blockquote type="cite"> if (!prange) { r = -ENOMEM; goto out; @@ -2161,18 +2171,17 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size, list_add(&old->update_list, remove_list); list_add(&prange->list, insert_list); list_add(&prange->update_list, update_list); - if (node->start < start) { pr_debug("change old range start\n"); r = svm_range_split_head(prange, start, - insert_list); + insert_list, remap_list); if (r) goto out; } if (node->last > last) { pr_debug("change old range last\n"); r = svm_range_split_tail(prange, last, - insert_list); + insert_list, remap_list); if (r) goto out; } @@ -3565,6 +3574,7 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm, struct list_head update_list; struct list_head insert_list; struct list_head remove_list; + struct list_head remap_list; struct svm_range_list *svms; struct svm_range *prange; struct svm_range *next; @@ -3596,7 +3606,7 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm, /* Add new range and split existing ranges as needed */ r = svm_range_add(p, start, size, nattr, attrs, &update_list, - &insert_list, &remove_list); + &insert_list, &remove_list, &remap_list); if (r) { mutex_unlock(&svms->lock); mmap_write_unlock(mm); @@ -3661,6 +3671,19 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm, ret = r; } + list_for_each_entry(prange, &remap_list, update_list) { + pr_debug("Remapping prange 0x%p [0x%lx 0x%lx]\n", + prange, prange->start, prange->last); + mutex_lock(&prange->migrate_mutex); + r = svm_range_validate_and_map(mm, prange, MAX_GPU_INSTANCE, + true, true, prange->mapped_to_gpu); + if (r) + pr_debug("failed %d on remap svm range\n", r); + mutex_unlock(&prange->migrate_mutex); + if (r) + ret = r; + } + dynamic_svm_range_dump(svms); mutex_unlock(&svms->lock); </blockquote> </blockquote> </body> </html>