* [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2
@ 2018-05-17 9:49 Christian König
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2018-05-17 9:49 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Only the moved state needs a separate spin lock protection. All other
states are protected by reserving the VM anyway.
v2: fix some more incorrect cases
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 66 +++++++++++-----------------------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 +--
2 files changed, 21 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 1a8f4e0dd023..f0deedcaf1c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -119,9 +119,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
* is currently evicted. add the bo to the evicted list to make sure it
* is validated on next vm use to avoid fault.
* */
- spin_lock(&vm->status_lock);
list_move_tail(&base->vm_status, &vm->evicted);
- spin_unlock(&vm->status_lock);
}
/**
@@ -228,7 +226,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct ttm_bo_global *glob = adev->mman.bdev.glob;
int r;
- spin_lock(&vm->status_lock);
while (!list_empty(&vm->evicted)) {
struct amdgpu_vm_bo_base *bo_base;
struct amdgpu_bo *bo;
@@ -236,10 +233,8 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
bo_base = list_first_entry(&vm->evicted,
struct amdgpu_vm_bo_base,
vm_status);
- spin_unlock(&vm->status_lock);
bo = bo_base->bo;
- BUG_ON(!bo);
if (bo->parent) {
r = validate(param, bo);
if (r)
@@ -259,13 +254,14 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
return r;
}
- spin_lock(&vm->status_lock);
- if (bo->tbo.type != ttm_bo_type_kernel)
+ if (bo->tbo.type != ttm_bo_type_kernel) {
+ spin_lock(&vm->moved_lock);
list_move(&bo_base->vm_status, &vm->moved);
- else
+ spin_unlock(&vm->moved_lock);
+ } else {
list_move(&bo_base->vm_status, &vm->relocated);
+ }
}
- spin_unlock(&vm->status_lock);
return 0;
}
@@ -279,13 +275,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
*/
bool amdgpu_vm_ready(struct amdgpu_vm *vm)
{
- bool ready;
-
- spin_lock(&vm->status_lock);
- ready = list_empty(&vm->evicted);
- spin_unlock(&vm->status_lock);
-
- return ready;
+ return list_empty(&vm->evicted);
}
/**
@@ -477,9 +467,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
pt->parent = amdgpu_bo_ref(parent->base.bo);
amdgpu_vm_bo_base_init(&entry->base, vm, pt);
- spin_lock(&vm->status_lock);
list_move(&entry->base.vm_status, &vm->relocated);
- spin_unlock(&vm->status_lock);
}
if (level < AMDGPU_VM_PTB) {
@@ -926,10 +914,8 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
if (!entry->base.bo)
continue;
- spin_lock(&vm->status_lock);
if (list_empty(&entry->base.vm_status))
list_add(&entry->base.vm_status, &vm->relocated);
- spin_unlock(&vm->status_lock);
amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
}
}
@@ -974,7 +960,6 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
params.func = amdgpu_vm_do_set_ptes;
}
- spin_lock(&vm->status_lock);
while (!list_empty(&vm->relocated)) {
struct amdgpu_vm_bo_base *bo_base, *parent;
struct amdgpu_vm_pt *pt, *entry;
@@ -984,13 +969,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
struct amdgpu_vm_bo_base,
vm_status);
list_del_init(&bo_base->vm_status);
- spin_unlock(&vm->status_lock);
bo = bo_base->bo->parent;
- if (!bo) {
- spin_lock(&vm->status_lock);
+ if (!bo)
continue;
- }
parent = list_first_entry(&bo->va, struct amdgpu_vm_bo_base,
bo_list);
@@ -999,12 +981,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
amdgpu_vm_update_pde(¶ms, vm, pt, entry);
- spin_lock(&vm->status_lock);
if (!vm->use_cpu_for_update &&
(ndw - params.ib->length_dw) < 32)
break;
}
- spin_unlock(&vm->status_lock);
if (vm->use_cpu_for_update) {
/* Flush HDP */
@@ -1107,9 +1087,7 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
if (entry->huge) {
/* Add the entry to the relocated list to update it. */
entry->huge = false;
- spin_lock(&p->vm->status_lock);
list_move(&entry->base.vm_status, &p->vm->relocated);
- spin_unlock(&p->vm->status_lock);
}
return;
}
@@ -1588,8 +1566,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
amdgpu_asic_flush_hdp(adev, NULL);
}
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->moved_lock);
list_del_init(&bo_va->base.vm_status);
+ spin_unlock(&vm->moved_lock);
/* If the BO is not in its preferred location add it back to
* the evicted list so that it gets validated again on the
@@ -1599,7 +1578,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
!(bo->preferred_domains &
amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)))
list_add_tail(&bo_va->base.vm_status, &vm->evicted);
- spin_unlock(&vm->status_lock);
list_splice_init(&bo_va->invalids, &bo_va->valids);
bo_va->cleared = clear;
@@ -1811,14 +1789,14 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
bool clear;
int r = 0;
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->moved_lock);
while (!list_empty(&vm->moved)) {
struct amdgpu_bo_va *bo_va;
struct reservation_object *resv;
bo_va = list_first_entry(&vm->moved,
struct amdgpu_bo_va, base.vm_status);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->moved_lock);
resv = bo_va->base.bo->tbo.resv;
@@ -1839,9 +1817,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
if (!clear && resv != vm->root.base.bo->tbo.resv)
reservation_object_unlock(resv);
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->moved_lock);
}
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->moved_lock);
return r;
}
@@ -1903,10 +1881,10 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
amdgpu_vm_prt_get(adev);
if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->moved_lock);
if (list_empty(&bo_va->base.vm_status))
list_add(&bo_va->base.vm_status, &vm->moved);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->moved_lock);
}
trace_amdgpu_vm_bo_map(bo_va, mapping);
}
@@ -2216,9 +2194,9 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
list_del(&bo_va->base.bo_list);
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->moved_lock);
list_del(&bo_va->base.vm_status);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->moved_lock);
list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
list_del(&mapping->list);
@@ -2261,28 +2239,24 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
bo_base->moved = true;
if (evicted && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
- spin_lock(&bo_base->vm->status_lock);
if (bo->tbo.type == ttm_bo_type_kernel)
list_move(&bo_base->vm_status, &vm->evicted);
else
list_move_tail(&bo_base->vm_status,
&vm->evicted);
- spin_unlock(&bo_base->vm->status_lock);
continue;
}
if (bo->tbo.type == ttm_bo_type_kernel) {
- spin_lock(&bo_base->vm->status_lock);
if (list_empty(&bo_base->vm_status))
list_add(&bo_base->vm_status, &vm->relocated);
- spin_unlock(&bo_base->vm->status_lock);
continue;
}
- spin_lock(&bo_base->vm->status_lock);
+ spin_lock(&bo_base->vm->moved_lock);
if (list_empty(&bo_base->vm_status))
list_add(&bo_base->vm_status, &vm->moved);
- spin_unlock(&bo_base->vm->status_lock);
+ spin_unlock(&bo_base->vm->moved_lock);
}
}
@@ -2391,9 +2365,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
vm->va = RB_ROOT_CACHED;
for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
vm->reserved_vmid[i] = NULL;
- spin_lock_init(&vm->status_lock);
INIT_LIST_HEAD(&vm->evicted);
INIT_LIST_HEAD(&vm->relocated);
+ spin_lock_init(&vm->moved_lock);
INIT_LIST_HEAD(&vm->moved);
INIT_LIST_HEAD(&vm->freed);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index d6827083572a..0196b9a782f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -168,9 +168,6 @@ struct amdgpu_vm {
/* tree of virtual addresses mapped */
struct rb_root_cached va;
- /* protecting invalidated */
- spinlock_t status_lock;
-
/* BOs who needs a validation */
struct list_head evicted;
@@ -179,6 +176,7 @@ struct amdgpu_vm {
/* BOs moved, but not yet updated in the PT */
struct list_head moved;
+ spinlock_t moved_lock;
/* BO mappings freed, but not yet updated in the PT */
struct list_head freed;
--
2.14.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_validate_pt_bos v2
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
@ 2018-05-17 9:49 ` Christian König
2018-05-17 9:49 ` [PATCH 3/6] drm/amdgpu: further optimize amdgpu_vm_handle_moved Christian König
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2018-05-17 9:49 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Use list_for_each_entry_safe here.
v2: Drop the optimization, it doesn't work as expected.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f0deedcaf1c9..3be4d5fc60b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -224,21 +224,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
void *param)
{
struct ttm_bo_global *glob = adev->mman.bdev.glob;
- int r;
-
- while (!list_empty(&vm->evicted)) {
- struct amdgpu_vm_bo_base *bo_base;
- struct amdgpu_bo *bo;
+ struct amdgpu_vm_bo_base *bo_base, *tmp;
+ int r = 0;
- bo_base = list_first_entry(&vm->evicted,
- struct amdgpu_vm_bo_base,
- vm_status);
+ list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
+ struct amdgpu_bo *bo = bo_base->bo;
- bo = bo_base->bo;
if (bo->parent) {
r = validate(param, bo);
if (r)
- return r;
+ break;
spin_lock(&glob->lru_lock);
ttm_bo_move_to_lru_tail(&bo->tbo);
@@ -251,7 +246,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
vm->use_cpu_for_update) {
r = amdgpu_bo_kmap(bo, NULL);
if (r)
- return r;
+ break;
}
if (bo->tbo.type != ttm_bo_type_kernel) {
@@ -263,7 +258,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
}
}
- return 0;
+ return r;
}
/**
--
2.14.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/6] drm/amdgpu: further optimize amdgpu_vm_handle_moved
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-05-17 9:49 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_validate_pt_bos v2 Christian König
@ 2018-05-17 9:49 ` Christian König
2018-05-17 9:49 ` [PATCH 4/6] drm/amdgpu: kmap PDs/PTs in amdgpu_vm_update_directories Christian König
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2018-05-17 9:49 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Splice the moved list to a local one to avoid taking the lock over and
over again.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3be4d5fc60b3..4d88b060fbde 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1781,19 +1781,18 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
{
+ struct amdgpu_bo_va *bo_va, *tmp;
+ struct list_head moved;
bool clear;
- int r = 0;
+ int r;
+ INIT_LIST_HEAD(&moved);
spin_lock(&vm->moved_lock);
- while (!list_empty(&vm->moved)) {
- struct amdgpu_bo_va *bo_va;
- struct reservation_object *resv;
-
- bo_va = list_first_entry(&vm->moved,
- struct amdgpu_bo_va, base.vm_status);
- spin_unlock(&vm->moved_lock);
+ list_splice_init(&vm->moved, &moved);
+ spin_unlock(&vm->moved_lock);
- resv = bo_va->base.bo->tbo.resv;
+ list_for_each_entry_safe(bo_va, tmp, &moved, base.vm_status) {
+ struct reservation_object *resv = bo_va->base.bo->tbo.resv;
/* Per VM BOs never need to bo cleared in the page tables */
if (resv == vm->root.base.bo->tbo.resv)
@@ -1806,17 +1805,19 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
clear = true;
r = amdgpu_vm_bo_update(adev, bo_va, clear);
- if (r)
+ if (r) {
+ spin_lock(&vm->moved_lock);
+ list_splice(&moved, &vm->moved);
+ spin_unlock(&vm->moved_lock);
return r;
+ }
if (!clear && resv != vm->root.base.bo->tbo.resv)
reservation_object_unlock(resv);
- spin_lock(&vm->moved_lock);
}
- spin_unlock(&vm->moved_lock);
- return r;
+ return 0;
}
/**
--
2.14.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/6] drm/amdgpu: kmap PDs/PTs in amdgpu_vm_update_directories
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-05-17 9:49 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_validate_pt_bos v2 Christian König
2018-05-17 9:49 ` [PATCH 3/6] drm/amdgpu: further optimize amdgpu_vm_handle_moved Christian König
@ 2018-05-17 9:49 ` Christian König
2018-05-17 9:49 ` [PATCH 5/6] drm/amdgpu: consistenly use VM moved flag Christian König
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2018-05-17 9:49 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In theory it is possible that PDs/PTs can move without eviction.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4d88b060fbde..a31afac8e8e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -242,13 +242,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
spin_unlock(&glob->lru_lock);
}
- if (bo->tbo.type == ttm_bo_type_kernel &&
- vm->use_cpu_for_update) {
- r = amdgpu_bo_kmap(bo, NULL);
- if (r)
- break;
- }
-
if (bo->tbo.type != ttm_bo_type_kernel) {
spin_lock(&vm->moved_lock);
list_move(&bo_base->vm_status, &vm->moved);
@@ -940,6 +933,14 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
params.adev = adev;
if (vm->use_cpu_for_update) {
+ struct amdgpu_vm_bo_base *bo_base;
+
+ list_for_each_entry(bo_base, &vm->relocated, vm_status) {
+ r = amdgpu_bo_kmap(bo_base->bo, NULL);
+ if (unlikely(r))
+ return r;
+ }
+
r = amdgpu_vm_wait_pd(adev, vm, AMDGPU_FENCE_OWNER_VM);
if (unlikely(r))
return r;
--
2.14.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/6] drm/amdgpu: consistenly use VM moved flag
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
` (2 preceding siblings ...)
2018-05-17 9:49 ` [PATCH 4/6] drm/amdgpu: kmap PDs/PTs in amdgpu_vm_update_directories Christian König
@ 2018-05-17 9:49 ` Christian König
2018-05-17 9:49 ` [PATCH 6/6] drm/amdgpu: move VM BOs on LRU again Christian König
2018-05-18 9:55 ` [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2 Zhang, Jerry (Junwei)
5 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2018-05-17 9:49 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Instead of sometimes checking if the vm_status is empty use the moved
flag and also reset it when the BO leaves the state machine.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a31afac8e8e9..f5dee4c6757c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -902,8 +902,8 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
if (!entry->base.bo)
continue;
- if (list_empty(&entry->base.vm_status))
- list_add(&entry->base.vm_status, &vm->relocated);
+ if (!entry->base.moved)
+ list_move(&entry->base.vm_status, &vm->relocated);
amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
}
}
@@ -964,6 +964,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
bo_base = list_first_entry(&vm->relocated,
struct amdgpu_vm_bo_base,
vm_status);
+ bo_base->moved = false;
list_del_init(&bo_base->vm_status);
bo = bo_base->bo->parent;
@@ -1877,10 +1878,10 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
if (mapping->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
- if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
+ if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv &&
+ !bo_va->base.moved) {
spin_lock(&vm->moved_lock);
- if (list_empty(&bo_va->base.vm_status))
- list_add(&bo_va->base.vm_status, &vm->moved);
+ list_move(&bo_va->base.vm_status, &vm->moved);
spin_unlock(&vm->moved_lock);
}
trace_amdgpu_vm_bo_map(bo_va, mapping);
@@ -2233,6 +2234,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
list_for_each_entry(bo_base, &bo->va, bo_list) {
struct amdgpu_vm *vm = bo_base->vm;
+ bool was_moved = bo_base->moved;
bo_base->moved = true;
if (evicted && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
@@ -2244,16 +2246,16 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
continue;
}
- if (bo->tbo.type == ttm_bo_type_kernel) {
- if (list_empty(&bo_base->vm_status))
- list_add(&bo_base->vm_status, &vm->relocated);
+ if (was_moved)
continue;
- }
- spin_lock(&bo_base->vm->moved_lock);
- if (list_empty(&bo_base->vm_status))
- list_add(&bo_base->vm_status, &vm->moved);
- spin_unlock(&bo_base->vm->moved_lock);
+ if (bo->tbo.type == ttm_bo_type_kernel) {
+ list_move(&bo_base->vm_status, &vm->relocated);
+ } else {
+ spin_lock(&bo_base->vm->moved_lock);
+ list_move(&bo_base->vm_status, &vm->moved);
+ spin_unlock(&bo_base->vm->moved_lock);
+ }
}
}
--
2.14.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 6/6] drm/amdgpu: move VM BOs on LRU again
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
` (3 preceding siblings ...)
2018-05-17 9:49 ` [PATCH 5/6] drm/amdgpu: consistenly use VM moved flag Christian König
@ 2018-05-17 9:49 ` Christian König
[not found] ` <20180517094936.4293-6-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-05-18 9:55 ` [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2 Zhang, Jerry (Junwei)
5 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2018-05-17 9:49 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Move all BOs belonging to a VM on the LRU with every submission.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28 +++++++++++++++++++++++-----
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +++
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f5dee4c6757c..ccba88cc8c54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -251,6 +251,19 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
}
}
+ spin_lock(&glob->lru_lock);
+ list_for_each_entry(bo_base, &vm->idle, vm_status) {
+ struct amdgpu_bo *bo = bo_base->bo;
+
+ if (!bo->parent)
+ continue;
+
+ ttm_bo_move_to_lru_tail(&bo->tbo);
+ if (bo->shadow)
+ ttm_bo_move_to_lru_tail(&bo->shadow->tbo);
+ }
+ spin_unlock(&glob->lru_lock);
+
return r;
}
@@ -965,7 +978,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
struct amdgpu_vm_bo_base,
vm_status);
bo_base->moved = false;
- list_del_init(&bo_base->vm_status);
+ list_move(&bo_base->vm_status, &vm->idle);
bo = bo_base->bo->parent;
if (!bo)
@@ -1571,10 +1584,14 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
* the evicted list so that it gets validated again on the
* next command submission.
*/
- if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv &&
- !(bo->preferred_domains &
- amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)))
- list_add_tail(&bo_va->base.vm_status, &vm->evicted);
+ if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
+ uint32_t mem_type = bo->tbo.mem.mem_type;
+
+ if (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type)))
+ list_add_tail(&bo_va->base.vm_status, &vm->evicted);
+ else
+ list_add(&bo_va->base.vm_status, &vm->idle);
+ }
list_splice_init(&bo_va->invalids, &bo_va->valids);
bo_va->cleared = clear;
@@ -2368,6 +2385,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
INIT_LIST_HEAD(&vm->relocated);
spin_lock_init(&vm->moved_lock);
INIT_LIST_HEAD(&vm->moved);
+ INIT_LIST_HEAD(&vm->idle);
INIT_LIST_HEAD(&vm->freed);
/* create scheduler entity for page table updates */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 0196b9a782f2..061b99a18cb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -178,6 +178,9 @@ struct amdgpu_vm {
struct list_head moved;
spinlock_t moved_lock;
+ /* All BOs of this VM not currently in the state machine */
+ struct list_head idle;
+
/* BO mappings freed, but not yet updated in the PT */
struct list_head freed;
--
2.14.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 6/6] drm/amdgpu: move VM BOs on LRU again
[not found] ` <20180517094936.4293-6-christian.koenig-5C7GfCeVMHo@public.gmane.org>
@ 2018-05-18 6:49 ` zhoucm1
0 siblings, 0 replies; 10+ messages in thread
From: zhoucm1 @ 2018-05-18 6:49 UTC (permalink / raw)
To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
CPU overhead is increased a bit, but we can optimize it later, the
series is Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
On 2018年05月17日 17:49, Christian König wrote:
> Move all BOs belonging to a VM on the LRU with every submission.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28 +++++++++++++++++++++++-----
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +++
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f5dee4c6757c..ccba88cc8c54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -251,6 +251,19 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> }
> }
>
> + spin_lock(&glob->lru_lock);
> + list_for_each_entry(bo_base, &vm->idle, vm_status) {
> + struct amdgpu_bo *bo = bo_base->bo;
> +
> + if (!bo->parent)
> + continue;
> +
> + ttm_bo_move_to_lru_tail(&bo->tbo);
> + if (bo->shadow)
> + ttm_bo_move_to_lru_tail(&bo->shadow->tbo);
> + }
> + spin_unlock(&glob->lru_lock);
> +
> return r;
> }
>
> @@ -965,7 +978,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> struct amdgpu_vm_bo_base,
> vm_status);
> bo_base->moved = false;
> - list_del_init(&bo_base->vm_status);
> + list_move(&bo_base->vm_status, &vm->idle);
>
> bo = bo_base->bo->parent;
> if (!bo)
> @@ -1571,10 +1584,14 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> * the evicted list so that it gets validated again on the
> * next command submission.
> */
> - if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv &&
> - !(bo->preferred_domains &
> - amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)))
> - list_add_tail(&bo_va->base.vm_status, &vm->evicted);
> + if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
> + uint32_t mem_type = bo->tbo.mem.mem_type;
> +
> + if (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type)))
> + list_add_tail(&bo_va->base.vm_status, &vm->evicted);
> + else
> + list_add(&bo_va->base.vm_status, &vm->idle);
> + }
>
> list_splice_init(&bo_va->invalids, &bo_va->valids);
> bo_va->cleared = clear;
> @@ -2368,6 +2385,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> INIT_LIST_HEAD(&vm->relocated);
> spin_lock_init(&vm->moved_lock);
> INIT_LIST_HEAD(&vm->moved);
> + INIT_LIST_HEAD(&vm->idle);
> INIT_LIST_HEAD(&vm->freed);
>
> /* create scheduler entity for page table updates */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 0196b9a782f2..061b99a18cb8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -178,6 +178,9 @@ struct amdgpu_vm {
> struct list_head moved;
> spinlock_t moved_lock;
>
> + /* All BOs of this VM not currently in the state machine */
> + struct list_head idle;
> +
> /* BO mappings freed, but not yet updated in the PT */
> struct list_head freed;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
` (4 preceding siblings ...)
2018-05-17 9:49 ` [PATCH 6/6] drm/amdgpu: move VM BOs on LRU again Christian König
@ 2018-05-18 9:55 ` Zhang, Jerry (Junwei)
[not found] ` <5AFEA313.8040905-5C7GfCeVMHo@public.gmane.org>
5 siblings, 1 reply; 10+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-05-18 9:55 UTC (permalink / raw)
To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
2, 3, 4, 5 are
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
Patch 1:
could you show the reserving VM?
Patch 6:
I could read that code, but not sure the purpose.
Jerry
On 05/17/2018 05:49 PM, Christian König wrote:
> Only the moved state needs a separate spin lock protection. All other
> states are protected by reserving the VM anyway.
>
> v2: fix some more incorrect cases
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 66 +++++++++++-----------------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 +--
> 2 files changed, 21 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 1a8f4e0dd023..f0deedcaf1c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -119,9 +119,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> * is currently evicted. add the bo to the evicted list to make sure it
> * is validated on next vm use to avoid fault.
> * */
> - spin_lock(&vm->status_lock);
> list_move_tail(&base->vm_status, &vm->evicted);
> - spin_unlock(&vm->status_lock);
> }
>
> /**
> @@ -228,7 +226,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> struct ttm_bo_global *glob = adev->mman.bdev.glob;
> int r;
>
> - spin_lock(&vm->status_lock);
> while (!list_empty(&vm->evicted)) {
> struct amdgpu_vm_bo_base *bo_base;
> struct amdgpu_bo *bo;
> @@ -236,10 +233,8 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> bo_base = list_first_entry(&vm->evicted,
> struct amdgpu_vm_bo_base,
> vm_status);
> - spin_unlock(&vm->status_lock);
>
> bo = bo_base->bo;
> - BUG_ON(!bo);
> if (bo->parent) {
> r = validate(param, bo);
> if (r)
> @@ -259,13 +254,14 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> return r;
> }
>
> - spin_lock(&vm->status_lock);
> - if (bo->tbo.type != ttm_bo_type_kernel)
> + if (bo->tbo.type != ttm_bo_type_kernel) {
> + spin_lock(&vm->moved_lock);
> list_move(&bo_base->vm_status, &vm->moved);
> - else
> + spin_unlock(&vm->moved_lock);
> + } else {
> list_move(&bo_base->vm_status, &vm->relocated);
> + }
> }
> - spin_unlock(&vm->status_lock);
>
> return 0;
> }
> @@ -279,13 +275,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> */
> bool amdgpu_vm_ready(struct amdgpu_vm *vm)
> {
> - bool ready;
> -
> - spin_lock(&vm->status_lock);
> - ready = list_empty(&vm->evicted);
> - spin_unlock(&vm->status_lock);
> -
> - return ready;
> + return list_empty(&vm->evicted);
> }
>
> /**
> @@ -477,9 +467,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
> pt->parent = amdgpu_bo_ref(parent->base.bo);
>
> amdgpu_vm_bo_base_init(&entry->base, vm, pt);
> - spin_lock(&vm->status_lock);
> list_move(&entry->base.vm_status, &vm->relocated);
> - spin_unlock(&vm->status_lock);
> }
>
> if (level < AMDGPU_VM_PTB) {
> @@ -926,10 +914,8 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
> if (!entry->base.bo)
> continue;
>
> - spin_lock(&vm->status_lock);
> if (list_empty(&entry->base.vm_status))
> list_add(&entry->base.vm_status, &vm->relocated);
> - spin_unlock(&vm->status_lock);
> amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
> }
> }
> @@ -974,7 +960,6 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> params.func = amdgpu_vm_do_set_ptes;
> }
>
> - spin_lock(&vm->status_lock);
> while (!list_empty(&vm->relocated)) {
> struct amdgpu_vm_bo_base *bo_base, *parent;
> struct amdgpu_vm_pt *pt, *entry;
> @@ -984,13 +969,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> struct amdgpu_vm_bo_base,
> vm_status);
> list_del_init(&bo_base->vm_status);
> - spin_unlock(&vm->status_lock);
>
> bo = bo_base->bo->parent;
> - if (!bo) {
> - spin_lock(&vm->status_lock);
> + if (!bo)
> continue;
> - }
>
> parent = list_first_entry(&bo->va, struct amdgpu_vm_bo_base,
> bo_list);
> @@ -999,12 +981,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>
> amdgpu_vm_update_pde(¶ms, vm, pt, entry);
>
> - spin_lock(&vm->status_lock);
> if (!vm->use_cpu_for_update &&
> (ndw - params.ib->length_dw) < 32)
> break;
> }
> - spin_unlock(&vm->status_lock);
>
> if (vm->use_cpu_for_update) {
> /* Flush HDP */
> @@ -1107,9 +1087,7 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
> if (entry->huge) {
> /* Add the entry to the relocated list to update it. */
> entry->huge = false;
> - spin_lock(&p->vm->status_lock);
> list_move(&entry->base.vm_status, &p->vm->relocated);
> - spin_unlock(&p->vm->status_lock);
> }
> return;
> }
> @@ -1588,8 +1566,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> amdgpu_asic_flush_hdp(adev, NULL);
> }
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> list_del_init(&bo_va->base.vm_status);
> + spin_unlock(&vm->moved_lock);
>
> /* If the BO is not in its preferred location add it back to
> * the evicted list so that it gets validated again on the
> @@ -1599,7 +1578,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> !(bo->preferred_domains &
> amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)))
> list_add_tail(&bo_va->base.vm_status, &vm->evicted);
> - spin_unlock(&vm->status_lock);
>
> list_splice_init(&bo_va->invalids, &bo_va->valids);
> bo_va->cleared = clear;
> @@ -1811,14 +1789,14 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> bool clear;
> int r = 0;
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> while (!list_empty(&vm->moved)) {
> struct amdgpu_bo_va *bo_va;
> struct reservation_object *resv;
>
> bo_va = list_first_entry(&vm->moved,
> struct amdgpu_bo_va, base.vm_status);
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
>
> resv = bo_va->base.bo->tbo.resv;
>
> @@ -1839,9 +1817,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> if (!clear && resv != vm->root.base.bo->tbo.resv)
> reservation_object_unlock(resv);
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> }
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
>
> return r;
> }
> @@ -1903,10 +1881,10 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
> amdgpu_vm_prt_get(adev);
>
> if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> if (list_empty(&bo_va->base.vm_status))
> list_add(&bo_va->base.vm_status, &vm->moved);
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
> }
> trace_amdgpu_vm_bo_map(bo_va, mapping);
> }
> @@ -2216,9 +2194,9 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>
> list_del(&bo_va->base.bo_list);
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> list_del(&bo_va->base.vm_status);
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
>
> list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
> list_del(&mapping->list);
> @@ -2261,28 +2239,24 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>
> bo_base->moved = true;
> if (evicted && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
> - spin_lock(&bo_base->vm->status_lock);
> if (bo->tbo.type == ttm_bo_type_kernel)
> list_move(&bo_base->vm_status, &vm->evicted);
> else
> list_move_tail(&bo_base->vm_status,
> &vm->evicted);
> - spin_unlock(&bo_base->vm->status_lock);
> continue;
> }
>
> if (bo->tbo.type == ttm_bo_type_kernel) {
> - spin_lock(&bo_base->vm->status_lock);
> if (list_empty(&bo_base->vm_status))
> list_add(&bo_base->vm_status, &vm->relocated);
> - spin_unlock(&bo_base->vm->status_lock);
> continue;
> }
>
> - spin_lock(&bo_base->vm->status_lock);
> + spin_lock(&bo_base->vm->moved_lock);
> if (list_empty(&bo_base->vm_status))
> list_add(&bo_base->vm_status, &vm->moved);
> - spin_unlock(&bo_base->vm->status_lock);
> + spin_unlock(&bo_base->vm->moved_lock);
> }
> }
>
> @@ -2391,9 +2365,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> vm->va = RB_ROOT_CACHED;
> for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
> vm->reserved_vmid[i] = NULL;
> - spin_lock_init(&vm->status_lock);
> INIT_LIST_HEAD(&vm->evicted);
> INIT_LIST_HEAD(&vm->relocated);
> + spin_lock_init(&vm->moved_lock);
> INIT_LIST_HEAD(&vm->moved);
> INIT_LIST_HEAD(&vm->freed);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index d6827083572a..0196b9a782f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -168,9 +168,6 @@ struct amdgpu_vm {
> /* tree of virtual addresses mapped */
> struct rb_root_cached va;
>
> - /* protecting invalidated */
> - spinlock_t status_lock;
> -
> /* BOs who needs a validation */
> struct list_head evicted;
>
> @@ -179,6 +176,7 @@ struct amdgpu_vm {
>
> /* BOs moved, but not yet updated in the PT */
> struct list_head moved;
> + spinlock_t moved_lock;
>
> /* BO mappings freed, but not yet updated in the PT */
> struct list_head freed;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2
[not found] ` <5AFEA313.8040905-5C7GfCeVMHo@public.gmane.org>
@ 2018-05-19 8:00 ` Zhang, Jerry
[not found] ` <CY4PR12MB1414D3B8AE6ECEF5A9277A9AFF970-rpdhrqHFk04b0eJBKAp/BwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Jerry @ 2018-05-19 8:00 UTC (permalink / raw)
To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
> Patch 6:
> I could read that code, but not sure the purpose.
on 2nd thought, perhaps going to reduce the movement for per vm bo for performance increasing.
However, the move lru tail in amdgpu_vm_validate_pt_bos() looks change the per vm bo order again.
IIRC, it expects relative stable order for per vm bo to get a better performance.
Please confirm that.
Regards,
Jerry(Junwei Zhang)
________________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Zhang, Jerry (Junwei) <Jerry.Zhang@amd.com>
Sent: Friday, May 18, 2018 5:55:31 PM
To: Christian König; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2
2, 3, 4, 5 are
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
Patch 1:
could you show the reserving VM?
Patch 6:
I could read that code, but not sure the purpose.
Jerry
On 05/17/2018 05:49 PM, Christian König wrote:
> Only the moved state needs a separate spin lock protection. All other
> states are protected by reserving the VM anyway.
>
> v2: fix some more incorrect cases
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 66 +++++++++++-----------------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 +--
> 2 files changed, 21 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 1a8f4e0dd023..f0deedcaf1c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -119,9 +119,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> * is currently evicted. add the bo to the evicted list to make sure it
> * is validated on next vm use to avoid fault.
> * */
> - spin_lock(&vm->status_lock);
> list_move_tail(&base->vm_status, &vm->evicted);
> - spin_unlock(&vm->status_lock);
> }
>
> /**
> @@ -228,7 +226,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> struct ttm_bo_global *glob = adev->mman.bdev.glob;
> int r;
>
> - spin_lock(&vm->status_lock);
> while (!list_empty(&vm->evicted)) {
> struct amdgpu_vm_bo_base *bo_base;
> struct amdgpu_bo *bo;
> @@ -236,10 +233,8 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> bo_base = list_first_entry(&vm->evicted,
> struct amdgpu_vm_bo_base,
> vm_status);
> - spin_unlock(&vm->status_lock);
>
> bo = bo_base->bo;
> - BUG_ON(!bo);
> if (bo->parent) {
> r = validate(param, bo);
> if (r)
> @@ -259,13 +254,14 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> return r;
> }
>
> - spin_lock(&vm->status_lock);
> - if (bo->tbo.type != ttm_bo_type_kernel)
> + if (bo->tbo.type != ttm_bo_type_kernel) {
> + spin_lock(&vm->moved_lock);
> list_move(&bo_base->vm_status, &vm->moved);
> - else
> + spin_unlock(&vm->moved_lock);
> + } else {
> list_move(&bo_base->vm_status, &vm->relocated);
> + }
> }
> - spin_unlock(&vm->status_lock);
>
> return 0;
> }
> @@ -279,13 +275,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> */
> bool amdgpu_vm_ready(struct amdgpu_vm *vm)
> {
> - bool ready;
> -
> - spin_lock(&vm->status_lock);
> - ready = list_empty(&vm->evicted);
> - spin_unlock(&vm->status_lock);
> -
> - return ready;
> + return list_empty(&vm->evicted);
> }
>
> /**
> @@ -477,9 +467,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
> pt->parent = amdgpu_bo_ref(parent->base.bo);
>
> amdgpu_vm_bo_base_init(&entry->base, vm, pt);
> - spin_lock(&vm->status_lock);
> list_move(&entry->base.vm_status, &vm->relocated);
> - spin_unlock(&vm->status_lock);
> }
>
> if (level < AMDGPU_VM_PTB) {
> @@ -926,10 +914,8 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
> if (!entry->base.bo)
> continue;
>
> - spin_lock(&vm->status_lock);
> if (list_empty(&entry->base.vm_status))
> list_add(&entry->base.vm_status, &vm->relocated);
> - spin_unlock(&vm->status_lock);
> amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
> }
> }
> @@ -974,7 +960,6 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> params.func = amdgpu_vm_do_set_ptes;
> }
>
> - spin_lock(&vm->status_lock);
> while (!list_empty(&vm->relocated)) {
> struct amdgpu_vm_bo_base *bo_base, *parent;
> struct amdgpu_vm_pt *pt, *entry;
> @@ -984,13 +969,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> struct amdgpu_vm_bo_base,
> vm_status);
> list_del_init(&bo_base->vm_status);
> - spin_unlock(&vm->status_lock);
>
> bo = bo_base->bo->parent;
> - if (!bo) {
> - spin_lock(&vm->status_lock);
> + if (!bo)
> continue;
> - }
>
> parent = list_first_entry(&bo->va, struct amdgpu_vm_bo_base,
> bo_list);
> @@ -999,12 +981,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>
> amdgpu_vm_update_pde(¶ms, vm, pt, entry);
>
> - spin_lock(&vm->status_lock);
> if (!vm->use_cpu_for_update &&
> (ndw - params.ib->length_dw) < 32)
> break;
> }
> - spin_unlock(&vm->status_lock);
>
> if (vm->use_cpu_for_update) {
> /* Flush HDP */
> @@ -1107,9 +1087,7 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
> if (entry->huge) {
> /* Add the entry to the relocated list to update it. */
> entry->huge = false;
> - spin_lock(&p->vm->status_lock);
> list_move(&entry->base.vm_status, &p->vm->relocated);
> - spin_unlock(&p->vm->status_lock);
> }
> return;
> }
> @@ -1588,8 +1566,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> amdgpu_asic_flush_hdp(adev, NULL);
> }
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> list_del_init(&bo_va->base.vm_status);
> + spin_unlock(&vm->moved_lock);
>
> /* If the BO is not in its preferred location add it back to
> * the evicted list so that it gets validated again on the
> @@ -1599,7 +1578,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> !(bo->preferred_domains &
> amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)))
> list_add_tail(&bo_va->base.vm_status, &vm->evicted);
> - spin_unlock(&vm->status_lock);
>
> list_splice_init(&bo_va->invalids, &bo_va->valids);
> bo_va->cleared = clear;
> @@ -1811,14 +1789,14 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> bool clear;
> int r = 0;
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> while (!list_empty(&vm->moved)) {
> struct amdgpu_bo_va *bo_va;
> struct reservation_object *resv;
>
> bo_va = list_first_entry(&vm->moved,
> struct amdgpu_bo_va, base.vm_status);
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
>
> resv = bo_va->base.bo->tbo.resv;
>
> @@ -1839,9 +1817,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> if (!clear && resv != vm->root.base.bo->tbo.resv)
> reservation_object_unlock(resv);
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> }
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
>
> return r;
> }
> @@ -1903,10 +1881,10 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
> amdgpu_vm_prt_get(adev);
>
> if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> if (list_empty(&bo_va->base.vm_status))
> list_add(&bo_va->base.vm_status, &vm->moved);
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
> }
> trace_amdgpu_vm_bo_map(bo_va, mapping);
> }
> @@ -2216,9 +2194,9 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>
> list_del(&bo_va->base.bo_list);
>
> - spin_lock(&vm->status_lock);
> + spin_lock(&vm->moved_lock);
> list_del(&bo_va->base.vm_status);
> - spin_unlock(&vm->status_lock);
> + spin_unlock(&vm->moved_lock);
>
> list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
> list_del(&mapping->list);
> @@ -2261,28 +2239,24 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>
> bo_base->moved = true;
> if (evicted && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
> - spin_lock(&bo_base->vm->status_lock);
> if (bo->tbo.type == ttm_bo_type_kernel)
> list_move(&bo_base->vm_status, &vm->evicted);
> else
> list_move_tail(&bo_base->vm_status,
> &vm->evicted);
> - spin_unlock(&bo_base->vm->status_lock);
> continue;
> }
>
> if (bo->tbo.type == ttm_bo_type_kernel) {
> - spin_lock(&bo_base->vm->status_lock);
> if (list_empty(&bo_base->vm_status))
> list_add(&bo_base->vm_status, &vm->relocated);
> - spin_unlock(&bo_base->vm->status_lock);
> continue;
> }
>
> - spin_lock(&bo_base->vm->status_lock);
> + spin_lock(&bo_base->vm->moved_lock);
> if (list_empty(&bo_base->vm_status))
> list_add(&bo_base->vm_status, &vm->moved);
> - spin_unlock(&bo_base->vm->status_lock);
> + spin_unlock(&bo_base->vm->moved_lock);
> }
> }
>
> @@ -2391,9 +2365,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> vm->va = RB_ROOT_CACHED;
> for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
> vm->reserved_vmid[i] = NULL;
> - spin_lock_init(&vm->status_lock);
> INIT_LIST_HEAD(&vm->evicted);
> INIT_LIST_HEAD(&vm->relocated);
> + spin_lock_init(&vm->moved_lock);
> INIT_LIST_HEAD(&vm->moved);
> INIT_LIST_HEAD(&vm->freed);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index d6827083572a..0196b9a782f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -168,9 +168,6 @@ struct amdgpu_vm {
> /* tree of virtual addresses mapped */
> struct rb_root_cached va;
>
> - /* protecting invalidated */
> - spinlock_t status_lock;
> -
> /* BOs who needs a validation */
> struct list_head evicted;
>
> @@ -179,6 +176,7 @@ struct amdgpu_vm {
>
> /* BOs moved, but not yet updated in the PT */
> struct list_head moved;
> + spinlock_t moved_lock;
>
> /* BO mappings freed, but not yet updated in the PT */
> struct list_head freed;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2
[not found] ` <CY4PR12MB1414D3B8AE6ECEF5A9277A9AFF970-rpdhrqHFk04b0eJBKAp/BwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-05-22 13:07 ` Christian König
0 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2018-05-22 13:07 UTC (permalink / raw)
To: Zhang, Jerry, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
> Patch 1:
> could you show the reserving VM?
The idea is that only the moved state is accessed without the VM being
reserved.
Because of this we can remove the locks when we a) are sure that the BO
isn't in the moved state and b) don't want to move it into the moved state.
> on 2nd thought, perhaps going to reduce the movement for per vm bo for performance increasing.
>
> However, the move lru tail in amdgpu_vm_validate_pt_bos() looks change the per vm bo order again.
>
> IIRC, it expects relative stable order for per vm bo to get a better performance.
What can happen is that a BO is not evicted because we need visible VRAM.
But the gross of all BOs should be evicted in the order they are
allocated and are then moved back into VRAM in the order they are
allocated etc...
That isn't 100% stable but still seems to be good enough.
Regards,
Christian.
Am 19.05.2018 um 10:00 schrieb Zhang, Jerry:
>> Patch 6:
>> I could read that code, but not sure the purpose.
> on 2nd thought, perhaps going to reduce the movement for per vm bo for performance increasing.
>
> However, the move lru tail in amdgpu_vm_validate_pt_bos() looks change the per vm bo order again.
>
> IIRC, it expects relative stable order for per vm bo to get a better performance.
>
> Please confirm that.
>
> Regards,
> Jerry(Junwei Zhang)
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Zhang, Jerry (Junwei) <Jerry.Zhang@amd.com>
> Sent: Friday, May 18, 2018 5:55:31 PM
> To: Christian König; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2
>
> 2, 3, 4, 5 are
> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
>
> Patch 1:
> could you show the reserving VM?
>
> Patch 6:
> I could read that code, but not sure the purpose.
>
> Jerry
>
> On 05/17/2018 05:49 PM, Christian König wrote:
>> Only the moved state needs a separate spin lock protection. All other
>> states are protected by reserving the VM anyway.
>>
>> v2: fix some more incorrect cases
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 66 +++++++++++-----------------------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 +--
>> 2 files changed, 21 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 1a8f4e0dd023..f0deedcaf1c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -119,9 +119,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>> * is currently evicted. add the bo to the evicted list to make sure it
>> * is validated on next vm use to avoid fault.
>> * */
>> - spin_lock(&vm->status_lock);
>> list_move_tail(&base->vm_status, &vm->evicted);
>> - spin_unlock(&vm->status_lock);
>> }
>>
>> /**
>> @@ -228,7 +226,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>> struct ttm_bo_global *glob = adev->mman.bdev.glob;
>> int r;
>>
>> - spin_lock(&vm->status_lock);
>> while (!list_empty(&vm->evicted)) {
>> struct amdgpu_vm_bo_base *bo_base;
>> struct amdgpu_bo *bo;
>> @@ -236,10 +233,8 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>> bo_base = list_first_entry(&vm->evicted,
>> struct amdgpu_vm_bo_base,
>> vm_status);
>> - spin_unlock(&vm->status_lock);
>>
>> bo = bo_base->bo;
>> - BUG_ON(!bo);
>> if (bo->parent) {
>> r = validate(param, bo);
>> if (r)
>> @@ -259,13 +254,14 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>> return r;
>> }
>>
>> - spin_lock(&vm->status_lock);
>> - if (bo->tbo.type != ttm_bo_type_kernel)
>> + if (bo->tbo.type != ttm_bo_type_kernel) {
>> + spin_lock(&vm->moved_lock);
>> list_move(&bo_base->vm_status, &vm->moved);
>> - else
>> + spin_unlock(&vm->moved_lock);
>> + } else {
>> list_move(&bo_base->vm_status, &vm->relocated);
>> + }
>> }
>> - spin_unlock(&vm->status_lock);
>>
>> return 0;
>> }
>> @@ -279,13 +275,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>> */
>> bool amdgpu_vm_ready(struct amdgpu_vm *vm)
>> {
>> - bool ready;
>> -
>> - spin_lock(&vm->status_lock);
>> - ready = list_empty(&vm->evicted);
>> - spin_unlock(&vm->status_lock);
>> -
>> - return ready;
>> + return list_empty(&vm->evicted);
>> }
>>
>> /**
>> @@ -477,9 +467,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>> pt->parent = amdgpu_bo_ref(parent->base.bo);
>>
>> amdgpu_vm_bo_base_init(&entry->base, vm, pt);
>> - spin_lock(&vm->status_lock);
>> list_move(&entry->base.vm_status, &vm->relocated);
>> - spin_unlock(&vm->status_lock);
>> }
>>
>> if (level < AMDGPU_VM_PTB) {
>> @@ -926,10 +914,8 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
>> if (!entry->base.bo)
>> continue;
>>
>> - spin_lock(&vm->status_lock);
>> if (list_empty(&entry->base.vm_status))
>> list_add(&entry->base.vm_status, &vm->relocated);
>> - spin_unlock(&vm->status_lock);
>> amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
>> }
>> }
>> @@ -974,7 +960,6 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>> params.func = amdgpu_vm_do_set_ptes;
>> }
>>
>> - spin_lock(&vm->status_lock);
>> while (!list_empty(&vm->relocated)) {
>> struct amdgpu_vm_bo_base *bo_base, *parent;
>> struct amdgpu_vm_pt *pt, *entry;
>> @@ -984,13 +969,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>> struct amdgpu_vm_bo_base,
>> vm_status);
>> list_del_init(&bo_base->vm_status);
>> - spin_unlock(&vm->status_lock);
>>
>> bo = bo_base->bo->parent;
>> - if (!bo) {
>> - spin_lock(&vm->status_lock);
>> + if (!bo)
>> continue;
>> - }
>>
>> parent = list_first_entry(&bo->va, struct amdgpu_vm_bo_base,
>> bo_list);
>> @@ -999,12 +981,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>>
>> amdgpu_vm_update_pde(¶ms, vm, pt, entry);
>>
>> - spin_lock(&vm->status_lock);
>> if (!vm->use_cpu_for_update &&
>> (ndw - params.ib->length_dw) < 32)
>> break;
>> }
>> - spin_unlock(&vm->status_lock);
>>
>> if (vm->use_cpu_for_update) {
>> /* Flush HDP */
>> @@ -1107,9 +1087,7 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
>> if (entry->huge) {
>> /* Add the entry to the relocated list to update it. */
>> entry->huge = false;
>> - spin_lock(&p->vm->status_lock);
>> list_move(&entry->base.vm_status, &p->vm->relocated);
>> - spin_unlock(&p->vm->status_lock);
>> }
>> return;
>> }
>> @@ -1588,8 +1566,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>> amdgpu_asic_flush_hdp(adev, NULL);
>> }
>>
>> - spin_lock(&vm->status_lock);
>> + spin_lock(&vm->moved_lock);
>> list_del_init(&bo_va->base.vm_status);
>> + spin_unlock(&vm->moved_lock);
>>
>> /* If the BO is not in its preferred location add it back to
>> * the evicted list so that it gets validated again on the
>> @@ -1599,7 +1578,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>> !(bo->preferred_domains &
>> amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)))
>> list_add_tail(&bo_va->base.vm_status, &vm->evicted);
>> - spin_unlock(&vm->status_lock);
>>
>> list_splice_init(&bo_va->invalids, &bo_va->valids);
>> bo_va->cleared = clear;
>> @@ -1811,14 +1789,14 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>> bool clear;
>> int r = 0;
>>
>> - spin_lock(&vm->status_lock);
>> + spin_lock(&vm->moved_lock);
>> while (!list_empty(&vm->moved)) {
>> struct amdgpu_bo_va *bo_va;
>> struct reservation_object *resv;
>>
>> bo_va = list_first_entry(&vm->moved,
>> struct amdgpu_bo_va, base.vm_status);
>> - spin_unlock(&vm->status_lock);
>> + spin_unlock(&vm->moved_lock);
>>
>> resv = bo_va->base.bo->tbo.resv;
>>
>> @@ -1839,9 +1817,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>> if (!clear && resv != vm->root.base.bo->tbo.resv)
>> reservation_object_unlock(resv);
>>
>> - spin_lock(&vm->status_lock);
>> + spin_lock(&vm->moved_lock);
>> }
>> - spin_unlock(&vm->status_lock);
>> + spin_unlock(&vm->moved_lock);
>>
>> return r;
>> }
>> @@ -1903,10 +1881,10 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
>> amdgpu_vm_prt_get(adev);
>>
>> if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
>> - spin_lock(&vm->status_lock);
>> + spin_lock(&vm->moved_lock);
>> if (list_empty(&bo_va->base.vm_status))
>> list_add(&bo_va->base.vm_status, &vm->moved);
>> - spin_unlock(&vm->status_lock);
>> + spin_unlock(&vm->moved_lock);
>> }
>> trace_amdgpu_vm_bo_map(bo_va, mapping);
>> }
>> @@ -2216,9 +2194,9 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>>
>> list_del(&bo_va->base.bo_list);
>>
>> - spin_lock(&vm->status_lock);
>> + spin_lock(&vm->moved_lock);
>> list_del(&bo_va->base.vm_status);
>> - spin_unlock(&vm->status_lock);
>> + spin_unlock(&vm->moved_lock);
>>
>> list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
>> list_del(&mapping->list);
>> @@ -2261,28 +2239,24 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>>
>> bo_base->moved = true;
>> if (evicted && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
>> - spin_lock(&bo_base->vm->status_lock);
>> if (bo->tbo.type == ttm_bo_type_kernel)
>> list_move(&bo_base->vm_status, &vm->evicted);
>> else
>> list_move_tail(&bo_base->vm_status,
>> &vm->evicted);
>> - spin_unlock(&bo_base->vm->status_lock);
>> continue;
>> }
>>
>> if (bo->tbo.type == ttm_bo_type_kernel) {
>> - spin_lock(&bo_base->vm->status_lock);
>> if (list_empty(&bo_base->vm_status))
>> list_add(&bo_base->vm_status, &vm->relocated);
>> - spin_unlock(&bo_base->vm->status_lock);
>> continue;
>> }
>>
>> - spin_lock(&bo_base->vm->status_lock);
>> + spin_lock(&bo_base->vm->moved_lock);
>> if (list_empty(&bo_base->vm_status))
>> list_add(&bo_base->vm_status, &vm->moved);
>> - spin_unlock(&bo_base->vm->status_lock);
>> + spin_unlock(&bo_base->vm->moved_lock);
>> }
>> }
>>
>> @@ -2391,9 +2365,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>> vm->va = RB_ROOT_CACHED;
>> for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
>> vm->reserved_vmid[i] = NULL;
>> - spin_lock_init(&vm->status_lock);
>> INIT_LIST_HEAD(&vm->evicted);
>> INIT_LIST_HEAD(&vm->relocated);
>> + spin_lock_init(&vm->moved_lock);
>> INIT_LIST_HEAD(&vm->moved);
>> INIT_LIST_HEAD(&vm->freed);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index d6827083572a..0196b9a782f2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -168,9 +168,6 @@ struct amdgpu_vm {
>> /* tree of virtual addresses mapped */
>> struct rb_root_cached va;
>>
>> - /* protecting invalidated */
>> - spinlock_t status_lock;
>> -
>> /* BOs who needs a validation */
>> struct list_head evicted;
>>
>> @@ -179,6 +176,7 @@ struct amdgpu_vm {
>>
>> /* BOs moved, but not yet updated in the PT */
>> struct list_head moved;
>> + spinlock_t moved_lock;
>>
>> /* BO mappings freed, but not yet updated in the PT */
>> struct list_head freed;
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-05-22 13:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-17 9:49 [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2 Christian König
[not found] ` <20180517094936.4293-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-05-17 9:49 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_validate_pt_bos v2 Christian König
2018-05-17 9:49 ` [PATCH 3/6] drm/amdgpu: further optimize amdgpu_vm_handle_moved Christian König
2018-05-17 9:49 ` [PATCH 4/6] drm/amdgpu: kmap PDs/PTs in amdgpu_vm_update_directories Christian König
2018-05-17 9:49 ` [PATCH 5/6] drm/amdgpu: consistenly use VM moved flag Christian König
2018-05-17 9:49 ` [PATCH 6/6] drm/amdgpu: move VM BOs on LRU again Christian König
[not found] ` <20180517094936.4293-6-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-05-18 6:49 ` zhoucm1
2018-05-18 9:55 ` [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2 Zhang, Jerry (Junwei)
[not found] ` <5AFEA313.8040905-5C7GfCeVMHo@public.gmane.org>
2018-05-19 8:00 ` Zhang, Jerry
[not found] ` <CY4PR12MB1414D3B8AE6ECEF5A9277A9AFF970-rpdhrqHFk04b0eJBKAp/BwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-05-22 13:07 ` Christian König
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.