[PATCH] drm/radeon: fence virtual address and free it once idle [3.5] v4

[PATCH] drm/radeon: fence virtual address and free it once idle [3.5] v4

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

Virtual address need to be fenced to know when we can safely remove it.
This patch also properly clear the pagetable. Previously it was
serouisly broken.

v2: For to update pagetable when unbinding bo (don't bailout if
    bo_va->valid is true).
v3: Fix compilation warnings
v4: We need a special version for 3.5 because the locking scheme
    is different btw 3.5 and 3.6. There is no longer cs mutex in
    3.6 instead there is a global vm mutex.

This version is for stable 3.5 only.

cc: stable@vger.kernel.org
Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h        |  1 +
 drivers/gpu/drm/radeon/radeon_cs.c     | 30 +++++++++++++++++++++++++++---
 drivers/gpu/drm/radeon/radeon_gart.c   | 24 ++++++++++++++++++++++--
 drivers/gpu/drm/radeon/radeon_gem.c    | 13 ++-----------
 drivers/gpu/drm/radeon/radeon_object.c |  6 +-----
 5 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index fefcca5..01d2a87 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -323,6 +323,7 @@ struct radeon_bo_va {
 	uint64_t			soffset;
 	uint64_t			eoffset;
 	uint32_t			flags;
+	struct radeon_fence		*fence;
 	bool				valid;
 };
 
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index 142f894..3680df0 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -294,6 +294,28 @@ int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data)
 	return 0;
 }
 
+static void radeon_bo_vm_fence_va(struct radeon_cs_parser *parser,
+				  struct radeon_fence *fence)
+{
+	struct radeon_fpriv *fpriv = parser->filp->driver_priv;
+	struct radeon_vm *vm = &fpriv->vm;
+	struct radeon_bo_list *lobj;
+
+	if (parser->chunk_ib_idx == -1)
+		return;
+	if ((parser->cs_flags & RADEON_CS_USE_VM) == 0)
+		return;
+
+	list_for_each_entry(lobj, &parser->validated, tv.head) {
+		struct radeon_bo_va *bo_va;
+		struct radeon_bo *rbo = lobj->bo;
+
+		bo_va = radeon_bo_va(rbo, vm);
+		radeon_fence_unref(&bo_va->fence);
+		bo_va->fence = radeon_fence_ref(fence);
+	}
+}
+
 /**
  * cs_parser_fini() - clean parser states
  * @parser:	parser structure holding parsing context.
@@ -306,11 +328,14 @@ static void radeon_cs_parser_fini(struct radeon_cs_parser *parser, int error)
 {
 	unsigned i;
 
-	if (!error)
+	if (!error) {
+		/* fence all bo va before ttm_eu_fence_buffer_objects so bo are still reserved */
+		radeon_bo_vm_fence_va(parser, parser->ib.fence);
 		ttm_eu_fence_buffer_objects(&parser->validated,
 					    parser->ib.fence);
-	else
+	} else {
 		ttm_eu_backoff_reservation(&parser->validated);
+	}
 
 	if (parser->relocs != NULL) {
 		for (i = 0; i < parser->nrelocs; i++) {
@@ -407,7 +432,6 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev,
 
 	if (parser->chunk_ib_idx == -1)
 		return 0;
-
 	if ((parser->cs_flags & RADEON_CS_USE_VM) == 0)
 		return 0;
 
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 84b648a..f651f22 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -564,7 +564,7 @@ int radeon_vm_bo_update_pte(struct radeon_device *rdev,
 		return -EINVAL;
 	}
 
-	if (bo_va->valid)
+	if (bo_va->valid && mem)
 		return 0;
 
 	ngpu_pages = radeon_bo_ngpu_pages(bo);
@@ -597,11 +597,27 @@ int radeon_vm_bo_rmv(struct radeon_device *rdev,
 		     struct radeon_bo *bo)
 {
 	struct radeon_bo_va *bo_va;
+	int r;
 
 	bo_va = radeon_bo_va(bo, vm);
 	if (bo_va == NULL)
 		return 0;
 
+	/* wait for va use to end */
+	while (bo_va->fence) {
+		r = radeon_fence_wait(bo_va->fence, false);
+		if (r) {
+			DRM_ERROR("error while waiting for fence: %d\n", r);
+		}
+		if (r == -EDEADLK) {
+			r = radeon_gpu_reset(rdev);
+			if (!r)
+				continue;
+		}
+		break;
+	}
+	radeon_fence_unref(&bo_va->fence);
+
 	radeon_mutex_lock(&rdev->cs_mutex);
 	mutex_lock(&vm->mutex);
 	radeon_vm_bo_update_pte(rdev, vm, bo, NULL);
@@ -661,12 +677,15 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm)
 	radeon_vm_unbind_locked(rdev, vm);
 	radeon_mutex_unlock(&rdev->cs_mutex);
 
-	/* remove all bo */
+	/* remove all bo at this point non are busy any more because unbind
+	 * waited for the last vm fence to signal
+	 */
 	r = radeon_bo_reserve(rdev->ring_tmp_bo.bo, false);
 	if (!r) {
 		bo_va = radeon_bo_va(rdev->ring_tmp_bo.bo, vm);
 		list_del_init(&bo_va->bo_list);
 		list_del_init(&bo_va->vm_list);
+		radeon_fence_unref(&bo_va->fence);
 		radeon_bo_unreserve(rdev->ring_tmp_bo.bo);
 		kfree(bo_va);
 	}
@@ -678,6 +697,7 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm)
 		r = radeon_bo_reserve(bo_va->bo, false);
 		if (!r) {
 			list_del_init(&bo_va->bo_list);
+			radeon_fence_unref(&bo_va->fence);
 			radeon_bo_unreserve(bo_va->bo);
 			kfree(bo_va);
 		}
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 21ec9f5..12207d9 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -134,25 +134,16 @@ void radeon_gem_object_close(struct drm_gem_object *obj,
 	struct radeon_device *rdev = rbo->rdev;
 	struct radeon_fpriv *fpriv = file_priv->driver_priv;
 	struct radeon_vm *vm = &fpriv->vm;
-	struct radeon_bo_va *bo_va, *tmp;
 
 	if (rdev->family < CHIP_CAYMAN) {
 		return;
 	}
 
 	if (radeon_bo_reserve(rbo, false)) {
+		dev_err(rdev->dev, "leaking bo va because we fail to reserve bo\n");
 		return;
 	}
-	list_for_each_entry_safe(bo_va, tmp, &rbo->va, bo_list) {
-		if (bo_va->vm == vm) {
-			/* remove from this vm address space */
-			mutex_lock(&vm->mutex);
-			list_del(&bo_va->vm_list);
-			mutex_unlock(&vm->mutex);
-			list_del(&bo_va->bo_list);
-			kfree(bo_va);
-		}
-	}
+	radeon_vm_bo_rmv(rdev, vm, rbo);
 	radeon_bo_unreserve(rbo);
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 830f1a7..1b2289f 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -52,11 +52,7 @@ void radeon_bo_clear_va(struct radeon_bo *bo)
 
 	list_for_each_entry_safe(bo_va, tmp, &bo->va, bo_list) {
 		/* remove from all vm address space */
-		mutex_lock(&bo_va->vm->mutex);
-		list_del(&bo_va->vm_list);
-		mutex_unlock(&bo_va->vm->mutex);
-		list_del(&bo_va->bo_list);
-		kfree(bo_va);
+		radeon_vm_bo_rmv(bo->rdev, bo_va->vm, bo);
 	}
 }
 
-- 
1.7.11.2

Re: [PATCH] drm/radeon: fence virtual address and free it once idle [3.5] v4

[Greg KH]

On Mon, Aug 06, 2012 at 12:56:04PM -0400, j.glisse@gmail.com wrote:
> From: Jerome Glisse <jglisse@redhat.com>
> 
> Virtual address need to be fenced to know when we can safely remove it.
> This patch also properly clear the pagetable. Previously it was
> serouisly broken.
> 
> v2: For to update pagetable when unbinding bo (don't bailout if
>     bo_va->valid is true).
> v3: Fix compilation warnings
> v4: We need a special version for 3.5 because the locking scheme
>     is different btw 3.5 and 3.6. There is no longer cs mutex in
>     3.6 instead there is a global vm mutex.
> 
> This version is for stable 3.5 only.

Is the version that fixes this problem in the 3.6 tree, already in
Linus's tree?  If so, what is the git commit id?  If not, I need to wait
until it gets in there, so please tell me when it does so.

greg k-h

Re: [PATCH] drm/radeon: fence virtual address and free it once idle [3.5] v4

[Alex Deucher]

On Thu, Aug 16, 2012 at 7:13 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Aug 06, 2012 at 12:56:04PM -0400, j.glisse@gmail.com wrote:
>> From: Jerome Glisse <jglisse@redhat.com>
>>
>> Virtual address need to be fenced to know when we can safely remove it.
>> This patch also properly clear the pagetable. Previously it was
>> serouisly broken.
>>
>> v2: For to update pagetable when unbinding bo (don't bailout if
>>     bo_va->valid is true).
>> v3: Fix compilation warnings
>> v4: We need a special version for 3.5 because the locking scheme
>>     is different btw 3.5 and 3.6. There is no longer cs mutex in
>>     3.6 instead there is a global vm mutex.
>>
>> This version is for stable 3.5 only.
>
> Is the version that fixes this problem in the 3.6 tree, already in
> Linus's tree?  If so, what is the git commit id?  If not, I need to wait
> until it gets in there, so please tell me when it does so.

Yes:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e43b5ec05afdc232be25aa481315035c1888d389

Please apply to stable.

Alex

Re: [PATCH] drm/radeon: fence virtual address and free it once idle [3.5] v4

[Greg KH]

On Fri, Aug 17, 2012 at 09:34:46AM -0400, Alex Deucher wrote:
> On Thu, Aug 16, 2012 at 7:13 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Aug 06, 2012 at 12:56:04PM -0400, j.glisse@gmail.com wrote:
> >> From: Jerome Glisse <jglisse@redhat.com>
> >>
> >> Virtual address need to be fenced to know when we can safely remove it.
> >> This patch also properly clear the pagetable. Previously it was
> >> serouisly broken.
> >>
> >> v2: For to update pagetable when unbinding bo (don't bailout if
> >>     bo_va->valid is true).
> >> v3: Fix compilation warnings
> >> v4: We need a special version for 3.5 because the locking scheme
> >>     is different btw 3.5 and 3.6. There is no longer cs mutex in
> >>     3.6 instead there is a global vm mutex.
> >>
> >> This version is for stable 3.5 only.
> >
> > Is the version that fixes this problem in the 3.6 tree, already in
> > Linus's tree?  If so, what is the git commit id?  If not, I need to wait
> > until it gets in there, so please tell me when it does so.
> 
> Yes:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e43b5ec05afdc232be25aa481315035c1888d389
> 
> Please apply to stable.

Now applied, thanks.

greg k-h