[PATCH 1/4] drm/amdgpu: remove unused member

[PATCH 1/4] drm/amdgpu: remove unused member

[Christian König]

This lock isn't used any more.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 30f080364c97..aef897b93866 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -186,9 +186,6 @@ struct amdgpu_vm {
 	struct amdgpu_vm_pt     root;
 	struct dma_fence	*last_update;
 
-	/* protecting freed */
-	spinlock_t		freed_lock;
-
 	/* Scheduler entity for page table updates */
 	struct drm_sched_entity	entity;
 
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/4] drm/amdgpu: rework VM state machine lock handling

[Christian König]

Only the moved state needs a separate spin lock protection. All other
states are protected by reserving the VM anyway.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 ++++++++++++----------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 +-
 2 files changed, 25 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6a372ca11ee3..dbf24b628835 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -198,7 +198,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;
@@ -206,10 +205,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)
@@ -229,13 +226,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;
 }
@@ -249,13 +247,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);
 }
 
 /**
@@ -449,9 +441,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 			entry->base.vm = vm;
 			entry->base.bo = pt;
 			list_add_tail(&entry->base.bo_list, &pt->va);
-			spin_lock(&vm->status_lock);
 			list_add(&entry->base.vm_status, &vm->relocated);
-			spin_unlock(&vm->status_lock);
 		}
 
 		if (level < AMDGPU_VM_PTB) {
@@ -898,10 +888,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);
 	}
 }
@@ -946,7 +934,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;
@@ -956,13 +943,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);
@@ -971,12 +955,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
 
 		amdgpu_vm_update_pde(&params, 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 */
@@ -1079,9 +1061,9 @@ 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);
+			spin_lock(&p->vm->moved_lock);
 			list_move(&entry->base.vm_status, &p->vm->relocated);
-			spin_unlock(&p->vm->status_lock);
+			spin_unlock(&p->vm->moved_lock);
 		}
 		return;
 	}
@@ -1561,8 +1543,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
@@ -1572,7 +1555,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv &&
 	    !(bo->preferred_domains & amdgpu_mem_type_to_domain(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;
@@ -1784,14 +1766,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;
 
@@ -1812,9 +1794,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;
 }
@@ -1868,9 +1850,9 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	 * 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);
+	spin_lock(&vm->moved_lock);
 	list_move_tail(&bo_va->base.vm_status, &vm->evicted);
-	spin_unlock(&vm->status_lock);
+	spin_unlock(&vm->moved_lock);
 
 	return bo_va;
 }
@@ -1900,10 +1882,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);
 }
@@ -2213,9 +2195,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);
@@ -2254,28 +2236,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);
 	}
 }
 
@@ -2383,9 +2361,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 aef897b93866..4e75b6959446 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -167,9 +167,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;
 
@@ -178,6 +175,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

[PATCH 3/4] drm/amdgpu: further optimize amdgpu_vm_validate_pt_bos

[Christian König]

Put all moved BOs on a temporary list and splice that one to the moved
list when we are done. Saves us taking and releasing the moved lock multiple times.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dbf24b628835..9da476637574 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -196,21 +196,18 @@ 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;
+	struct amdgpu_vm_bo_base *bo_base, *tmp;
+	struct list_head moved;
+	int r = 0;
 
-	while (!list_empty(&vm->evicted)) {
-		struct amdgpu_vm_bo_base *bo_base;
-		struct amdgpu_bo *bo;
+	INIT_LIST_HEAD(&moved);
+	list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
+		struct amdgpu_bo *bo = bo_base->bo;
 
-		bo_base = list_first_entry(&vm->evicted,
-					   struct amdgpu_vm_bo_base,
-					   vm_status);
-
-		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);
@@ -223,19 +220,20 @@ 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) {
-			spin_lock(&vm->moved_lock);
-			list_move(&bo_base->vm_status, &vm->moved);
-			spin_unlock(&vm->moved_lock);
-		} else {
+		if (bo->tbo.type != ttm_bo_type_kernel)
+			list_move(&bo_base->vm_status, &moved);
+		else
 			list_move(&bo_base->vm_status, &vm->relocated);
-		}
 	}
 
-	return 0;
+	spin_lock(&vm->moved_lock);
+	list_splice_tail(&moved, &vm->moved);
+	spin_unlock(&vm->moved_lock);
+
+	return r;
 }
 
 /**
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/4] drm/amdgpu: further optimize amdgpu_vm_handle_moved

[Christian König]

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 9da476637574..9d4ab7f70b1f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1761,19 +1761,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)
@@ -1786,17 +1785,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

Re: [PATCH 3/4] drm/amdgpu: further optimize amdgpu_vm_validate_pt_bos

[Christian König]

Please ignore this one, it doesn't work correctly.

Validating a BO can put it back on the moved list.

Christian.

Am 19.04.2018 um 11:29 schrieb Christian König:
> Put all moved BOs on a temporary list and splice that one to the moved
> list when we are done. Saves us taking and releasing the moved lock multiple times.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 ++++++++++++++++------------------
>   1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index dbf24b628835..9da476637574 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -196,21 +196,18 @@ 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;
> +	struct amdgpu_vm_bo_base *bo_base, *tmp;
> +	struct list_head moved;
> +	int r = 0;
>   
> -	while (!list_empty(&vm->evicted)) {
> -		struct amdgpu_vm_bo_base *bo_base;
> -		struct amdgpu_bo *bo;
> +	INIT_LIST_HEAD(&moved);
> +	list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
> +		struct amdgpu_bo *bo = bo_base->bo;
>   
> -		bo_base = list_first_entry(&vm->evicted,
> -					   struct amdgpu_vm_bo_base,
> -					   vm_status);
> -
> -		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);
> @@ -223,19 +220,20 @@ 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) {
> -			spin_lock(&vm->moved_lock);
> -			list_move(&bo_base->vm_status, &vm->moved);
> -			spin_unlock(&vm->moved_lock);
> -		} else {
> +		if (bo->tbo.type != ttm_bo_type_kernel)
> +			list_move(&bo_base->vm_status, &moved);
> +		else
>   			list_move(&bo_base->vm_status, &vm->relocated);
> -		}
>   	}
>   
> -	return 0;
> +	spin_lock(&vm->moved_lock);
> +	list_splice_tail(&moved, &vm->moved);
> +	spin_unlock(&vm->moved_lock);
> +
> +	return r;
>   }
>   
>   /**

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/4] drm/amdgpu: remove unused member

[zhoucm1]

On 2018年04月19日 17:29, Christian König wrote:
> This lock isn't used any more.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 30f080364c97..aef897b93866 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -186,9 +186,6 @@ struct amdgpu_vm {
>   	struct amdgpu_vm_pt     root;
>   	struct dma_fence	*last_update;
>   
> -	/* protecting freed */
> -	spinlock_t		freed_lock;
> -
>   	/* Scheduler entity for page table updates */
>   	struct drm_sched_entity	entity;
>   

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/4] drm/amdgpu: further optimize amdgpu_vm_handle_moved

[zhoucm1]

I failed to git am the patch to test, the patch format has problem?

 From the patch self, is it possible that the vm->moved list is updated 
when you use local one?


Regards,

David Zhou


On 2018年04月19日 17:29, Christian König wrote:
> 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 9da476637574..9d4ab7f70b1f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1761,19 +1761,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)
> @@ -1786,17 +1785,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;
>   }
>   
>   /**

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx