Re: [PATCH 2/3] drm/amdgpu: support userptr cross VMAs case with HMM v3
Acked-by: Christian König
But I have the strong feeling that we sooner or later need to rewrite
the whole stuff from scratch.
Especially the struct amdgpu_ttm_tt structure now has a lot of
superfluous stuff left.
Christian.
Am 13.03.19 um 02:47 schrieb Kuehling, Felix:
This patch is Reviewed-by: Felix Kuehling
Regards,
Felix
On 3/12/2019 9:17 PM, Yang, Philip wrote:
userptr may cross two VMAs if the forked child process (not call exec
after fork) malloc buffer, then free it, and then malloc larger size
buf, kerenl will create new VMA adjacent to old VMA which was cloned
from parent process, some pages of userptr are in the first VMA, the
rest pages are in the second VMA.
HMM expects range only have one VMA, loop over all VMAs in the address
range, create multiple ranges to handle this case. See
is_mergeable_anon_vma in mm/mmap.c for details.
Change-Id: I0ca8c77e28deabccc139906f9ffee04b7e383314
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 126 +---
1 file changed, 91 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c1240bf243ba..c14198737dcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -711,7 +711,8 @@ struct amdgpu_ttm_tt {
struct task_struct *usertask;
uint32_tuserflags;
#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
- struct hmm_rangerange;
+ struct hmm_range*ranges;
+ int nr_ranges;
#endif
};
@@ -723,62 +724,108 @@ struct amdgpu_ttm_tt {
* once afterwards to stop HMM tracking
*/
#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
+
+/* Support Userptr pages cross max 16 vmas */
+#define MAX_NR_VMAS(16)
+
int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
{
struct amdgpu_ttm_tt *gtt = (void *)ttm;
struct mm_struct *mm = gtt->usertask->mm;
- unsigned long end = gtt->userptr + ttm->num_pages * PAGE_SIZE;
- struct hmm_range *range = >range;
- int r = 0, i;
+ unsigned long start = gtt->userptr;
+ unsigned long end = start + ttm->num_pages * PAGE_SIZE;
+ struct hmm_range *ranges;
+ struct vm_area_struct *vma = NULL, *vmas[MAX_NR_VMAS];
+ uint64_t *pfns, f;
+ int r = 0, i, nr_pages;
if (!mm) /* Happens during process shutdown */
return -ESRCH;
- amdgpu_hmm_init_range(range);
-
down_read(>mmap_sem);
- range->vma = find_vma(mm, gtt->userptr);
- if (!range_in_vma(range->vma, gtt->userptr, end))
- r = -EFAULT;
- else if ((gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) &&
- range->vma->vm_file)
+ /* user pages may cross multiple VMAs */
+ gtt->nr_ranges = 0;
+ do {
+ unsigned long vm_start;
+
+ if (gtt->nr_ranges >= MAX_NR_VMAS) {
+ DRM_ERROR("Too many VMAs in userptr range\n");
+ r = -EFAULT;
+ goto out;
+ }
+
+ vm_start = vma ? vma->vm_end : start;
+ vma = find_vma(mm, vm_start);
+ if (unlikely(!vma || vm_start < vma->vm_start)) {
+ r = -EFAULT;
+ goto out;
+ }
+ vmas[gtt->nr_ranges++] = vma;
+ } while (end > vma->vm_end);
+
+ DRM_DEBUG_DRIVER("0x%lx nr_ranges %d pages 0x%lx\n",
+ start, gtt->nr_ranges, ttm->num_pages);
+
+ if (unlikely((gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) &&
+ vmas[0]->vm_file)) {
r = -EPERM;
- if (r)
goto out;
+ }
- range->pfns = kvmalloc_array(ttm->num_pages, sizeof(uint64_t),
-GFP_KERNEL);
- if (range->pfns == NULL) {
+ ranges = kvmalloc_array(gtt->nr_ranges, sizeof(*ranges), GFP_KERNEL);
+ if (unlikely(!ranges)) {
r = -ENOMEM;
goto out;
}
- range->start = gtt->userptr;
- range->end = end;
- range->pfns[0] = range->flags[HMM_PFN_VALID];
- range->pfns[0] |= amdgpu_ttm_tt_is_readonly(ttm) ?
- 0 : range->flags[HMM_PFN_WRITE];
- for (i = 1; i < ttm->num_pages; i++)
- range->pfns[i] = range->pfns[0];
+ pfns = kvmalloc_array(ttm->num_pages, sizeof(*pfns), GFP_KERNEL);
+ if (unlikely(!pfns)) {
+ r = -ENOMEM;
+ goto out_free_ranges;
+ }
+
+ for (i = 0; i < gtt->nr_ranges; i++)
+ amdgpu_hmm_init_range([i]);
+
+ f = ranges[0].flags[HMM_PFN_VALID];
+ f |= amdgpu_ttm_tt_is_readonly(ttm) ?
+ 0 : ranges[0].flags[HMM_PFN_WRITE];
+ memset64(pfns, f, ttm->num_pages);
+
+ for (nr_pages = 0, i =
Re: [PATCH 2/3] drm/amdgpu: support userptr cross VMAs case with HMM v3
This patch is Reviewed-by: Felix Kuehling
Regards,
Felix
On 3/12/2019 9:17 PM, Yang, Philip wrote:
> userptr may cross two VMAs if the forked child process (not call exec
> after fork) malloc buffer, then free it, and then malloc larger size
> buf, kerenl will create new VMA adjacent to old VMA which was cloned
> from parent process, some pages of userptr are in the first VMA, the
> rest pages are in the second VMA.
>
> HMM expects range only have one VMA, loop over all VMAs in the address
> range, create multiple ranges to handle this case. See
> is_mergeable_anon_vma in mm/mmap.c for details.
>
> Change-Id: I0ca8c77e28deabccc139906f9ffee04b7e383314
> Signed-off-by: Philip Yang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 126 +---
> 1 file changed, 91 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index c1240bf243ba..c14198737dcd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -711,7 +711,8 @@ struct amdgpu_ttm_tt {
> struct task_struct *usertask;
> uint32_tuserflags;
> #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
> - struct hmm_rangerange;
> + struct hmm_range*ranges;
> + int nr_ranges;
> #endif
> };
>
> @@ -723,62 +724,108 @@ struct amdgpu_ttm_tt {
>* once afterwards to stop HMM tracking
>*/
> #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
> +
> +/* Support Userptr pages cross max 16 vmas */
> +#define MAX_NR_VMAS (16)
> +
> int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
> {
> struct amdgpu_ttm_tt *gtt = (void *)ttm;
> struct mm_struct *mm = gtt->usertask->mm;
> - unsigned long end = gtt->userptr + ttm->num_pages * PAGE_SIZE;
> - struct hmm_range *range = >range;
> - int r = 0, i;
> + unsigned long start = gtt->userptr;
> + unsigned long end = start + ttm->num_pages * PAGE_SIZE;
> + struct hmm_range *ranges;
> + struct vm_area_struct *vma = NULL, *vmas[MAX_NR_VMAS];
> + uint64_t *pfns, f;
> + int r = 0, i, nr_pages;
>
> if (!mm) /* Happens during process shutdown */
> return -ESRCH;
>
> - amdgpu_hmm_init_range(range);
> -
> down_read(>mmap_sem);
>
> - range->vma = find_vma(mm, gtt->userptr);
> - if (!range_in_vma(range->vma, gtt->userptr, end))
> - r = -EFAULT;
> - else if ((gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) &&
> - range->vma->vm_file)
> + /* user pages may cross multiple VMAs */
> + gtt->nr_ranges = 0;
> + do {
> + unsigned long vm_start;
> +
> + if (gtt->nr_ranges >= MAX_NR_VMAS) {
> + DRM_ERROR("Too many VMAs in userptr range\n");
> + r = -EFAULT;
> + goto out;
> + }
> +
> + vm_start = vma ? vma->vm_end : start;
> + vma = find_vma(mm, vm_start);
> + if (unlikely(!vma || vm_start < vma->vm_start)) {
> + r = -EFAULT;
> + goto out;
> + }
> + vmas[gtt->nr_ranges++] = vma;
> + } while (end > vma->vm_end);
> +
> + DRM_DEBUG_DRIVER("0x%lx nr_ranges %d pages 0x%lx\n",
> + start, gtt->nr_ranges, ttm->num_pages);
> +
> + if (unlikely((gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) &&
> + vmas[0]->vm_file)) {
> r = -EPERM;
> - if (r)
> goto out;
> + }
>
> - range->pfns = kvmalloc_array(ttm->num_pages, sizeof(uint64_t),
> - GFP_KERNEL);
> - if (range->pfns == NULL) {
> + ranges = kvmalloc_array(gtt->nr_ranges, sizeof(*ranges), GFP_KERNEL);
> + if (unlikely(!ranges)) {
> r = -ENOMEM;
> goto out;
> }
> - range->start = gtt->userptr;
> - range->end = end;
>
> - range->pfns[0] = range->flags[HMM_PFN_VALID];
> - range->pfns[0] |= amdgpu_ttm_tt_is_readonly(ttm) ?
> - 0 : range->flags[HMM_PFN_WRITE];
> - for (i = 1; i < ttm->num_pages; i++)
> - range->pfns[i] = range->pfns[0];
> + pfns = kvmalloc_array(ttm->num_pages, sizeof(*pfns), GFP_KERNEL);
> + if (unlikely(!pfns)) {
> + r = -ENOMEM;
> + goto out_free_ranges;
> + }
> +
> + for (i = 0; i < gtt->nr_ranges; i++)
> + amdgpu_hmm_init_range([i]);
> +
> + f = ranges[0].flags[HMM_PFN_VALID];
> + f |= amdgpu_ttm_tt_is_readonly(ttm) ?
> + 0 : ranges[0].flags[HMM_PFN_WRITE];
> + memset64(pfns, f, ttm->num_pages);
> +
> + for (nr_pages = 0, i = 0; i < gtt->nr_ranges; i++) {
> + ranges[i].vma = vmas[i];
> + ranges[i].start = max(start, vmas[i]->vm_start);
> + ranges[i].end = min(end,
[PATCH 2/3] drm/amdgpu: support userptr cross VMAs case with HMM v3
userptr may cross two VMAs if the forked child process (not call exec
after fork) malloc buffer, then free it, and then malloc larger size
buf, kerenl will create new VMA adjacent to old VMA which was cloned
from parent process, some pages of userptr are in the first VMA, the
rest pages are in the second VMA.
HMM expects range only have one VMA, loop over all VMAs in the address
range, create multiple ranges to handle this case. See
is_mergeable_anon_vma in mm/mmap.c for details.
Change-Id: I0ca8c77e28deabccc139906f9ffee04b7e383314
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 126 +---
1 file changed, 91 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c1240bf243ba..c14198737dcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -711,7 +711,8 @@ struct amdgpu_ttm_tt {
struct task_struct *usertask;
uint32_tuserflags;
#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
- struct hmm_rangerange;
+ struct hmm_range*ranges;
+ int nr_ranges;
#endif
};
@@ -723,62 +724,108 @@ struct amdgpu_ttm_tt {
* once afterwards to stop HMM tracking
*/
#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
+
+/* Support Userptr pages cross max 16 vmas */
+#define MAX_NR_VMAS(16)
+
int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
{
struct amdgpu_ttm_tt *gtt = (void *)ttm;
struct mm_struct *mm = gtt->usertask->mm;
- unsigned long end = gtt->userptr + ttm->num_pages * PAGE_SIZE;
- struct hmm_range *range = >range;
- int r = 0, i;
+ unsigned long start = gtt->userptr;
+ unsigned long end = start + ttm->num_pages * PAGE_SIZE;
+ struct hmm_range *ranges;
+ struct vm_area_struct *vma = NULL, *vmas[MAX_NR_VMAS];
+ uint64_t *pfns, f;
+ int r = 0, i, nr_pages;
if (!mm) /* Happens during process shutdown */
return -ESRCH;
- amdgpu_hmm_init_range(range);
-
down_read(>mmap_sem);
- range->vma = find_vma(mm, gtt->userptr);
- if (!range_in_vma(range->vma, gtt->userptr, end))
- r = -EFAULT;
- else if ((gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) &&
- range->vma->vm_file)
+ /* user pages may cross multiple VMAs */
+ gtt->nr_ranges = 0;
+ do {
+ unsigned long vm_start;
+
+ if (gtt->nr_ranges >= MAX_NR_VMAS) {
+ DRM_ERROR("Too many VMAs in userptr range\n");
+ r = -EFAULT;
+ goto out;
+ }
+
+ vm_start = vma ? vma->vm_end : start;
+ vma = find_vma(mm, vm_start);
+ if (unlikely(!vma || vm_start < vma->vm_start)) {
+ r = -EFAULT;
+ goto out;
+ }
+ vmas[gtt->nr_ranges++] = vma;
+ } while (end > vma->vm_end);
+
+ DRM_DEBUG_DRIVER("0x%lx nr_ranges %d pages 0x%lx\n",
+ start, gtt->nr_ranges, ttm->num_pages);
+
+ if (unlikely((gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) &&
+ vmas[0]->vm_file)) {
r = -EPERM;
- if (r)
goto out;
+ }
- range->pfns = kvmalloc_array(ttm->num_pages, sizeof(uint64_t),
-GFP_KERNEL);
- if (range->pfns == NULL) {
+ ranges = kvmalloc_array(gtt->nr_ranges, sizeof(*ranges), GFP_KERNEL);
+ if (unlikely(!ranges)) {
r = -ENOMEM;
goto out;
}
- range->start = gtt->userptr;
- range->end = end;
- range->pfns[0] = range->flags[HMM_PFN_VALID];
- range->pfns[0] |= amdgpu_ttm_tt_is_readonly(ttm) ?
- 0 : range->flags[HMM_PFN_WRITE];
- for (i = 1; i < ttm->num_pages; i++)
- range->pfns[i] = range->pfns[0];
+ pfns = kvmalloc_array(ttm->num_pages, sizeof(*pfns), GFP_KERNEL);
+ if (unlikely(!pfns)) {
+ r = -ENOMEM;
+ goto out_free_ranges;
+ }
+
+ for (i = 0; i < gtt->nr_ranges; i++)
+ amdgpu_hmm_init_range([i]);
+
+ f = ranges[0].flags[HMM_PFN_VALID];
+ f |= amdgpu_ttm_tt_is_readonly(ttm) ?
+ 0 : ranges[0].flags[HMM_PFN_WRITE];
+ memset64(pfns, f, ttm->num_pages);
+
+ for (nr_pages = 0, i = 0; i < gtt->nr_ranges; i++) {
+ ranges[i].vma = vmas[i];
+ ranges[i].start = max(start, vmas[i]->vm_start);
+ ranges[i].end = min(end, vmas[i]->vm_end);
+ ranges[i].pfns = pfns + nr_pages;
+ nr_pages += (ranges[i].end - ranges[i].start) / PAGE_SIZE;
+
+ r = hmm_vma_fault([i], true);
+ if
