All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix
@ 2017-02-15 14:57 Christian König
       [not found] ` <1487170641-1927-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-02-15 14:57 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Christian König <christian.koenig@amd.com>

When two VMs stop using PRT support at the same time we might
not disable it in the right order otherwise.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bc32239..447cda5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
 {
 	struct amdgpu_prt_cb *cb = container_of(_cb, struct amdgpu_prt_cb, cb);
 
-	amdgpu_vm_update_prt_state(cb->adev);
+	if (atomic_dec_return(&cb->adev->vm_manager.num_prt_mappings) == 0)
+		amdgpu_vm_update_prt_state(cb->adev);
 	kfree(cb);
 }
 
@@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device *adev,
 				   struct amdgpu_bo_va_mapping *mapping,
 				   struct fence *fence)
 {
-	if ((mapping->flags & AMDGPU_PTE_PRT) &&
-	    atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0) {
+	if (mapping->flags & AMDGPU_PTE_PRT) {
 		struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
 						   GFP_KERNEL);
 
 		cb->adev = adev;
 		if (!fence || fence_add_callback(fence, &cb->cb,
-						 amdgpu_vm_prt_cb)) {
-			amdgpu_vm_update_prt_state(adev);
-			kfree(cb);
-		}
+						 amdgpu_vm_prt_cb))
+			amdgpu_vm_prt_cb(fence, &cb->cb);
 	}
 	kfree(mapping);
 }
-- 
2.5.0

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

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

* [PATCH 2/3] drm/amdgpu: add OOM fallback on PRT teardown
       [not found] ` <1487170641-1927-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-02-15 14:57   ` Christian König
  2017-02-15 14:57   ` [PATCH 3/3] drm/amdgpu: fix PRT teardown on VM fini v2 Christian König
  2017-02-15 15:59   ` [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix Christian König
  2 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2017-02-15 14:57 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Christian König <christian.koenig@amd.com>

Don't assume kmalloc will always succeed.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 447cda5..c11b6b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1194,14 +1194,22 @@ static void amdgpu_vm_update_prt_state(struct amdgpu_device *adev)
 }
 
 /**
+ * amdgpu_vm_prt_put - drop a PRT user
+ */
+static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
+{
+	if (atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0)
+		amdgpu_vm_update_prt_state(adev);
+}
+
+/**
  * amdgpu_vm_prt - callback for updating the PRT status
  */
 static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
 {
 	struct amdgpu_prt_cb *cb = container_of(_cb, struct amdgpu_prt_cb, cb);
 
-	if (atomic_dec_return(&cb->adev->vm_manager.num_prt_mappings) == 0)
-		amdgpu_vm_update_prt_state(cb->adev);
+	amdgpu_vm_prt_put(cb->adev);
 	kfree(cb);
 }
 
@@ -1224,10 +1232,18 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device *adev,
 		struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
 						   GFP_KERNEL);
 
-		cb->adev = adev;
-		if (!fence || fence_add_callback(fence, &cb->cb,
-						 amdgpu_vm_prt_cb))
-			amdgpu_vm_prt_cb(fence, &cb->cb);
+		if (!cb) {
+			/* Last resort when we are OOM */
+			if (fence)
+				fence_wait(fence, false);
+
+			amdgpu_vm_prt_put(cb->adev);
+		} else {
+			cb->adev = adev;
+			if (!fence || fence_add_callback(fence, &cb->cb,
+							 amdgpu_vm_prt_cb))
+				amdgpu_vm_prt_cb(fence, &cb->cb);
+		}
 	}
 	kfree(mapping);
 }
-- 
2.5.0

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

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

* [PATCH 3/3] drm/amdgpu: fix PRT teardown on VM fini v2
       [not found] ` <1487170641-1927-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2017-02-15 14:57   ` [PATCH 2/3] drm/amdgpu: add OOM fallback on PRT teardown Christian König
@ 2017-02-15 14:57   ` Christian König
  2017-02-15 15:59   ` [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix Christian König
  2 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2017-02-15 14:57 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Christian König <christian.koenig@amd.com>

v2: new approach fixing this by registering a fence callback for
    all users of the VM on teardown

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c11b6b6..d3437ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1188,22 +1188,31 @@ static void amdgpu_vm_update_prt_state(struct amdgpu_device *adev)
 	bool enable;
 
 	spin_lock_irqsave(&adev->vm_manager.prt_lock, flags);
-	enable = !!atomic_read(&adev->vm_manager.num_prt_mappings);
+	enable = !!atomic_read(&adev->vm_manager.num_prt_users);
 	adev->gart.gart_funcs->set_prt(adev, enable);
 	spin_unlock_irqrestore(&adev->vm_manager.prt_lock, flags);
 }
 
 /**
+ * amdgpu_vm_prt_put - add a PRT user
+ */
+static void amdgpu_vm_prt_get(struct amdgpu_device *adev)
+{
+	if (atomic_inc_return(&adev->vm_manager.num_prt_users) == 1)
+		amdgpu_vm_update_prt_state(adev);
+}
+
+/**
  * amdgpu_vm_prt_put - drop a PRT user
  */
 static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
 {
-	if (atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0)
+	if (atomic_dec_return(&adev->vm_manager.num_prt_users) == 0)
 		amdgpu_vm_update_prt_state(adev);
 }
 
 /**
- * amdgpu_vm_prt - callback for updating the PRT status
+ * amdgpu_vm_prt_cb - callback for updating the PRT status
  */
 static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
 {
@@ -1214,6 +1223,29 @@ static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
 }
 
 /**
+ * amdgpu_vm_add_prt_cb - add callback for updating the PRT status
+ */
+static void amdgpu_vm_add_prt_cb(struct amdgpu_device *adev,
+				 struct fence *fence)
+{
+	struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
+					   GFP_KERNEL);
+
+	if (!cb) {
+		/* Last resort when we are OOM */
+		if (fence)
+			fence_wait(fence, false);
+
+		amdgpu_vm_prt_put(cb->adev);
+	} else {
+		cb->adev = adev;
+		if (!fence || fence_add_callback(fence, &cb->cb,
+						 amdgpu_vm_prt_cb))
+			amdgpu_vm_prt_cb(fence, &cb->cb);
+	}
+}
+
+/**
  * amdgpu_vm_free_mapping - free a mapping
  *
  * @adev: amdgpu_device pointer
@@ -1228,24 +1260,47 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device *adev,
 				   struct amdgpu_bo_va_mapping *mapping,
 				   struct fence *fence)
 {
-	if (mapping->flags & AMDGPU_PTE_PRT) {
-		struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
-						   GFP_KERNEL);
+	if (mapping->flags & AMDGPU_PTE_PRT)
+		amdgpu_vm_add_prt_cb(adev, fence);
+	kfree(mapping);
+}
 
-		if (!cb) {
-			/* Last resort when we are OOM */
-			if (fence)
-				fence_wait(fence, false);
+/**
+ * amdgpu_vm_prt_fini - finish all prt mappings
+ *
+ * @adev: amdgpu_device pointer
+ * @vm: requested vm
+ *
+ * Register a cleanup callback to disable PRT support after VM dies.
+ */
+static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
+{
+	struct reservation_object *resv = vm->page_directory->tbo.resv;
+	struct fence *excl, **shared;
+	unsigned i, shared_count;
+	int r;
 
-			amdgpu_vm_prt_put(cb->adev);
-		} else {
-			cb->adev = adev;
-			if (!fence || fence_add_callback(fence, &cb->cb,
-							 amdgpu_vm_prt_cb))
-				amdgpu_vm_prt_cb(fence, &cb->cb);
-		}
+	r = reservation_object_get_fences_rcu(resv, &excl,
+					      &shared_count, &shared);
+	if (r) {
+		/* Not enough memory to grab the fence list, as last resort
+		 * block for all the fences to complete.
+		 */
+		reservation_object_wait_timeout_rcu(resv, true, false,
+						    MAX_SCHEDULE_TIMEOUT);
+		return;
 	}
-	kfree(mapping);
+
+	/* Add a callback for each fence in the reservation object */
+	amdgpu_vm_prt_get(adev);
+	amdgpu_vm_add_prt_cb(adev, excl);
+
+	for (i = 0; i < shared_count; ++i) {
+		amdgpu_vm_prt_get(adev);
+		amdgpu_vm_add_prt_cb(adev, shared[i]);
+	}
+
+	kfree(shared);
 }
 
 /**
@@ -1395,8 +1450,7 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 		if (!adev->gart.gart_funcs->set_prt)
 			return -EINVAL;
 
-		if (atomic_inc_return(&adev->vm_manager.num_prt_mappings) == 1)
-			amdgpu_vm_update_prt_state(adev);
+		amdgpu_vm_prt_get(adev);
 	}
 
 	/* make sure object fit at this offset */
@@ -1699,6 +1753,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 {
 	struct amdgpu_bo_va_mapping *mapping, *tmp;
+	bool prt_fini_called = false;
 	int i;
 
 	amd_sched_entity_fini(vm->entity.sched, &vm->entity);
@@ -1712,13 +1767,14 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 		kfree(mapping);
 	}
 	list_for_each_entry_safe(mapping, tmp, &vm->freed, list) {
-		if (mapping->flags & AMDGPU_PTE_PRT)
-			continue;
+		if (mapping->flags & AMDGPU_PTE_PRT && !prt_fini_called) {
+			amdgpu_vm_prt_fini(adev, vm);
+			prt_fini_called = true;
+		}
 
 		list_del(&mapping->list);
-		kfree(mapping);
+		amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
 	}
-	amdgpu_vm_clear_freed(adev, vm);
 
 	for (i = 0; i < amdgpu_vm_num_pdes(adev); i++) {
 		struct amdgpu_bo *pt = vm->page_tables[i].bo;
@@ -1764,7 +1820,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
 	atomic_set(&adev->vm_manager.vm_pte_next_ring, 0);
 	atomic64_set(&adev->vm_manager.client_counter, 0);
 	spin_lock_init(&adev->vm_manager.prt_lock);
-	atomic_set(&adev->vm_manager.num_prt_mappings, 0);
+	atomic_set(&adev->vm_manager.num_prt_users, 0);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 51fa12f..7fccd486 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -164,7 +164,7 @@ struct amdgpu_vm_manager {
 
 	/* partial resident texture handling */
 	spinlock_t				prt_lock;
-	atomic_t				num_prt_mappings;
+	atomic_t				num_prt_users;
 };
 
 void amdgpu_vm_manager_init(struct amdgpu_device *adev);
-- 
2.5.0

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

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

* Re: [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix
       [not found] ` <1487170641-1927-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2017-02-15 14:57   ` [PATCH 2/3] drm/amdgpu: add OOM fallback on PRT teardown Christian König
  2017-02-15 14:57   ` [PATCH 3/3] drm/amdgpu: fix PRT teardown on VM fini v2 Christian König
