Re: [PATCH 2/2] drm/amdgpu: replace get_user_pages with HMM address mirror helpers v2
On 2018-12-03 8:52 p.m., Kuehling, Felix wrote:
> See comments inline. I didn't review the amdgpu_cs and amdgpu_gem parts
> as I don't know them very well.
>
> On 2018-12-03 3:19 p.m., Yang, Philip wrote:
>> Use HMM helper function hmm_vma_fault() to get physical pages backing
>> userptr and start CPU page table update track of those pages. Then use
>> hmm_vma_range_done() to check if those pages are updated before
>> amdgpu_cs_submit for gfx or before user queues are resumed for kfd.
>>
>> If userptr pages are updated, for gfx, amdgpu_cs_ioctl will restart
>> from scratch, for kfd, restore worker is rescheduled to retry.
>>
>> To avoid circular lock dependency, no nested locking between mmap_sem
>> and bo::reserve. The locking order is:
>> bo::reserve -> amdgpu_mn_lock(p->mn)
>>
>> HMM simplify the CPU page table concurrent update check, so remove
>> guptasklock, mmu_invalidations, last_set_pages fields from
>> amdgpu_ttm_tt struct.
>>
>> HMM does not pin the page (increase page ref count), so remove related
>> operations like release_pages(), put_page(), mark_page_dirty().
>>
>> v2:
>> * Remove nested locking between mmap_sem and bo::reserve
>> * Change locking order to bo::reserve -> amdgpu_mn_lock()
>> * Use dynamic allocation to replace VLA in kernel stack
>>
>> Change-Id: Iffd5f855cc9ce402cdfca167f68f83fe39ac56f9
>> Signed-off-by: Philip Yang
>> ---
>> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 101 --
>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 -
>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 3 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 188 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 14 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 34 +++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h| 7 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 164 ++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 3 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 1 -
>> .../drm/amd/amdkfd/kfd_device_queue_manager.c | 67 ---
>> 11 files changed, 312 insertions(+), 272 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index f3129b912714..5ce6ba24fc72 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -614,8 +614,7 @@ static int init_user_pages(struct kgd_mem *mem, struct
>> mm_struct *mm,
>> amdgpu_bo_unreserve(bo);
>>
>> release_out:
>> -if (ret)
>> -release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
>> +amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
>> free_out:
>> kvfree(mem->user_pages);
>> mem->user_pages = NULL;
>> @@ -677,7 +676,6 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
>> ctx->kfd_bo.priority = 0;
>> ctx->kfd_bo.tv.bo = >tbo;
>> ctx->kfd_bo.tv.shared = true;
>> -ctx->kfd_bo.user_pages = NULL;
>> list_add(>kfd_bo.tv.head, >list);
>>
>> amdgpu_vm_get_pd_bo(vm, >list, >vm_pd[0]);
>> @@ -741,7 +739,6 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
>> ctx->kfd_bo.priority = 0;
>> ctx->kfd_bo.tv.bo = >tbo;
>> ctx->kfd_bo.tv.shared = true;
>> -ctx->kfd_bo.user_pages = NULL;
>> list_add(>kfd_bo.tv.head, >list);
>>
>> i = 0;
>> @@ -1332,9 +1329,6 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>> /* Free user pages if necessary */
>> if (mem->user_pages) {
>> pr_debug("%s: Freeing user_pages array\n", __func__);
>> -if (mem->user_pages[0])
>> -release_pages(mem->user_pages,
>> -mem->bo->tbo.ttm->num_pages);
>> kvfree(mem->user_pages);
>> }
>>
>> @@ -1761,8 +1755,6 @@ static int update_invalid_user_pages(struct
>> amdkfd_process_info *process_info,
>> __func__);
>> return -ENOMEM;
>> }
>> -} else if (mem->user_pages[0]) {
>> -release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
>> }
>>
>> /* Get updated user pages */
>> @@ -1778,12 +1770,6 @@ static int update_invalid_user_pages(struct
>> amdkfd_process_info *process_info,
>> * stalled user mode queues.
>> */
>> }
>> -
>> -/* Mark the BO as valid unless it was invalidated
>> - * again concurrently
>> - */
>> -if (atomic_cmpxchg(>invalid, invalid, 0) != invalid)
>> -return -EAGAIN;
>> }
>>
>> return 0;
>> @@ -1876,14 +1862,10 @@ static int validate_invalid_user_pages(struct
>> amdkfd_process_info *process_info)
>> }
>>
>> /* Validate succeeded, now the BO owns the pages, free
>> - * our copy of the
Re: [PATCH 2/2] drm/amdgpu: replace get_user_pages with HMM address mirror helpers v2
See comments inline. I didn't review the amdgpu_cs and amdgpu_gem parts
as I don't know them very well.
On 2018-12-03 3:19 p.m., Yang, Philip wrote:
> Use HMM helper function hmm_vma_fault() to get physical pages backing
> userptr and start CPU page table update track of those pages. Then use
> hmm_vma_range_done() to check if those pages are updated before
> amdgpu_cs_submit for gfx or before user queues are resumed for kfd.
>
> If userptr pages are updated, for gfx, amdgpu_cs_ioctl will restart
> from scratch, for kfd, restore worker is rescheduled to retry.
>
> To avoid circular lock dependency, no nested locking between mmap_sem
> and bo::reserve. The locking order is:
> bo::reserve -> amdgpu_mn_lock(p->mn)
>
> HMM simplify the CPU page table concurrent update check, so remove
> guptasklock, mmu_invalidations, last_set_pages fields from
> amdgpu_ttm_tt struct.
>
> HMM does not pin the page (increase page ref count), so remove related
> operations like release_pages(), put_page(), mark_page_dirty().
>
> v2:
> * Remove nested locking between mmap_sem and bo::reserve
> * Change locking order to bo::reserve -> amdgpu_mn_lock()
> * Use dynamic allocation to replace VLA in kernel stack
>
> Change-Id: Iffd5f855cc9ce402cdfca167f68f83fe39ac56f9
> Signed-off-by: Philip Yang
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 101 --
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 3 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 188 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 14 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 34 +++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h| 7 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 164 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 3 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 1 -
> .../drm/amd/amdkfd/kfd_device_queue_manager.c | 67 ---
> 11 files changed, 312 insertions(+), 272 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index f3129b912714..5ce6ba24fc72 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -614,8 +614,7 @@ static int init_user_pages(struct kgd_mem *mem, struct
> mm_struct *mm,
> amdgpu_bo_unreserve(bo);
>
> release_out:
> - if (ret)
> - release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
> + amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
> free_out:
> kvfree(mem->user_pages);
> mem->user_pages = NULL;
> @@ -677,7 +676,6 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
> ctx->kfd_bo.priority = 0;
> ctx->kfd_bo.tv.bo = >tbo;
> ctx->kfd_bo.tv.shared = true;
> - ctx->kfd_bo.user_pages = NULL;
> list_add(>kfd_bo.tv.head, >list);
>
> amdgpu_vm_get_pd_bo(vm, >list, >vm_pd[0]);
> @@ -741,7 +739,6 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
> ctx->kfd_bo.priority = 0;
> ctx->kfd_bo.tv.bo = >tbo;
> ctx->kfd_bo.tv.shared = true;
> - ctx->kfd_bo.user_pages = NULL;
> list_add(>kfd_bo.tv.head, >list);
>
> i = 0;
> @@ -1332,9 +1329,6 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> /* Free user pages if necessary */
> if (mem->user_pages) {
> pr_debug("%s: Freeing user_pages array\n", __func__);
> - if (mem->user_pages[0])
> - release_pages(mem->user_pages,
> - mem->bo->tbo.ttm->num_pages);
> kvfree(mem->user_pages);
> }
>
> @@ -1761,8 +1755,6 @@ static int update_invalid_user_pages(struct
> amdkfd_process_info *process_info,
> __func__);
> return -ENOMEM;
> }
> - } else if (mem->user_pages[0]) {
> - release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
> }
>
> /* Get updated user pages */
> @@ -1778,12 +1770,6 @@ static int update_invalid_user_pages(struct
> amdkfd_process_info *process_info,
>* stalled user mode queues.
>*/
> }
> -
> - /* Mark the BO as valid unless it was invalidated
> - * again concurrently
> - */
> - if (atomic_cmpxchg(>invalid, invalid, 0) != invalid)
> - return -EAGAIN;
> }
>
> return 0;
> @@ -1876,14 +1862,10 @@ static int validate_invalid_user_pages(struct
> amdkfd_process_info *process_info)
> }
>
> /* Validate succeeded, now the BO owns the pages, free
> - * our copy of the pointer array. Put this BO back on
> - * the userptr_valid_list. If we need to revalidate
> - * it, we need to start from
Re: [PATCH 2/2] drm/amdgpu: replace get_user_pages with HMM address mirror helpers
On 2018-10-17 04:15 AM, Christian König wrote:
> Am 17.10.18 um 04:56 schrieb Yang, Philip:
>> Use HMM helper function hmm_vma_fault() to get physical pages backing
>> userptr and start CPU page table update track of those pages. Then use
>> hmm_vma_range_done() to check if those pages are updated before
>> amdgpu_cs_submit for gfx or before user queues are resumed for kfd.
>>
>> If userptr pages are updated, for gfx, amdgpu_cs_ioctl will restart
>> from scratch, for kfd, restore worker is rescheduled to retry.
>>
>> To avoid circular lock dependency, the locking order is:
>> mmap_sem -> amdgpu_mn_lock(p->mn) -> bo::reserve
>> mmap_sem -> bo::reserve
>
> I'm not sure that this will work, we used to have some dependencies on
> bo::reserve -> mmap_sem.
>
> See the following patch as well:
>
> commit 2f568dbd6b944c2e8c0c54b53c2211c23995e6a4
> Author: Christian König
> Date: Tue Feb 23 12:36:59 2016 +0100
>
> drm/amdgpu: move get_user_pages out of amdgpu_ttm_tt_pin_userptr v6
>
> That avoids lock inversion between the BO reservation lock
> and the anon_vma lock.
>
> A whole bunch of more problems below.
>
Thanks for the review.
I will remove the nested lock between mmap_sem and bo::reserve, and
change the kfd side locking order
to bo::reserve -> amdgpu_mn_lock(p->mn) as the original gfx side locking
order.
Will submit patch v2 with other changes.
Philip
>>
>> HMM simplify the CPU page table concurrently update check, so remove
>> guptasklock, mmu_invalidations, last_set_pages fields from
>> amdgpu_ttm_tt struct.
>>
>> HMM doesnot pin the page (increase page ref count), so remove related
>> operations like release_pages(), put_page(), mark_page_dirty().
>>
>> Change-Id: Iffd5f855cc9ce402cdfca167f68f83fe39ac56f9
>> Signed-off-by: Philip Yang
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 101 ++---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 -
>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 3 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 171
>> +--
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 14 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 34 -
>> drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h | 7 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 164
>> +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 3 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 1 -
>> 10 files changed, 252 insertions(+), 248 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index df0a059..3fd0340 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -615,8 +615,7 @@ static int init_user_pages(struct kgd_mem *mem,
>> struct mm_struct *mm,
>> amdgpu_bo_unreserve(bo);
>> release_out:
>> - if (ret)
>> - release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
>> + amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
>> free_out:
>> kvfree(mem->user_pages);
>> mem->user_pages = NULL;
>> @@ -678,7 +677,6 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
>> ctx->kfd_bo.priority = 0;
>> ctx->kfd_bo.tv.bo = >tbo;
>> ctx->kfd_bo.tv.shared = true;
>> - ctx->kfd_bo.user_pages = NULL;
>> list_add(>kfd_bo.tv.head, >list);
>> amdgpu_vm_get_pd_bo(vm, >list, >vm_pd[0]);
>> @@ -742,7 +740,6 @@ static int reserve_bo_and_cond_vms(struct kgd_mem
>> *mem,
>> ctx->kfd_bo.priority = 0;
>> ctx->kfd_bo.tv.bo = >tbo;
>> ctx->kfd_bo.tv.shared = true;
>> - ctx->kfd_bo.user_pages = NULL;
>> list_add(>kfd_bo.tv.head, >list);
>> i = 0;
>> @@ -1311,9 +1308,6 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>> /* Free user pages if necessary */
>> if (mem->user_pages) {
>> pr_debug("%s: Freeing user_pages array\n", __func__);
>> - if (mem->user_pages[0])
>> - release_pages(mem->user_pages,
>> - mem->bo->tbo.ttm->num_pages);
>> kvfree(mem->user_pages);
>> }
>> @@ -1739,8 +1733,6 @@ static int update_invalid_user_pages(struct
>> amdkfd_process_info *process_info,
>> __func__);
>> return -ENOMEM;
>> }
>> - } else if (mem->user_pages[0]) {
>> - release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
>> }
>> /* Get updated user pages */
>> @@ -1756,12 +1748,6 @@ static int update_invalid_user_pages(struct
>> amdkfd_process_info *process_info,
>> * stalled user mode queues.
>> */
>> }
>> -
>> - /* Mark the BO as valid unless it was invalidated
>> - * again concurrently
>> - */
>> - if (atomic_cmpxchg(>invalid, invalid, 0) != invalid)
>> - return -EAGAIN;
>> }
>>
Re: [PATCH 2/2] drm/amdgpu: replace get_user_pages with HMM address mirror helpers
Am 17.10.18 um 04:56 schrieb Yang, Philip:
Use HMM helper function hmm_vma_fault() to get physical pages backing
userptr and start CPU page table update track of those pages. Then use
hmm_vma_range_done() to check if those pages are updated before
amdgpu_cs_submit for gfx or before user queues are resumed for kfd.
If userptr pages are updated, for gfx, amdgpu_cs_ioctl will restart
from scratch, for kfd, restore worker is rescheduled to retry.
To avoid circular lock dependency, the locking order is:
mmap_sem -> amdgpu_mn_lock(p->mn) -> bo::reserve
mmap_sem -> bo::reserve
I'm not sure that this will work, we used to have some dependencies on
bo::reserve -> mmap_sem.
See the following patch as well:
commit 2f568dbd6b944c2e8c0c54b53c2211c23995e6a4
Author: Christian König
Date: Tue Feb 23 12:36:59 2016 +0100
drm/amdgpu: move get_user_pages out of amdgpu_ttm_tt_pin_userptr v6
That avoids lock inversion between the BO reservation lock
and the anon_vma lock.
A whole bunch of more problems below.
HMM simplify the CPU page table concurrently update check, so remove
guptasklock, mmu_invalidations, last_set_pages fields from
amdgpu_ttm_tt struct.
HMM doesnot pin the page (increase page ref count), so remove related
operations like release_pages(), put_page(), mark_page_dirty().
Change-Id: Iffd5f855cc9ce402cdfca167f68f83fe39ac56f9
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 101 ++---
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 171 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 14 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 34 -
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h | 7 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 164 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 1 -
10 files changed, 252 insertions(+), 248 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index df0a059..3fd0340 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -615,8 +615,7 @@ static int init_user_pages(struct kgd_mem *mem, struct
mm_struct *mm,
amdgpu_bo_unreserve(bo);
release_out:
- if (ret)
- release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
+ amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
free_out:
kvfree(mem->user_pages);
mem->user_pages = NULL;
@@ -678,7 +677,6 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
ctx->kfd_bo.priority = 0;
ctx->kfd_bo.tv.bo = >tbo;
ctx->kfd_bo.tv.shared = true;
- ctx->kfd_bo.user_pages = NULL;
list_add(>kfd_bo.tv.head, >list);
amdgpu_vm_get_pd_bo(vm, >list, >vm_pd[0]);
@@ -742,7 +740,6 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
ctx->kfd_bo.priority = 0;
ctx->kfd_bo.tv.bo = >tbo;
ctx->kfd_bo.tv.shared = true;
- ctx->kfd_bo.user_pages = NULL;
list_add(>kfd_bo.tv.head, >list);
i = 0;
@@ -1311,9 +1308,6 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
/* Free user pages if necessary */
if (mem->user_pages) {
pr_debug("%s: Freeing user_pages array\n", __func__);
- if (mem->user_pages[0])
- release_pages(mem->user_pages,
- mem->bo->tbo.ttm->num_pages);
kvfree(mem->user_pages);
}
@@ -1739,8 +1733,6 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info,
__func__);
return -ENOMEM;
}
- } else if (mem->user_pages[0]) {
- release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
}
/* Get updated user pages */
@@ -1756,12 +1748,6 @@ static int update_invalid_user_pages(struct
amdkfd_process_info *process_info,
* stalled user mode queues.
*/
}
-
- /* Mark the BO as valid unless it was invalidated
-* again concurrently
-*/
- if (atomic_cmpxchg(>invalid, invalid, 0) != invalid)
- return -EAGAIN;
}
return 0;
@@ -1854,14 +1840,10 @@ static int validate_invalid_user_pages(struct
amdkfd_process_info *process_info)
}
/* Validate succeeded, now the BO owns the pages, free
-* our copy of the pointer array. Put this BO back on
-* the userptr_valid_list. If we need to revalidate
-* it, we need to start from
Re: [PATCH 2/2] drm/amdgpu: replace get_user_pages with HMM address mirror helpers
Am 17.10.18 um 04:56 schrieb Yang, Philip:
Use HMM helper function hmm_vma_fault() to get physical pages backing
userptr and start CPU page table update track of those pages. Then use
hmm_vma_range_done() to check if those pages are updated before
amdgpu_cs_submit for gfx or before user queues are resumed for kfd.
If userptr pages are updated, for gfx, amdgpu_cs_ioctl will restart
from scratch, for kfd, restore worker is rescheduled to retry.
To avoid circular lock dependency, the locking order is:
mmap_sem -> amdgpu_mn_lock(p->mn) -> bo::reserve
mmap_sem -> bo::reserve
I'm not sure that this will work, we used to have some dependencies on
bo::reserve -> mmap_sem.
See the following patch as well:
commit 2f568dbd6b944c2e8c0c54b53c2211c23995e6a4
Author: Christian König
Date: Tue Feb 23 12:36:59 2016 +0100
drm/amdgpu: move get_user_pages out of amdgpu_ttm_tt_pin_userptr v6
That avoids lock inversion between the BO reservation lock
and the anon_vma lock.
A few more comments below.
HMM simplify the CPU page table concurrently update check, so remove
guptasklock, mmu_invalidations, last_set_pages fields from
amdgpu_ttm_tt struct.
HMM doesnot pin the page (increase page ref count), so remove related
operations like release_pages(), put_page(), mark_page_dirty().
Change-Id: Iffd5f855cc9ce402cdfca167f68f83fe39ac56f9
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 101 ++---
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 171 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 14 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 34 -
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h | 7 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 164 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 1 -
10 files changed, 252 insertions(+), 248 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index df0a059..3fd0340 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -615,8 +615,7 @@ static int init_user_pages(struct kgd_mem *mem, struct
mm_struct *mm,
amdgpu_bo_unreserve(bo);
release_out:
- if (ret)
- release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
+ amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
free_out:
kvfree(mem->user_pages);
mem->user_pages = NULL;
@@ -678,7 +677,6 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
ctx->kfd_bo.priority = 0;
ctx->kfd_bo.tv.bo = >tbo;
ctx->kfd_bo.tv.shared = true;
- ctx->kfd_bo.user_pages = NULL;
list_add(>kfd_bo.tv.head, >list);
amdgpu_vm_get_pd_bo(vm, >list, >vm_pd[0]);
@@ -742,7 +740,6 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
ctx->kfd_bo.priority = 0;
ctx->kfd_bo.tv.bo = >tbo;
ctx->kfd_bo.tv.shared = true;
- ctx->kfd_bo.user_pages = NULL;
list_add(>kfd_bo.tv.head, >list);
i = 0;
@@ -1311,9 +1308,6 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
/* Free user pages if necessary */
if (mem->user_pages) {
pr_debug("%s: Freeing user_pages array\n", __func__);
- if (mem->user_pages[0])
- release_pages(mem->user_pages,
- mem->bo->tbo.ttm->num_pages);
kvfree(mem->user_pages);
}
@@ -1739,8 +1733,6 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info,
__func__);
return -ENOMEM;
}
- } else if (mem->user_pages[0]) {
- release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
}
/* Get updated user pages */
@@ -1756,12 +1748,6 @@ static int update_invalid_user_pages(struct
amdkfd_process_info *process_info,
* stalled user mode queues.
*/
}
-
- /* Mark the BO as valid unless it was invalidated
-* again concurrently
-*/
- if (atomic_cmpxchg(>invalid, invalid, 0) != invalid)
- return -EAGAIN;
}
return 0;
@@ -1854,14 +1840,10 @@ static int validate_invalid_user_pages(struct
amdkfd_process_info *process_info)
}
/* Validate succeeded, now the BO owns the pages, free
-* our copy of the pointer array. Put this BO back on
-* the userptr_valid_list. If we need to revalidate
-* it, we need to start from scratch.
+
