All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed
@ 2017-03-23 19:27 Nicolai Hähnle
       [not found] ` <20170323192705.5474-1-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Nicolai Hähnle @ 2017-03-23 19:27 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Nicolai Hähnle

From: Nicolai Hähnle <nicolai.haehnle@amd.com>

We will add the fence to freed buffer objects in a later commit, to ensure
that the underlying memory can only be re-used after all references in
page tables have been cleared.

Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 55d553a..85e6070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
 	int i, r;
 
 	r = amdgpu_vm_update_page_directory(adev, vm);
 	if (r)
 		return r;
 
 	r = amdgpu_sync_fence(adev, &p->job->sync, vm->page_directory_fence);
 	if (r)
 		return r;
 
-	r = amdgpu_vm_clear_freed(adev, vm);
+	r = amdgpu_vm_clear_freed(adev, vm, NULL);
 	if (r)
 		return r;
 
 	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
 	if (r)
 		return r;
 
 	r = amdgpu_sync_fence(adev, &p->job->sync,
 			      fpriv->prt_va->last_pt_update);
 	if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index be9fb2c..4a53c43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 
 	r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,
 				      NULL);
 	if (r)
 		goto error;
 
 	r = amdgpu_vm_update_page_directory(adev, vm);
 	if (r)
 		goto error;
 
-	r = amdgpu_vm_clear_freed(adev, vm);
+	r = amdgpu_vm_clear_freed(adev, vm, NULL);
 	if (r)
 		goto error;
 
 	if (operation == AMDGPU_VA_OP_MAP ||
 	    operation == AMDGPU_VA_OP_REPLACE)
 		r = amdgpu_vm_bo_update(adev, bo_va, false);
 
 error:
 	if (r && r != -ERESTARTSYS)
 		DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dd7df45..2c95a75 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	}
 
 	kfree(shared);
 }
 
 /**
  * amdgpu_vm_clear_freed - clear freed BOs in the PT
  *
  * @adev: amdgpu_device pointer
  * @vm: requested vm
+ * @fence: optional resulting fence (unchanged if no work needed to be done
+ * or if an error occurred)
  *
  * Make sure all freed BOs are cleared in the PT.
  * Returns 0 for success.
  *
  * PTs have to be reserved and mutex must be locked!
  */
 int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
-			  struct amdgpu_vm *vm)
+			  struct amdgpu_vm *vm,
+			  struct fence **fence)
 {
 	struct amdgpu_bo_va_mapping *mapping;
-	struct fence *fence = NULL;
+	struct fence *f = NULL;
 	int r;
 
 	while (!list_empty(&vm->freed)) {
 		mapping = list_first_entry(&vm->freed,
 			struct amdgpu_bo_va_mapping, list);
 		list_del(&mapping->list);
 
 		r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,
-					       0, 0, &fence);
-		amdgpu_vm_free_mapping(adev, vm, mapping, fence);
+					       0, 0, &f);
+		amdgpu_vm_free_mapping(adev, vm, mapping, f);
 		if (r) {
-			fence_put(fence);
+			fence_put(f);
 			return r;
 		}
+	}
 
+	if (fence && f) {
+		fence_put(*fence);
+		*fence = f;
+	} else {
+		fence_put(f);
 	}
-	fence_put(fence);
+
 	return 0;
 
 }
 
 /**
  * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
  *
  * @adev: amdgpu_device pointer
  * @vm: requested vm
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index ff10fa5..9d5a572 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 			struct amdgpu_vm *vm,
 			uint64_t saddr, uint64_t size);
 int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
 		      struct amdgpu_sync *sync, struct fence *fence,
 		      struct amdgpu_job *job);
 int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job);
 void amdgpu_vm_reset_id(struct amdgpu_device *adev, unsigned vm_id);
 int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
 				    struct amdgpu_vm *vm);
 int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
-			  struct amdgpu_vm *vm);
+			  struct amdgpu_vm *vm,
+			  struct fence **fence);
 int amdgpu_vm_clear_invalids(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			     struct amdgpu_sync *sync);
 int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 			struct amdgpu_bo_va *bo_va,
 			bool clear);
 void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
 			     struct amdgpu_bo *bo);
 struct amdgpu_bo_va *amdgpu_vm_bo_find(struct amdgpu_vm *vm,
 				       struct amdgpu_bo *bo);
 struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] drm/amdgpu: clear freed mappings immediately when BO may be freed
       [not found] ` <20170323192705.5474-1-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-03-23 19:27   ` Nicolai Hähnle
       [not found]     ` <20170323192705.5474-2-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-03-24  2:30   ` [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed zhoucm1
  2017-03-24  7:40   ` Christian König
  2 siblings, 1 reply; 8+ messages in thread
From: Nicolai Hähnle @ 2017-03-23 19:27 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Nicolai Hähnle

From: Nicolai Hähnle <nicolai.haehnle@amd.com>

Also, add the fence of the clear operations to the BO to ensure that
the underlying memory can only be re-used after all PTEs pointing to
it have been cleared.

This avoids the following sequence of events that could be triggered
by user space:

1. Submit a CS that accesses some BO _without_ adding that BO to the
   buffer list.
2. Free that BO.
3. Some other task re-uses the memory underlying the BO.
4. The CS is submitted to the hardware and accesses memory that is
   now already in use by somebody else.

By clearing the page tables immediately in step 2, a GPU VM fault will
be triggered in step 4 instead of wild memory accesses.

v2: use amdgpu_bo_fence directly

Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 4a53c43..8b0f5f18 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -145,20 +145,21 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
 	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 	struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
 	struct amdgpu_vm *vm = &fpriv->vm;
 
 	struct amdgpu_bo_list_entry vm_pd;
 	struct list_head list, duplicates;
 	struct ttm_validate_buffer tv;
 	struct ww_acquire_ctx ticket;
 	struct amdgpu_bo_va *bo_va;
+	struct fence *fence = NULL;
 	int r;
 
 	INIT_LIST_HEAD(&list);
 	INIT_LIST_HEAD(&duplicates);
 
 	tv.bo = &bo->tbo;
 	tv.shared = true;
 	list_add(&tv.head, &list);
 
 	amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
@@ -166,20 +167,31 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
 	r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
 	if (r) {
 		dev_err(adev->dev, "leaking bo va because "
 			"we fail to reserve bo (%d)\n", r);
 		return;
 	}
 	bo_va = amdgpu_vm_bo_find(vm, bo);
 	if (bo_va) {
 		if (--bo_va->ref_count == 0) {
 			amdgpu_vm_bo_rmv(adev, bo_va);
+
+			r = amdgpu_vm_clear_freed(adev, vm, &fence);
+			if (unlikely(r)) {
+				dev_err(adev->dev, "failed to clear page "
+					"tables on GEM object close (%d)\n", r);
+			}
+
+			if (fence) {
+				amdgpu_bo_fence(bo, fence, true);
+				fence_put(fence);
+			}
 		}
 	}
 	ttm_eu_backoff_reservation(&ticket, &list);
 }
 
 static int amdgpu_gem_handle_lockup(struct amdgpu_device *adev, int r)
 {
 	if (r == -EDEADLK) {
 		r = amdgpu_gpu_reset(adev);
 		if (!r)
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed
       [not found] ` <20170323192705.5474-1-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-03-23 19:27   ` [PATCH v2 2/2] drm/amdgpu: clear freed mappings immediately when BO may be freed Nicolai Hähnle
@ 2017-03-24  2:30   ` zhoucm1
       [not found]     ` <58D484CB.1020306-5C7GfCeVMHo@public.gmane.org>
  2017-03-24  7:40   ` Christian König
  2 siblings, 1 reply; 8+ messages in thread
From: zhoucm1 @ 2017-03-24  2:30 UTC (permalink / raw)
  To: Nicolai Hähnle, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Nicolai Hähnle



On 2017年03月24日 03:27, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>
> We will add the fence to freed buffer objects in a later commit, to ensure
> that the underlying memory can only be re-used after all references in
> page tables have been cleared.
>
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
>   4 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 55d553a..85e6070 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>   	int i, r;
>   
>   	r = amdgpu_vm_update_page_directory(adev, vm);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_sync_fence(adev, &p->job->sync, vm->page_directory_fence);
>   	if (r)
>   		return r;
>   
> -	r = amdgpu_vm_clear_freed(adev, vm);
> +	r = amdgpu_vm_clear_freed(adev, vm, NULL);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_sync_fence(adev, &p->job->sync,
>   			      fpriv->prt_va->last_pt_update);
>   	if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index be9fb2c..4a53c43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
>   
>   	r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,
>   				      NULL);
>   	if (r)
>   		goto error;
>   
>   	r = amdgpu_vm_update_page_directory(adev, vm);
>   	if (r)
>   		goto error;
>   
> -	r = amdgpu_vm_clear_freed(adev, vm);
> +	r = amdgpu_vm_clear_freed(adev, vm, NULL);
>   	if (r)
>   		goto error;
>   
>   	if (operation == AMDGPU_VA_OP_MAP ||
>   	    operation == AMDGPU_VA_OP_REPLACE)
>   		r = amdgpu_vm_bo_update(adev, bo_va, false);
>   
>   error:
>   	if (r && r != -ERESTARTSYS)
>   		DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index dd7df45..2c95a75 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   	}
>   
>   	kfree(shared);
>   }
>   
>   /**
>    * amdgpu_vm_clear_freed - clear freed BOs in the PT
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
> + * @fence: optional resulting fence (unchanged if no work needed to be done
> + * or if an error occurred)
>    *
>    * Make sure all freed BOs are cleared in the PT.
>    * Returns 0 for success.
>    *
>    * PTs have to be reserved and mutex must be locked!
>    */
>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> -			  struct amdgpu_vm *vm)
> +			  struct amdgpu_vm *vm,
> +			  struct fence **fence)
>   {
>   	struct amdgpu_bo_va_mapping *mapping;
> -	struct fence *fence = NULL;
> +	struct fence *f = NULL;
>   	int r;
>   
>   	while (!list_empty(&vm->freed)) {
>   		mapping = list_first_entry(&vm->freed,
>   			struct amdgpu_bo_va_mapping, list);
>   		list_del(&mapping->list);
>   
>   		r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,
> -					       0, 0, &fence);
> -		amdgpu_vm_free_mapping(adev, vm, mapping, fence);
> +					       0, 0, &f);
> +		amdgpu_vm_free_mapping(adev, vm, mapping, f);
>   		if (r) {
> -			fence_put(fence);
> +			fence_put(f);
>   			return r;
>   		}
> +	}
>   
> +	if (fence && f) {
> +		fence_put(*fence);
> +		*fence = f;
> +	} else {
> +		fence_put(f);
>   	}
> -	fence_put(fence);
> +
>   	return 0;
>   
>   }
>   
>   /**
>    * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index ff10fa5..9d5a572 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   			struct amdgpu_vm *vm,
>   			uint64_t saddr, uint64_t size);
>   int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
>   		      struct amdgpu_sync *sync, struct fence *fence,
>   		      struct amdgpu_job *job);
>   int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job);
>   void amdgpu_vm_reset_id(struct amdgpu_device *adev, unsigned vm_id);
>   int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
>   				    struct amdgpu_vm *vm);
>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> -			  struct amdgpu_vm *vm);
> +			  struct amdgpu_vm *vm,
> +			  struct fence **fence);
>   int amdgpu_vm_clear_invalids(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			     struct amdgpu_sync *sync);
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va *bo_va,
>   			bool clear);
>   void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   			     struct amdgpu_bo *bo);
>   struct amdgpu_bo_va *amdgpu_vm_bo_find(struct amdgpu_vm *vm,
>   				       struct amdgpu_bo *bo);
>   struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] drm/amdgpu: clear freed mappings immediately when BO may be freed
       [not found]     ` <20170323192705.5474-2-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-03-24  2:31       ` zhoucm1
  2017-03-24  3:42       ` Zhang, Jerry (Junwei)
  1 sibling, 0 replies; 8+ messages in thread
From: zhoucm1 @ 2017-03-24  2:31 UTC (permalink / raw)
  To: Nicolai Hähnle, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Nicolai Hähnle



On 2017年03月24日 03:27, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>
> Also, add the fence of the clear operations to the BO to ensure that
> the underlying memory can only be re-used after all PTEs pointing to
> it have been cleared.
>
> This avoids the following sequence of events that could be triggered
> by user space:
>
> 1. Submit a CS that accesses some BO _without_ adding that BO to the
>     buffer list.
> 2. Free that BO.
> 3. Some other task re-uses the memory underlying the BO.
> 4. The CS is submitted to the hardware and accesses memory that is
>     now already in use by somebody else.
>
> By clearing the page tables immediately in step 2, a GPU VM fault will
> be triggered in step 4 instead of wild memory accesses.
>
> v2: use amdgpu_bo_fence directly
>
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 4a53c43..8b0f5f18 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -145,20 +145,21 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>   	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>   	struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
>   	struct amdgpu_vm *vm = &fpriv->vm;
>   
>   	struct amdgpu_bo_list_entry vm_pd;
>   	struct list_head list, duplicates;
>   	struct ttm_validate_buffer tv;
>   	struct ww_acquire_ctx ticket;
>   	struct amdgpu_bo_va *bo_va;
> +	struct fence *fence = NULL;
>   	int r;
>   
>   	INIT_LIST_HEAD(&list);
>   	INIT_LIST_HEAD(&duplicates);
>   
>   	tv.bo = &bo->tbo;
>   	tv.shared = true;
>   	list_add(&tv.head, &list);
>   
>   	amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
> @@ -166,20 +167,31 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>   	r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
>   	if (r) {
>   		dev_err(adev->dev, "leaking bo va because "
>   			"we fail to reserve bo (%d)\n", r);
>   		return;
>   	}
>   	bo_va = amdgpu_vm_bo_find(vm, bo);
>   	if (bo_va) {
>   		if (--bo_va->ref_count == 0) {
>   			amdgpu_vm_bo_rmv(adev, bo_va);
> +
> +			r = amdgpu_vm_clear_freed(adev, vm, &fence);
> +			if (unlikely(r)) {
> +				dev_err(adev->dev, "failed to clear page "
> +					"tables on GEM object close (%d)\n", r);
> +			}
> +
> +			if (fence) {
> +				amdgpu_bo_fence(bo, fence, true);
> +				fence_put(fence);
> +			}
>   		}
>   	}
>   	ttm_eu_backoff_reservation(&ticket, &list);
>   }
>   
>   static int amdgpu_gem_handle_lockup(struct amdgpu_device *adev, int r)
>   {
>   	if (r == -EDEADLK) {
>   		r = amdgpu_gpu_reset(adev);
>   		if (!r)

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed
       [not found]     ` <58D484CB.1020306-5C7GfCeVMHo@public.gmane.org>
@ 2017-03-24  3:40       ` Zhang, Jerry (Junwei)
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Jerry (Junwei) @ 2017-03-24  3:40 UTC (permalink / raw)
  To: zhoucm1, Nicolai Hähnle, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Nicolai Hähnle

On 03/24/2017 10:30 AM, zhoucm1 wrote:
>
>
> On 2017年03月24日 03:27, Nicolai Hähnle wrote:
>> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>>
>> We will add the fence to freed buffer objects in a later commit, to ensure
>> that the underlying memory can only be re-used after all references in
>> page tables have been cleared.
>>
>> Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
> Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++++++++++++++------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
>>   4 files changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index 55d553a..85e6070 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct
>> amdgpu_cs_parser *p)
>>       int i, r;
>>       r = amdgpu_vm_update_page_directory(adev, vm);
>>       if (r)
>>           return r;
>>       r = amdgpu_sync_fence(adev, &p->job->sync, vm->page_directory_fence);
>>       if (r)
>>           return r;
>> -    r = amdgpu_vm_clear_freed(adev, vm);
>> +    r = amdgpu_vm_clear_freed(adev, vm, NULL);
>>       if (r)
>>           return r;
>>       r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
>>       if (r)
>>           return r;
>>       r = amdgpu_sync_fence(adev, &p->job->sync,
>>                     fpriv->prt_va->last_pt_update);
>>       if (r)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index be9fb2c..4a53c43 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct
>> amdgpu_device *adev,
>>       r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,
>>                         NULL);
>>       if (r)
>>           goto error;
>>       r = amdgpu_vm_update_page_directory(adev, vm);
>>       if (r)
>>           goto error;
>> -    r = amdgpu_vm_clear_freed(adev, vm);
>> +    r = amdgpu_vm_clear_freed(adev, vm, NULL);
>>       if (r)
>>           goto error;
>>       if (operation == AMDGPU_VA_OP_MAP ||
>>           operation == AMDGPU_VA_OP_REPLACE)
>>           r = amdgpu_vm_bo_update(adev, bo_va, false);
>>   error:
>>       if (r && r != -ERESTARTSYS)
>>           DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index dd7df45..2c95a75 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device
>> *adev, struct amdgpu_vm *vm)
>>       }
>>       kfree(shared);
>>   }
>>   /**
>>    * amdgpu_vm_clear_freed - clear freed BOs in the PT
>>    *
>>    * @adev: amdgpu_device pointer
>>    * @vm: requested vm
>> + * @fence: optional resulting fence (unchanged if no work needed to be done
>> + * or if an error occurred)
>>    *
>>    * Make sure all freed BOs are cleared in the PT.
>>    * Returns 0 for success.
>>    *
>>    * PTs have to be reserved and mutex must be locked!
>>    */
>>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>> -              struct amdgpu_vm *vm)
>> +              struct amdgpu_vm *vm,
>> +              struct fence **fence)
>>   {
>>       struct amdgpu_bo_va_mapping *mapping;
>> -    struct fence *fence = NULL;
>> +    struct fence *f = NULL;
>>       int r;
>>       while (!list_empty(&vm->freed)) {
>>           mapping = list_first_entry(&vm->freed,
>>               struct amdgpu_bo_va_mapping, list);
>>           list_del(&mapping->list);
>>           r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,
>> -                           0, 0, &fence);
>> -        amdgpu_vm_free_mapping(adev, vm, mapping, fence);
>> +                           0, 0, &f);
>> +        amdgpu_vm_free_mapping(adev, vm, mapping, f);
>>           if (r) {
>> -            fence_put(fence);
>> +            fence_put(f);
>>               return r;
>>           }
>> +    }
>> +    if (fence && f) {
>> +        fence_put(*fence);
>> +        *fence = f;
>> +    } else {
>> +        fence_put(f);
>>       }
>> -    fence_put(fence);
>> +
>>       return 0;
>>   }
>>   /**
>>    * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
>>    *
>>    * @adev: amdgpu_device pointer
>>    * @vm: requested vm
>>    *
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index ff10fa5..9d5a572 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>>               struct amdgpu_vm *vm,
>>               uint64_t saddr, uint64_t size);
>>   int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
>>                 struct amdgpu_sync *sync, struct fence *fence,
>>                 struct amdgpu_job *job);
>>   int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job);
>>   void amdgpu_vm_reset_id(struct amdgpu_device *adev, unsigned vm_id);
>>   int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
>>                       struct amdgpu_vm *vm);
>>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>> -              struct amdgpu_vm *vm);
>> +              struct amdgpu_vm *vm,
>> +              struct fence **fence);
>>   int amdgpu_vm_clear_invalids(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>                    struct amdgpu_sync *sync);
>>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>               struct amdgpu_bo_va *bo_va,
>>               bool clear);
>>   void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>>                    struct amdgpu_bo *bo);
>>   struct amdgpu_bo_va *amdgpu_vm_bo_find(struct amdgpu_vm *vm,
>>                          struct amdgpu_bo *bo);
>>   struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>
> _______________________________________________
> 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] 8+ messages in thread

* Re: [PATCH v2 2/2] drm/amdgpu: clear freed mappings immediately when BO may be freed
       [not found]     ` <20170323192705.5474-2-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-03-24  2:31       ` zhoucm1
@ 2017-03-24  3:42       ` Zhang, Jerry (Junwei)
       [not found]         ` <58D495AA.9010607-5C7GfCeVMHo@public.gmane.org>
  1 sibling, 1 reply; 8+ messages in thread
From: Zhang, Jerry (Junwei) @ 2017-03-24  3:42 UTC (permalink / raw)
  To: Nicolai Hähnle, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Nicolai Hähnle

On 03/24/2017 03:27 AM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>
> Also, add the fence of the clear operations to the BO to ensure that
> the underlying memory can only be re-used after all PTEs pointing to
> it have been cleared.
>
> This avoids the following sequence of events that could be triggered
> by user space:
>
> 1. Submit a CS that accesses some BO _without_ adding that BO to the
>     buffer list.
> 2. Free that BO.
> 3. Some other task re-uses the memory underlying the BO.
> 4. The CS is submitted to the hardware and accesses memory that is
>     now already in use by somebody else.
>
> By clearing the page tables immediately in step 2, a GPU VM fault will
> be triggered in step 4 instead of wild memory accesses.
>
> v2: use amdgpu_bo_fence directly
>
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 4a53c43..8b0f5f18 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -145,20 +145,21 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>   	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>   	struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
>   	struct amdgpu_vm *vm = &fpriv->vm;
>
>   	struct amdgpu_bo_list_entry vm_pd;
>   	struct list_head list, duplicates;
>   	struct ttm_validate_buffer tv;
>   	struct ww_acquire_ctx ticket;
>   	struct amdgpu_bo_va *bo_va;
> +	struct fence *fence = NULL;
>   	int r;
>
>   	INIT_LIST_HEAD(&list);
>   	INIT_LIST_HEAD(&duplicates);
>
>   	tv.bo = &bo->tbo;
>   	tv.shared = true;
>   	list_add(&tv.head, &list);
>
>   	amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
> @@ -166,20 +167,31 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>   	r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
>   	if (r) {
>   		dev_err(adev->dev, "leaking bo va because "
>   			"we fail to reserve bo (%d)\n", r);
>   		return;
>   	}
>   	bo_va = amdgpu_vm_bo_find(vm, bo);
>   	if (bo_va) {
>   		if (--bo_va->ref_count == 0) {
>   			amdgpu_vm_bo_rmv(adev, bo_va);
> +
> +			r = amdgpu_vm_clear_freed(adev, vm, &fence);
> +			if (unlikely(r)) {
> +				dev_err(adev->dev, "failed to clear page "
> +					"tables on GEM object close (%d)\n", r);
> +			}
> +
> +			if (fence) {

I think it's always true.
Maybe you mean *fence?

> +				amdgpu_bo_fence(bo, fence, true);
> +				fence_put(fence);
> +			}
>   		}
>   	}
>   	ttm_eu_backoff_reservation(&ticket, &list);
>   }
>
>   static int amdgpu_gem_handle_lockup(struct amdgpu_device *adev, int r)
>   {
>   	if (r == -EDEADLK) {
>   		r = amdgpu_gpu_reset(adev);
>   		if (!r)
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] drm/amdgpu: clear freed mappings immediately when BO may be freed
       [not found]         ` <58D495AA.9010607-5C7GfCeVMHo@public.gmane.org>
@ 2017-03-24  3:45           ` Zhang, Jerry (Junwei)
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Jerry (Junwei) @ 2017-03-24  3:45 UTC (permalink / raw)
  To: Nicolai Hähnle, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Nicolai Hähnle

On 03/24/2017 11:42 AM, Zhang, Jerry (Junwei) wrote:
> On 03/24/2017 03:27 AM, Nicolai Hähnle wrote:
>> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>>
>> Also, add the fence of the clear operations to the BO to ensure that
>> the underlying memory can only be re-used after all PTEs pointing to
>> it have been cleared.
>>
>> This avoids the following sequence of events that could be triggered
>> by user space:
>>
>> 1. Submit a CS that accesses some BO _without_ adding that BO to the
>>     buffer list.
>> 2. Free that BO.
>> 3. Some other task re-uses the memory underlying the BO.
>> 4. The CS is submitted to the hardware and accesses memory that is
>>     now already in use by somebody else.
>>
>> By clearing the page tables immediately in step 2, a GPU VM fault will
>> be triggered in step 4 instead of wild memory accesses.
>>
>> v2: use amdgpu_bo_fence directly
>>
>> Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 4a53c43..8b0f5f18 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -145,20 +145,21 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>>       struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>       struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
>>       struct amdgpu_vm *vm = &fpriv->vm;
>>
>>       struct amdgpu_bo_list_entry vm_pd;
>>       struct list_head list, duplicates;
>>       struct ttm_validate_buffer tv;
>>       struct ww_acquire_ctx ticket;
>>       struct amdgpu_bo_va *bo_va;
>> +    struct fence *fence = NULL;
>>       int r;
>>
>>       INIT_LIST_HEAD(&list);
>>       INIT_LIST_HEAD(&duplicates);
>>
>>       tv.bo = &bo->tbo;
>>       tv.shared = true;
>>       list_add(&tv.head, &list);
>>
>>       amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
>> @@ -166,20 +167,31 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>>       r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
>>       if (r) {
>>           dev_err(adev->dev, "leaking bo va because "
>>               "we fail to reserve bo (%d)\n", r);
>>           return;
>>       }
>>       bo_va = amdgpu_vm_bo_find(vm, bo);
>>       if (bo_va) {
>>           if (--bo_va->ref_count == 0) {
>>               amdgpu_vm_bo_rmv(adev, bo_va);
>> +
>> +            r = amdgpu_vm_clear_freed(adev, vm, &fence);
>> +            if (unlikely(r)) {
>> +                dev_err(adev->dev, "failed to clear page "
>> +                    "tables on GEM object close (%d)\n", r);
>> +            }
>> +
>> +            if (fence) {
>
> I think it's always true.
> Maybe you mean *fence?

My fault to pick a wrong mail thread at the same time.
it's already a pointer, not **fence defined.

Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

>
>> +                amdgpu_bo_fence(bo, fence, true);
>> +                fence_put(fence);
>> +            }
>>           }
>>       }
>>       ttm_eu_backoff_reservation(&ticket, &list);
>>   }
>>
>>   static int amdgpu_gem_handle_lockup(struct amdgpu_device *adev, int r)
>>   {
>>       if (r == -EDEADLK) {
>>           r = amdgpu_gpu_reset(adev);
>>           if (!r)
>>
> _______________________________________________
> 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] 8+ messages in thread

* Re: [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed
       [not found] ` <20170323192705.5474-1-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-03-23 19:27   ` [PATCH v2 2/2] drm/amdgpu: clear freed mappings immediately when BO may be freed Nicolai Hähnle
  2017-03-24  2:30   ` [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed zhoucm1
@ 2017-03-24  7:40   ` Christian König
  2 siblings, 0 replies; 8+ messages in thread
From: Christian König @ 2017-03-24  7:40 UTC (permalink / raw)
  To: Nicolai Hähnle, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Nicolai Hähnle

Am 23.03.2017 um 20:27 schrieb Nicolai Hähnle:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>
> We will add the fence to freed buffer objects in a later commit, to ensure
> that the underlying memory can only be re-used after all references in
> page tables have been cleared.
>
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com> for both.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
>   4 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 55d553a..85e6070 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>   	int i, r;
>   
>   	r = amdgpu_vm_update_page_directory(adev, vm);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_sync_fence(adev, &p->job->sync, vm->page_directory_fence);
>   	if (r)
>   		return r;
>   
> -	r = amdgpu_vm_clear_freed(adev, vm);
> +	r = amdgpu_vm_clear_freed(adev, vm, NULL);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_sync_fence(adev, &p->job->sync,
>   			      fpriv->prt_va->last_pt_update);
>   	if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index be9fb2c..4a53c43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
>   
>   	r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,
>   				      NULL);
>   	if (r)
>   		goto error;
>   
>   	r = amdgpu_vm_update_page_directory(adev, vm);
>   	if (r)
>   		goto error;
>   
> -	r = amdgpu_vm_clear_freed(adev, vm);
> +	r = amdgpu_vm_clear_freed(adev, vm, NULL);
>   	if (r)
>   		goto error;
>   
>   	if (operation == AMDGPU_VA_OP_MAP ||
>   	    operation == AMDGPU_VA_OP_REPLACE)
>   		r = amdgpu_vm_bo_update(adev, bo_va, false);
>   
>   error:
>   	if (r && r != -ERESTARTSYS)
>   		DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index dd7df45..2c95a75 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   	}
>   
>   	kfree(shared);
>   }
>   
>   /**
>    * amdgpu_vm_clear_freed - clear freed BOs in the PT
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
> + * @fence: optional resulting fence (unchanged if no work needed to be done
> + * or if an error occurred)
>    *
>    * Make sure all freed BOs are cleared in the PT.
>    * Returns 0 for success.
>    *
>    * PTs have to be reserved and mutex must be locked!
>    */
>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> -			  struct amdgpu_vm *vm)
> +			  struct amdgpu_vm *vm,
> +			  struct fence **fence)
>   {
>   	struct amdgpu_bo_va_mapping *mapping;
> -	struct fence *fence = NULL;
> +	struct fence *f = NULL;
>   	int r;
>   
>   	while (!list_empty(&vm->freed)) {
>   		mapping = list_first_entry(&vm->freed,
>   			struct amdgpu_bo_va_mapping, list);
>   		list_del(&mapping->list);
>   
>   		r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,
> -					       0, 0, &fence);
> -		amdgpu_vm_free_mapping(adev, vm, mapping, fence);
> +					       0, 0, &f);
> +		amdgpu_vm_free_mapping(adev, vm, mapping, f);
>   		if (r) {
> -			fence_put(fence);
> +			fence_put(f);
>   			return r;
>   		}
> +	}
>   
> +	if (fence && f) {
> +		fence_put(*fence);
> +		*fence = f;
> +	} else {
> +		fence_put(f);
>   	}
> -	fence_put(fence);
> +
>   	return 0;
>   
>   }
>   
>   /**
>    * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index ff10fa5..9d5a572 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   			struct amdgpu_vm *vm,
>   			uint64_t saddr, uint64_t size);
>   int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
>   		      struct amdgpu_sync *sync, struct fence *fence,
>   		      struct amdgpu_job *job);
>   int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job);
>   void amdgpu_vm_reset_id(struct amdgpu_device *adev, unsigned vm_id);
>   int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
>   				    struct amdgpu_vm *vm);
>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> -			  struct amdgpu_vm *vm);
> +			  struct amdgpu_vm *vm,
> +			  struct fence **fence);
>   int amdgpu_vm_clear_invalids(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			     struct amdgpu_sync *sync);
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va *bo_va,
>   			bool clear);
>   void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   			     struct amdgpu_bo *bo);
>   struct amdgpu_bo_va *amdgpu_vm_bo_find(struct amdgpu_vm *vm,
>   				       struct amdgpu_bo *bo);
>   struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-03-24  7:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 19:27 [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed Nicolai Hähnle
     [not found] ` <20170323192705.5474-1-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-03-23 19:27   ` [PATCH v2 2/2] drm/amdgpu: clear freed mappings immediately when BO may be freed Nicolai Hähnle
     [not found]     ` <20170323192705.5474-2-nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-03-24  2:31       ` zhoucm1
2017-03-24  3:42       ` Zhang, Jerry (Junwei)
     [not found]         ` <58D495AA.9010607-5C7GfCeVMHo@public.gmane.org>
2017-03-24  3:45           ` Zhang, Jerry (Junwei)
2017-03-24  2:30   ` [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed zhoucm1
     [not found]     ` <58D484CB.1020306-5C7GfCeVMHo@public.gmane.org>
2017-03-24  3:40       ` Zhang, Jerry (Junwei)
2017-03-24  7:40   ` 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.