@ 2017-02-15 15:59   ` Christian König
       [not found]     ` <79b48c6a-964c-7339-4b5b-1d97401bb107-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-02-15 15:59 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Haehnle, Nicolai

Nicolai could you give that set a try?

It should fix your problems with PRT tear down on process crash.

Regards,
Christian.

Am 15.02.2017 um 15:57 schrieb Christian König:
> From: Christian König <christian.koenig@amd.com>
>
> When two VMs stop using PRT support at the same time we might
> not disable it in the right order otherwise.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index bc32239..447cda5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
>   {
>   	struct amdgpu_prt_cb *cb = container_of(_cb, struct amdgpu_prt_cb, cb);
>   
> -	amdgpu_vm_update_prt_state(cb->adev);
> +	if (atomic_dec_return(&cb->adev->vm_manager.num_prt_mappings) == 0)
> +		amdgpu_vm_update_prt_state(cb->adev);
>   	kfree(cb);
>   }
>   
> @@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device *adev,
>   				   struct amdgpu_bo_va_mapping *mapping,
>   				   struct fence *fence)
>   {
> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&
> -	    atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0) {
> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>   		struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
>   						   GFP_KERNEL);
>   
>   		cb->adev = adev;
>   		if (!fence || fence_add_callback(fence, &cb->cb,
> -						 amdgpu_vm_prt_cb)) {
> -			amdgpu_vm_update_prt_state(adev);
> -			kfree(cb);
> -		}
> +						 amdgpu_vm_prt_cb))
> +			amdgpu_vm_prt_cb(fence, &cb->cb);
>   	}
>   	kfree(mapping);
>   }


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

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

* Re: [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix
       [not found]     ` <79b48c6a-964c-7339-4b5b-1d97401bb107-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-02-15 17:28       ` Nicolai Hähnle
       [not found]         ` <1300e2ee-ed90-7920-d135-b75a09347880-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Nicolai Hähnle @ 2017-02-15 17:28 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Hi Christian,

On 15.02.2017 16:59, Christian König wrote:
> Nicolai could you give that set a try?
>
> It should fix your problems with PRT tear down on process crash.

Yes, it fixes those issues for me, thanks! The first two patches have my 
R-b, for the third one I don't really understand the bug that it fixes, 
but I have to leave soon, so...

Thanks
Nicolai

> Regards,
> Christian.
>
> Am 15.02.2017 um 15:57 schrieb Christian König:
>> From: Christian König <christian.koenig@amd.com>
>>
>> When two VMs stop using PRT support at the same time we might
>> not disable it in the right order otherwise.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++++-------
>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index bc32239..447cda5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence
>> *fence, struct fence_cb *_cb)
>>   {
>>       struct amdgpu_prt_cb *cb = container_of(_cb, struct
>> amdgpu_prt_cb, cb);
>>   -    amdgpu_vm_update_prt_state(cb->adev);
>> +    if (atomic_dec_return(&cb->adev->vm_manager.num_prt_mappings) == 0)
>> +        amdgpu_vm_update_prt_state(cb->adev);
>>       kfree(cb);
>>   }
>>   @@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct
>> amdgpu_device *adev,
>>                      struct amdgpu_bo_va_mapping *mapping,
>>                      struct fence *fence)
>>   {
>> -    if ((mapping->flags & AMDGPU_PTE_PRT) &&
>> -        atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0) {
>> +    if (mapping->flags & AMDGPU_PTE_PRT) {
>>           struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct
>> amdgpu_prt_cb),
>>                              GFP_KERNEL);
>>             cb->adev = adev;
>>           if (!fence || fence_add_callback(fence, &cb->cb,
>> -                         amdgpu_vm_prt_cb)) {
>> -            amdgpu_vm_update_prt_state(adev);
>> -            kfree(cb);
>> -        }
>> +                         amdgpu_vm_prt_cb))
>> +            amdgpu_vm_prt_cb(fence, &cb->cb);
>>       }
>>       kfree(mapping);
>>   }
>
>
> _______________________________________________
> 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] 7+ messages in thread

* Re: [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix
       [not found]         ` <1300e2ee-ed90-7920-d135-b75a09347880-5C7GfCeVMHo@public.gmane.org>
@ 2017-02-16 10:01           ` Christian König
       [not found]             ` <7820c81d-28f6-c2a3-2c3b-31be54bfe29e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-02-16 10:01 UTC (permalink / raw)
  To: Nicolai Hähnle, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 15.02.2017 um 18:28 schrieb Nicolai Hähnle:
> Hi Christian,
>
> On 15.02.2017 16:59, Christian König wrote:
>> Nicolai could you give that set a try?
>>
>> It should fix your problems with PRT tear down on process crash.
>
> Yes, it fixes those issues for me, thanks! The first two patches have 
> my R-b, for the third one I don't really understand the bug that it 
> fixes, but I have to leave soon, so...

The problem was what to do when we tear down the VM, but still have PRT 
mappings.

Initially I've just tried to unmap the PRT mappings as if they would 
have been cleaned up by userspace, but this doesn't work because the 
PD/PT might be swapped out etc....

Now instead of trying to unmap the PRT mappings I just increase the PRT 
reference count for each command submission still using the VM.

Can I get an review on those patches?

Thanks,
Christian.

>
> Thanks
> Nicolai
>
>> Regards,
>> Christian.
>>
>> Am 15.02.2017 um 15:57 schrieb Christian König:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> When two VMs stop using PRT support at the same time we might
>>> not disable it in the right order otherwise.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++++-------
>>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index bc32239..447cda5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence
>>> *fence, struct fence_cb *_cb)
>>>   {
>>>       struct amdgpu_prt_cb *cb = container_of(_cb, struct
>>> amdgpu_prt_cb, cb);
>>>   -    amdgpu_vm_update_prt_state(cb->adev);
>>> +    if (atomic_dec_return(&cb->adev->vm_manager.num_prt_mappings) 
>>> == 0)
>>> +        amdgpu_vm_update_prt_state(cb->adev);
>>>       kfree(cb);
>>>   }
>>>   @@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct
>>> amdgpu_device *adev,
>>>                      struct amdgpu_bo_va_mapping *mapping,
>>>                      struct fence *fence)
>>>   {
>>> -    if ((mapping->flags & AMDGPU_PTE_PRT) &&
>>> - atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0) {
>>> +    if (mapping->flags & AMDGPU_PTE_PRT) {
>>>           struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct
>>> amdgpu_prt_cb),
>>>                              GFP_KERNEL);
>>>             cb->adev = adev;
>>>           if (!fence || fence_add_callback(fence, &cb->cb,
>>> -                         amdgpu_vm_prt_cb)) {
>>> -            amdgpu_vm_update_prt_state(adev);
>>> -            kfree(cb);
>>> -        }
>>> +                         amdgpu_vm_prt_cb))
>>> +            amdgpu_vm_prt_cb(fence, &cb->cb);
>>>       }
>>>       kfree(mapping);
>>>   }
>>
>>
>> _______________________________________________
>> 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] 7+ messages in thread

* Re: [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix
       [not found]             ` <7820c81d-28f6-c2a3-2c3b-31be54bfe29e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-02-16 10:12               ` Nicolai Hähnle
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolai Hähnle @ 2017-02-16 10:12 UTC (permalink / raw)
  To: Christian König, Nicolai Hähnle,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 16.02.2017 11:01, Christian König wrote:
> Am 15.02.2017 um 18:28 schrieb Nicolai Hähnle:
>> Hi Christian,
>>
>> On 15.02.2017 16:59, Christian König wrote:
>>> Nicolai could you give that set a try?
>>>
>>> It should fix your problems with PRT tear down on process crash.
>>
>> Yes, it fixes those issues for me, thanks! The first two patches have
>> my R-b, for the third one I don't really understand the bug that it
>> fixes, but I have to leave soon, so...
>
> The problem was what to do when we tear down the VM, but still have PRT
> mappings.
>
> Initially I've just tried to unmap the PRT mappings as if they would
> have been cleaned up by userspace, but this doesn't work because the
> PD/PT might be swapped out etc....
>
> Now instead of trying to unmap the PRT mappings I just increase the PRT
> reference count for each command submission still using the VM.

Ah, I see, the update functions assume that the relevant buffers holding 
the page tables have been validated, but this isn't the case during 
teardown. Makes sense to me now, thanks!

For the series:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>

> Can I get an review on those patches?
>
> Thanks,
> Christian.
>
>>
>> Thanks
>> Nicolai
>>
>>> Regards,
>>> Christian.
>>>
>>> Am 15.02.2017 um 15:57 schrieb Christian König:
>>>> From: Christian König <christian.koenig@amd.com>
>>>>
>>>> When two VMs stop using PRT support at the same time we might
>>>> not disable it in the right order otherwise.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++++-------
>>>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> index bc32239..447cda5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence
>>>> *fence, struct fence_cb *_cb)
>>>>   {
>>>>       struct amdgpu_prt_cb *cb = container_of(_cb, struct
>>>> amdgpu_prt_cb, cb);
>>>>   -    amdgpu_vm_update_prt_state(cb->adev);
>>>> +    if (atomic_dec_return(&cb->adev->vm_manager.num_prt_mappings)
>>>> == 0)
>>>> +        amdgpu_vm_update_prt_state(cb->adev);
>>>>       kfree(cb);
>>>>   }
>>>>   @@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct
>>>> amdgpu_device *adev,
>>>>                      struct amdgpu_bo_va_mapping *mapping,
>>>>                      struct fence *fence)
>>>>   {
>>>> -    if ((mapping->flags & AMDGPU_PTE_PRT) &&
>>>> - atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0) {
>>>> +    if (mapping->flags & AMDGPU_PTE_PRT) {
>>>>           struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct
>>>> amdgpu_prt_cb),
>>>>                              GFP_KERNEL);
>>>>             cb->adev = adev;
>>>>           if (!fence || fence_add_callback(fence, &cb->cb,
>>>> -                         amdgpu_vm_prt_cb)) {
>>>> -            amdgpu_vm_update_prt_state(adev);
>>>> -            kfree(cb);
>>>> -        }
>>>> +                         amdgpu_vm_prt_cb))
>>>> +            amdgpu_vm_prt_cb(fence, &cb->cb);
>>>>       }
>>>>       kfree(mapping);
>>>>   }
>>>
>>>
>>> _______________________________________________
>>> 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

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

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

end of thread, other threads:[~2017-02-16 10:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-15 14:57 [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix Christian König
     [not found] ` <1487170641-1927-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-02-15 14:57   ` [PATCH 2/3] drm/amdgpu: add OOM fallback on PRT teardown Christian König
2017-02-15 14:57   ` [PATCH 3/3] drm/amdgpu: fix PRT teardown on VM fini v2 Christian König
2017-02-15 15:59   ` [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix Christian König
     [not found]     ` <79b48c6a-964c-7339-4b5b-1d97401bb107-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-02-15 17:28       ` Nicolai Hähnle
     [not found]         ` <1300e2ee-ed90-7920-d135-b75a09347880-5C7GfCeVMHo@public.gmane.org>
2017-02-16 10:01           ` Christian König
     [not found]             ` <7820c81d-28f6-c2a3-2c3b-31be54bfe29e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-02-16 10:12               ` Nicolai Hähnle

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.